feat(telemetry)!: default to experimental GenAI semconv, with ADK_TELEMETRY_SCHEMA_VERSION_OPT_IN=otel_semconv_1_36 rollback - #1635
Conversation
46a49d4 to
01f76b0
Compare
krisztianfekete
left a comment
There was a problem hiding this comment.
Hi @RKest again 👋
We are also improving our OTel support as per kagent-dev/kagent#2907 over at kagent, and stumbled upon this PR. Since I am already deep into checking latest upstream genai semconv changes, I Ieft some comments/questions on the PR in case they are helpful. I see it's still a draft, but might help still as there are lots of changes.
| if useLegacySchema() || isWorkflowAgent(root) { | ||
| return run(ctx) | ||
| } |
There was a problem hiding this comment.
As per semconv invoke_workflow "SHOULD NOT be reported for standalone agent invocations", and its ADK example is Runner.run(...) "with multi-agent or graph workflow".
Could this span only be opened when the root has sub-agents, so a single LLM agent root stays invoke_agent?
| if ctx.Value(workflowScopeKey{}) != nil && !useLegacySchema() { | ||
| attrs = append(attrs, genAIWorkflowNested.Bool(true)) | ||
| } |
There was a problem hiding this comment.
gen_ai.workflow.nested isn't in the GenAI registry, and the spec says invoke_workflow SHOULD NOT be reported at all when it is an internal detail such as agent-as-tool. Wdyt about not opening the span in that case, instead of marking it? Or else propose the attribute upstream in semantic-conventions-genai together with adk-python, rather than defining it in the gen_ai.* namespace?
| // The event is emitted whatever the capture mode, so usage and finish reasons | ||
| // reach logs; messages are added only when content capture on events is on. | ||
| // No-op under the legacy schema, which logs through [logRequest] and | ||
| // [logResponse] instead. | ||
| func logInferenceOperationDetails(ctx context.Context, params GenerateContentParams, result generateContentResult) { | ||
| if useLegacySchema() { | ||
| return | ||
| } |
There was a problem hiding this comment.
I think this is opt-in? Can it be emitted only under EVENT_ONLY or SPAN_AND_EVENT?
| // startGenerateContentSpan starts a new semconv generate_content span. | ||
| func startGenerateContentSpan(ctx context.Context, params GenerateContentParams) (context.Context, trace.Span) { | ||
| modelName := params.ModelName | ||
| attrs := []attribute.KeyValue{ |
There was a problem hiding this comment.
gen_ai.provider.name is Required on inference spans, but here it is only added to the event. Can it go on this span too? And can it come from an optional interface that any model.LLM can implement (for example ProviderName() string)? Otherwise non-Google models (OpenAI, Anthropic, Bedrock and others, as used by downstream projects like kagent) can never report it.
| if sys, ok := GenAISystemAttr(params.Backend); ok { | ||
| attrs = append(attrs, genAIProviderName.String(sys.Value.AsString())) | ||
| } |
There was a problem hiding this comment.
This comes from the same GenAISystemAttr mapping, which only knows the Gemini and Vertex backends. For any other model.LLM the event has no gen_ai.provider.name, which the spec lists as Required on this event. Could this use the same provider source suggested for the span?
| func recordErrorAndStatus(span trace.Span, err error) { | ||
| if err == nil { | ||
| return | ||
| } | ||
| span.RecordError(err) | ||
| span.SetStatus(codes.Error, err.Error()) | ||
| } |
There was a problem hiding this comment.
error.type is Conditionally Required on every GenAI span and on the inference event, but nothing sets it. Can this helper add it, using a bounded value such as the API or HTTP status code, the Go error type, or _OTHER? Generally, error rate queries and span-derived metrics rely on this.
| ) | ||
|
|
||
| const ( | ||
| systemName = "gcp.vertex.agent" |
There was a problem hiding this comment.
The default now emits names that don't exist in 1.36, but the schema URL stays at v1.36.0; the same applies to the logger too. Since this PR is already the breaking change, can the schema URL follow the selected schema (so 1.36 only for legacy)?
01f76b0 to
6cfc47a
Compare
…EMETRY_SCHEMA_VERSION_OPT_IN=otel_semconv_1_36 rollback ADK now emits the experimental OpenTelemetry GenAI semantic conventions by default, following adk-python's schema v2: one gen_ai.client.inference.operation.details event per model call, gen_ai.tool.call.arguments/result on execute_tool gated on span content capture, and an entrypoint invoke_workflow span. Message content is recorded as structured attribute values on spans as well as events, where adk-python records JSON strings on spans. ADK_TELEMETRY_SCHEMA_VERSION_OPT_IN=otel_semconv_1_36 restores the previous format, and otel_semconv_1_44 names the default. Values are case-insensitive, and adk-python's 1 and 2 are accepted for them. No otel_semconv_1_36 golden changes in this commit. A model call and a runner invocation are traced only through InstrumentGenerateContent and InstrumentInvocation, so the generate_content span and its log records cannot drift apart. Closes google#1634
6cfc47a to
0174c33
Compare
Please ensure you have read the contribution guide before creating a pull request.
Linked issue
Problem:
ADK Go emits three telemetry formats side by side:
generate_content;gcp.vertex.agent.tool_call_args/tool_responseattributes, which record tool arguments and results on everyexecute_toolspan whatever the content-capture setting.The OpenTelemetry GenAI instrumentations now default to the experimental conventions, and adk-python is migrating the same way behind
ADK_TELEMETRY_SCHEMA_VERSION_OPT_IN(_schema_version.py#L72).Solution:
The experimental GenAI semantic conventions (as of semantic conventions v1.44.0) become the default.
ADK_TELEMETRY_SCHEMA_VERSION_OPT_IN=otel_semconv_1_36is a full rollback to today's format, andotel_semconv_1_44names the default. Values are case-insensitive, and adk-python's1and2are accepted for them.otel_semconv_1_44)otel_semconv_1_36(legacy)gen_ai.client.inference.operation.detailsper call. Finish reasons, usage andgen_ai.provider.nameare always present; messages only withEVENT_ONLY/SPAN_AND_EVENT(ortrue).gen_ai.system.message,gen_ai.user.message,gen_ai.choiceSPAN_ONLY/SPAN_AND_EVENT)attribute.SliceValue/MapValue), as semconv asks when the SDK supports themexecute_toolpayloadsgen_ai.tool.call.arguments/gen_ai.tool.call.result(the result only when the call succeeded), only withSPAN_ONLY/SPAN_AND_EVENTgcp.vertex.agent.tool_call_args/tool_response, alwaysexecute_tool (merged)tool_call_args: "N/A"plus the merged eventinvoke_workflow {root}above a non-workflow root agent, as adk-python'srecord_invocationdoesinvoke_agent {root}invoke_workflow(e.g. agent-as-tool)gen_ai.workflow.nested: true, as innode_tracing.py#L223Structure:
internal/telemetry/instrumentation.gois the only way to trace a model call or a runner invocation.generateContentwraps its model call inInstrumentGenerateContent, which owns thegenerate_contentspan, the legacy log events and the new event, so the span and its logs cannot drift apart.runner.Runwraps its body inInstrumentInvocation. The pieces they compose are now unexported.resolve_schema_versiondoes. There is no package-level state.telemetry.googleapis.com, the endpoint ADK exports spans to: it rejects the whole export request when one span attribute is over 64 KiB, and it measures a structured value by its encoding (a 90 KiBSliceValuemade of 30 KiB strings was rejected). Events are not capped by ADK; the log SDK'sOTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMITapplies to each string inside them.Other changes:
attributes, which is where the bundled UI reads the new event from. The debug server rewrites message parts the UI's schema rejects (reasoning,uri, blob content, non-object tool payloads) into shapes it accepts. Without that, one thought part blanks the whole Traces panel.otel_semconv_1_44goldens and none of theotel_semconv_1_36ones. The other 8 are workflow failures that end before any model call.Deliberate divergences from adk-python (also commented in code):
OTEL_SEMCONV_STABILITY_OPT_IN. Go goes straight to the end state of that migration plan, with one env var.execute_tooltogen_ai.tool.call.*(step 3 of its plan). Go does it now, because the legacy attributes capture content by default._experimental_semconv.py#L724), because its OpenTelemetry API has no structured span attributes. otel-go has had them since v1.44 (SLICE) and v1.45 (MAP).Behavior change
Breaking change for telemetry consumers. After upgrading:
gen_ai.client.inference.operation.details.execute_toolno longer records tool arguments or results unless span content capture is on, and then undergen_ai.tool.call.*.invoke_workflowparent span.Escape hatch:
ADK_TELEMETRY_SCHEMA_VERSION_OPT_IN=otel_semconv_1_36(or1) restores the previous format exactly. It is documented in thetelemetrypackage doc and supported until at least March 2027. No Go API changes.Testing Plan
Unit Tests:
go mod tidy -diff,go build,go test -race -shuffle=on,golangci-lint runv2.3.1) is clean in both modules.Legacy is unchanged: no
otel_semconv_1_36golden changes in this PR; they are the ones #1640 records againstmain.With your source change reverted and your tests kept, which test fails?
I mutated each new guard and branch on its own (30 mutations in this revision). All but one turn the suite red. The survivor is the
span.IsRecording()check before converting tool arguments: it only skips work on a span the sampler dropped, and changes no output. Most are caught byTestTelemetrySchema/<scenario>/<id>. The rest have focused tests:TestInstrumentInvocation_FirstErrorAndEarlyStopTestInstrumentGenerateContent_EarlyStopTestInferenceEventDescribesTheRequestTheSpanDoes: a model appending toreq.Contentsmid-callTestStartExecuteToolSpan_ArgumentsAfterTheSamplingDecisionTestOversizedAttributeIsDropped: both schemasTestEventContentAttributesTestUseLegacySchemaTestTraceMergedToolCallsResultTestExecuteTool_OmitsWhatIsUnknownTestInvokeWorkflowNestedTestLogInferenceOperationDetails_ProviderNameTestTruthyValueDoesNotPutContentOnSpansTestInferenceDetailsPartsMatchWebUITestUnrepresentableLogAttributesAreDroppedManual End-to-End (E2E) Tests:
One real turn against
gemini-2.5-flashon Vertex AI (llmagent plus one function tool), exported throughtelemetry.New(WithOtelToCloud(true))to Cloud Trace and read back through the Cloud Trace API:SPAN_AND_EVENT: the tree isinvoke_workflow→invoke_agent→generate_content/execute_tool/generate_content.gen_ai.input.messages/output.messagesareSLICEvalues andgen_ai.tool.call.arguments/resultareMAPvalues.ADK_TELEMETRY_SCHEMA_VERSION_OPT_IN=1(the alias): today's tree, with JSON-string content.telemetry.googleapis.comanswer400 'Span.Attributes[1].value' is too large; at most 64.0K is allowed, rejecting the whole request. This is why the span guard stays in v2.Checklist
Additional context
Follow-ups, not in this PR:
gen_ai.tool.definitionson the event (feat(telemetry): record tool definitions on model requests #1454 / Tool definitions are not recorded on the request path (gen_ai.tool.definitions) #789).gen_ai.agent.name,gen_ai.conversation.idanduser.idon the event and ongenerate_content, which adk-python records.gen_ai.invoke_workflow.durationmetric.run_stepandadk_experimental_*span renames, once adk-python lands them.