diff --git a/internal/locate/region_request.go b/internal/locate/region_request.go index 683dacd3bd..b952127f29 100644 --- a/internal/locate/region_request.go +++ b/internal/locate/region_request.go @@ -1015,6 +1015,17 @@ func (s *sendReqState) next() (done bool) { } } + if len(req.Context.TraceId) == 0 { + if trace.IsCategoryEnabled(trace.CategoryDevDebug) { + trace.TraceEvent(bo.GetCtx(), trace.CategoryDevDebug, "send request with trace_id missing", + zap.Stack("stack")) + + // If trace ID is not set, it may indicate the caller is not using client-go properly. + // Print the stack trace to capture all those cases. + trace.CheckFlightRecorderDumpTrigger(bo.GetCtx(), "dump_trigger.suspicious_event.dev_debug", "send_request_trace_id_missing") + } + } + if _, err := util.EvalFailpoint("beforeSendReqToRegion"); err == nil { if hook := bo.GetCtx().Value("sendReqToRegionHook"); hook != nil { h := hook.(func(*tikvrpc.Request)) @@ -1165,7 +1176,6 @@ func (s *sendReqState) send() (canceled bool) { fields := []zap.Field{ zap.Stringer("cmd", req.Type), zap.Duration("latency", rpcDuration), - zap.Bool("success", s.vars.err == nil && s.vars.resp != nil), } if s.vars.err != nil { fields = append(fields, zap.Error(s.vars.err)) diff --git a/trace/trace.go b/trace/trace.go index b69529d9fa..63c6be177b 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -33,6 +33,8 @@ const ( CategoryKVRequest // CategoryRegionCache traces region cache operations and PD lookups. CategoryRegionCache + // CategoryDevDebug is used for development and debugging purposes. + CategoryDevDebug ) // TraceEventFunc is the function signature for recording trace events. @@ -55,11 +57,17 @@ func noopTraceControlExtractor(context.Context) TraceControlFlags { return FlagTiKVCategoryRequest } +// CheckFlightRecorderDumpTriggerFunc is the function signature for checking whether flight recorder should trigger a dump. +type CheckFlightRecorderDumpTriggerFunc func(ctx context.Context, triggerCanonicalName string, val any) + +func noopCheckFlightRecorderDumpTrigger(ctx context.Context, triggerCanonicalName string, val any) {} + // Global function pointers stored independently var ( - globalTraceEventFunc atomic.Pointer[TraceEventFunc] - globalIsCategoryEnabledFunc atomic.Pointer[IsCategoryEnabledFunc] - globalTraceControlExtractorFunc atomic.Pointer[TraceControlExtractorFunc] + globalTraceEventFunc atomic.Pointer[TraceEventFunc] + globalIsCategoryEnabledFunc atomic.Pointer[IsCategoryEnabledFunc] + globalTraceControlExtractorFunc atomic.Pointer[TraceControlExtractorFunc] + globalCheckFlightRecorderDumpTriggerFunc atomic.Pointer[CheckFlightRecorderDumpTriggerFunc] ) func init() { @@ -67,9 +75,27 @@ func init() { defaultTraceEvent := TraceEventFunc(noopTraceEvent) defaultIsCategoryEnabled := IsCategoryEnabledFunc(noopIsCategoryEnabled) defaultTraceControlExtractor := TraceControlExtractorFunc(noopTraceControlExtractor) + defaultCheckFlightRecorderDumpTrigger := CheckFlightRecorderDumpTriggerFunc(noopCheckFlightRecorderDumpTrigger) globalTraceEventFunc.Store(&defaultTraceEvent) globalIsCategoryEnabledFunc.Store(&defaultIsCategoryEnabled) globalTraceControlExtractorFunc.Store(&defaultTraceControlExtractor) + globalCheckFlightRecorderDumpTriggerFunc.Store(&defaultCheckFlightRecorderDumpTrigger) +} + +// CheckFlightRecorderDumpTrigger checks whether flight recorder should trigger a dump. +func CheckFlightRecorderDumpTrigger(ctx context.Context, triggerCanonicalName string, val any) { + fn := *globalCheckFlightRecorderDumpTriggerFunc.Load() + fn(ctx, triggerCanonicalName, val) +} + +// SetCheckFlightRecorderDumpTriggerFunc sets the flight recorder dump trigger function. +// This is typically called once during application initialization (e.g., by TiDB). +// Passing nil will use a no-op implementation. +func SetCheckFlightRecorderDumpTriggerFunc(fn CheckFlightRecorderDumpTriggerFunc) { + if fn == nil { + fn = noopCheckFlightRecorderDumpTrigger + } + globalCheckFlightRecorderDumpTriggerFunc.Store(&fn) } // SetTraceEventFunc registers the trace event handler function. diff --git a/trace/trace_test.go b/trace/trace_test.go index 191a3bfe8e..a47fad4b67 100644 --- a/trace/trace_test.go +++ b/trace/trace_test.go @@ -142,3 +142,25 @@ func TestTraceIDContext(t *testing.T) { extractedID2 := TraceIDFromContext(ctxWithTrace2) require.Equal(t, traceID2, extractedID2) } + +func TestCheckFlightRecorderDumpTrigger(t *testing.T) { + original := globalCheckFlightRecorderDumpTriggerFunc.Load() + defer func() { + globalCheckFlightRecorderDumpTriggerFunc.Store(original) + }() + + // Test default behavior (returns false) + ctx := context.Background() + CheckFlightRecorderDumpTrigger(ctx, "just cover some code", 42) + + // Test custom behavior + var covered bool + mock := func(ctx context.Context, triggerName string, val any) { + covered = true + } + SetCheckFlightRecorderDumpTriggerFunc(CheckFlightRecorderDumpTriggerFunc(mock)) + + require.False(t, covered) + CheckFlightRecorderDumpTrigger(ctx, "custom trigger", 123) + require.True(t, covered) +}