fix(llmobs): include missing tags and make SubmitEvaluationFromSpan more flexible (#4050)

Author: rarguelloF

instrumentation/testutils/testtracer/testtracer.go

@@ -93,6 +93,7 @@ type WaitCondition func(*Payloads) bool
 
 // TestTracer is an inspectable tracer useful for tests.
 type TestTracer struct {
+	startError   error
 	payloads     *Payloads
 	roundTripper *mockTransport
 }
@@ -136,8 +137,10 @@ func Start(t testing.TB, opts ...Option) *TestTracer {
 	}, cfg.TracerStartOpts...)
 
 	err := tracer.Start(startOpts...)
-	require.NoError(t, err)
-
+	if cfg.RequireNoError {
+		require.NoError(t, err)
+	}
+	tt.startError = err
 	return tt
 }
 
@@ -146,6 +149,7 @@ type config struct {
 	AgentInfoResponse AgentInfo
 	RequestDelay      time.Duration
 	MockResponse      MockResponseFunc
+	RequireNoError    bool
 }
 
 func defaultConfig() *config {
@@ -154,6 +158,7 @@ func defaultConfig() *config {
 		AgentInfoResponse: AgentInfo{},
 		RequestDelay:      0,
 		MockResponse:      nil,
+		RequireNoError:    true,
 	}
 }
 
@@ -190,6 +195,14 @@ func WithMockResponses(mr MockResponseFunc) Option {
 	}
 }
 
