fix(weave): align integration otel keys and retain instrumentation scope - #7645
Open
gtarpenning wants to merge 4 commits into
Open
fix(weave): align integration otel keys and retain instrumentation scope#7645gtarpenning wants to merge 4 commits into
gtarpenning wants to merge 4 commits into
Conversation
Four independent bugs on the flattened OTel attribution path. Python `as_otel_attributes()` emitted `integration.*` while the node SDK emits `weave.integration.*`, so a consumer keyed on either spelling silently missed the other SDK's spans. Python now emits the `weave.integration.*` form `AGENTS.md` already documents as canonical. The literals are restated locally because the import-linter forbids sharing them, following the `google_adk/_semconv.py` precedent, and a parity test reads the node literal so the two cannot drift again. `ScopeSpans.from_proto` set `scope` on the container but built its spans via `Span.from_proto(s, resource)`, so the instrumentation scope was parsed and then discarded for every span. It now threads onto `Span` and into `otel_dump`, the only route by which a call retains it. The two docstrings described behavior that does not exist: the trace server never rebuilt a nested `attributes["integration"]` dict, and `gen_ai_aliases` already holds non-`gen_ai.*` keys via `ERROR_TYPE`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
HiveMind Sessions1 session · 6m · $2.24
View all sessions in HiveMind → Run |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
… paths ScopeSpans.from_proto was the only site threading scope onto Span, and it is reachable only from TracesData.from_proto, which has no non-test callers. All three real ingest loops walked scope_spans themselves and dropped the scope, so no ingested span carried its instrumentation-library identity. Adds iter_proto_spans so the walk pairs each span with its enclosing scope in one place, and replaces the flat scope_name/scope_version "" sentinels with a Scope dataclass mirroring Resource, so an omitted scope persists as null instead of being indistinguishable from an empty one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gen_ai_aliases already held non-gen_ai keys (ERROR_TYPE takes error.type), so the name carries the contract instead of a docstring carve-out that goes stale on the next non-gen_ai alias. Drops comments that narrate where a change came from rather than describe current behavior, and removes the as_otel_attributes claim about the trace server not re-nesting these keys: unflatten_key_values nests every dotted key, so the claim was wrong as well as out of place. Also removes the parity test's pytest.skip, which would silently drop the cross-SDK drift guard in any checkout without the node SDK. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gtarpenning
marked this pull request as ready for review
July 28, 2026 22:03
jtschoonhoven
approved these changes
Jul 30, 2026
jtschoonhoven
left a comment
jtschoonhoven
left a comment
Contributor
There was a problem hiding this comment.
You might want a stamp from the sdk team on this, but LGTM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
as_otel_attributes()emittedintegration.*while the node SDK emitsweave.integration.*, so a consumer keyed on either spelling missed the other SDK's spans. Python now emits theweave.integration.*formAGENTS.md:322documents as canonical.ScopeSpans.from_protowas the only site threading it, and it is reachable only fromTracesData.from_proto, which has no non-test callers; all three real ingest loops dropped it.iter_proto_spansnow pairs each span with its enclosing scope in one place, so the three loops cannot disagree.Scope | Nonemirroringresourcerather than"", so "sender omitted the scope" and "sender sent an empty scope" stay distinguishable in the persistedotel_dump. This addsattributes.otel_span.scopeas a permanent user-visible key.gen_ai_aliases->wire_aliases: it already held non-gen_aikeys (ERROR_TYPEtakeserror.type), so the name carries the contract instead of a docstring carve-out.No server-side ladder accepting both spellings: no weave server code consumes either key, so converging the producers now means a consumer needs one key rather than two forever. Note the old spelling is already persisted as custom attrs in real projects, so those will list both until the old spans age out, and a saved filter on
integration.namestops matching new spans. #7644 should rebase and drop its dual-key rung.Testing
The scope assertion runs through the real
otel_exportpath, notTracesData.from_proto; reverting the threading inclickhouse_trace_server_batched.pyfails it withassert None == {...}. A parity test reads the node literal out ofsdks/node/src/genai/semconv.tsso the SDKs cannot drift again; the import-linter forbids sharing the constant, so the literals are restated per thegoogle_adk/_semconv.pyprecedent.