feat(log-enrichment): add custom session ID attribute for enriched spans#181
Conversation
| } | ||
|
|
||
| # agentevals custom attributes (repository-specific, outside OTel semconv) | ||
| AGENTEVALS_SESSION_ID = "agentevals.session.id" |
There was a problem hiding this comment.
While it's not great, let's follow the existing conventions in this codebase and go with agentevals.session_id. We can clean it up to follow the OTel pattern in a follow-up PR.
There was a problem hiding this comment.
I agree, sounds like a great plan
I have renamed all references of agentevals.session.id to agentevals.session_id
| return input_messages, output_messages | ||
|
|
||
|
|
||
| def _append_session_id(attrs: list[dict], session_id: str | None) -> None: |
There was a problem hiding this comment.
One q: what reads agentevals.session.id downstream? I only see it written, never read. There might be benefits for OTLP export consumers, just want to confirm it's intentional and not dead now.
There was a problem hiding this comment.
It is write-only inside runtime code. I found no downstream internal reader in the repo beyond being appended during enrichment. So this looks intentional as exported metadata for external OTLP consumers rather than an in-repo dependency today.
|
|
||
| agent_name = _get_injected_attr(result[0], "gen_ai.agent.name", parse_json=False) | ||
| assert agent_name == "my-session" | ||
| session_attr = _get_injected_attr(result[0], "agentevals.session.id", parse_json=False) |
There was a problem hiding this comment.
Could we add a test for the actual bug this fixes? i.e. span comes in with a real gen_ai.agent.name, enrich with a session_id, assert the real name survives AND session id lands in the new attr.
There was a problem hiding this comment.
added test_agent_name_preserved_with_session_id
| attrs=[{"key": "gen_ai.agent.name", "value": {"stringValue": "roll_die_agent"}}], | ||
| ) | ||
| logs = [_make_log("gen_ai.user.message", {"content": "hi"}, span_id="s1")] | ||
| result = enrich_spans_with_logs([span], logs, session_id="sess-123") |
There was a problem hiding this comment.
Not sure this catches the regression. Let's do it via flatten_otlp_attributes(result[0]["attributes"])["gen_ai.agent.name"] (or assert there's exactly one gen_ai.agent.name attr). The agentevals.session_id assertion is fine!
2 participants
Summary
This PR stops storing session IDs in the OpenTelemetry semantic convention attribute
gen_ai.agent.nameand instead records them in a repository-specific custom attribute,agentevals.session.id.Previously, the log enrichment pipeline wrote the session ID into
gen_ai.agent.name, overwriting legitimate agent names from SDK and OTLP telemetry. This caused downstream metadata extraction to incorrectly report the session ID as the agent name.Changes
agentevals.session.id.agentevals.session.id.gen_ai.agent.namefor genuine agent names supplied by telemetry.