+// WithRequireNoTracerStartError allows to customize the behavior for the Start function. By default, it calls require.NoError
+// on the error returned by tracer.Start, but that can be changed by using this option with false as argument.
+func WithRequireNoTracerStartError(requireNoErr bool) Option {
+	return func(cfg *config) {
+		cfg.RequireNoError = requireNoErr
+	}
+}
+
 // collectPayloads runs in a goroutine and collects payloads from the channel
 func (tt *TestTracer) collectPayloads(payloadChan <-chan any) {
 	for payload := range payloadChan {
@@ -206,6 +219,11 @@ func (tt *TestTracer) collectPayloads(payloadChan <-chan any) {
 	}
 }
 
+// StartError returns the error from tracer.Start.
+func (tt *TestTracer) StartError() error {
+	return tt.startError
+}
+
 // Stop stops the tracer. It should be called after the test finishes.
 func (tt *TestTracer) Stop() {
 	tt.roundTripper.Stop()
@@ -328,31 +346,25 @@ func (rt *mockTransport) handleRequest(r *http.Request) *http.Response {
 		return rt.emptyResponse(r)
 	}
 
-	var (
-		resp            *http.Response
-		respOverwritten = false
-	)
+	var resp *http.Response
+
 	if rt.mockResponse != nil {
 		resp = rt.mockResponse(r)
-		respOverwritten = true
-	}
-	if resp == nil {
-		resp = rt.emptyResponse(r)
+		if resp != nil {
+			return resp
+		}
 	}
+	resp = rt.emptyResponse(r)
 
 	switch r.URL.Path {
 	case "/v0.4/traces":
 		rt.handleTraces(r)
 	case "/info":
-		if !respOverwritten {
-			resp = rt.handleInfo(r)
-		}
+		resp = rt.handleInfo(r)
 	case "/evp_proxy/v2/api/v2/llmobs", "/api/v2/llmobs":
 		rt.handleLLMObsSpanEvents(r)
 	case "/evp_proxy/v2/api/intake/llm-obs/v2/eval-metric", "/api/intake/llm-obs/v2/eval-metric":
 		rt.handleLLMObsEvalMetrics(r)
-	case "/v0.7/config", "/telemetry/proxy/api/v2/apmtelemetry":
-		// known cases, no need to log these
 	default:
 		logWarn := true
 		for _, p := range noLogPaths {

internal/llmobs/llmobs.go

@@ -36,6 +36,7 @@ var (
 	errLLMObsNotEnabled        = errors.New("LLMObs is not enabled. Ensure the tracer has been started with the option tracer.WithLLMObsEnabled(true) or set DD_LLMOBS_ENABLED=true")
 	errAgentlessRequiresAPIKey = errors.New("LLMOBs agentless mode requires a valid API key - set the DD_API_KEY env variable to configure one")
 	errMLAppRequired           = errors.New("ML App is required for sending LLM Observability data")
+	errAgentModeNotSupported   = errors.New("DD_LLMOBS_AGENTLESS_ENABLED has been configured to false but the agent is not available or does not support LLMObs")
 )
 
 const (
@@ -139,15 +140,22 @@ type LLMObs struct {
 }
 
 func newLLMObs(cfg *config.Config, tracer Tracer) (*LLMObs, error) {
+	agentSupportsLLMObs := cfg.AgentFeatures.EVPProxyV2
+	if !agentSupportsLLMObs {
+		log.Debug("llmobs: agent not available or does not support llmobs")
+	}
 	if cfg.AgentlessEnabled != nil {
+		if !*cfg.AgentlessEnabled && !agentSupportsLLMObs {
+			return nil, errAgentModeNotSupported
+		}
 		cfg.ResolvedAgentlessEnabled = *cfg.AgentlessEnabled
 	} else {
 		// if agentlessEnabled is not set and evp_proxy is supported in the agent, default to use the agent
-		cfg.ResolvedAgentlessEnabled = !cfg.AgentFeatures.EVPProxyV2
+		cfg.ResolvedAgentlessEnabled = !agentSupportsLLMObs
 		if cfg.ResolvedAgentlessEnabled {
-			log.Debug("llmobs: DD_LLMOBS_AGENTLESS_ENABLED not set, defaulting to true since agent mode is supported")
+			log.Debug("llmobs: DD_LLMOBS_AGENTLESS_ENABLED not set, defaulting to agentless mode")
 		} else {
-			log.Debug("llmobs: DD_LLMOBS_AGENTLESS_ENABLED not set, defaulting to false since agent mode is not supported")
+			log.Debug("llmobs: DD_LLMOBS_AGENTLESS_ENABLED not set, defaulting to agent mode")
 		}
 	}
 
@@ -531,6 +539,11 @@ func (l *LLMObs) llmobsSpanEvent(span *Span) *transport.LLMObsSpanEvent {
 	tags["ddtrace.version"] = version.Tag
 	tags["language"] = "go"
 
+	sessionID := span.propagatedSessionID()
+	if sessionID != "" {
+		tags["session_id"] = sessionID
+	}
+
 	errTag := "0"
 	if span.error != nil {
 		errTag = "1"
@@ -556,7 +569,7 @@ func (l *LLMObs) llmobsSpanEvent(span *Span) *transport.LLMObsSpanEvent {
 		SpanID:           spanID,
 		TraceID:          span.llmTraceID,
 		ParentID:         parentID,
-		SessionID:        span.propagatedSessionID(),
+		SessionID:        sessionID,
 		Tags:             tagsSlice,
 		Name:             span.name,
 		StartNS:          span.startTime.UnixNano(),
@@ -711,7 +724,6 @@ func (l *LLMObs) SubmitEvaluation(cfg EvaluationConfig) error {
 	if mlApp == "" {
 		mlApp = l.Config.MLApp
 	}
-
 	timestampMS := cfg.TimestampMS
 	if timestampMS == 0 {
 		timestampMS = time.Now().UnixMilli()
@@ -731,12 +743,20 @@ func (l *LLMObs) SubmitEvaluation(cfg EvaluationConfig) error {
 		}
 	}
 
+	tags := make([]string, 0, len(cfg.Tags)+1)
+	for _, tag := range cfg.Tags {
+		if !strings.HasPrefix(tag, "ddtrace.version:") {
+			tags = append(tags, tag)
+		}
+	}
+	tags = append(tags, fmt.Sprintf("ddtrace.version:%s", version.Tag))
+
 	metric := &transport.LLMObsMetric{
 		JoinOn:      joinOn,
 		Label:       cfg.Label,
 		MLApp:       mlApp,
 		TimestampMS: timestampMS,
-		Tags:        cfg.Tags,
+		Tags:        tags,
 	}
 
 	if cfg.CategoricalValue != nil {