fix(openai-agents): propagate gen_ai.agent.name through an agent flow + set workflow name to fast mcp #3388
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughCaptures FastMCP server name during FastMCP.init and stores it on the instrumentor; attaches that name as Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App
participant FastMCP as FastMCP (wrapped)
participant Instr as FastMCPInstrumentor
participant OTel as Tracer/OTel
User->>App: Start server
App->>FastMCP: FastMCP.__init__(name=...)
FastMCP-->>Instr: __init__ (wrapped by _fastmcp_init_wrapper)
Instr->>Instr: capture server_name -> "<name>.mcp"
App->>FastMCP: invoke tool
FastMCP->>Instr: tool wrapper executes
Instr->>OTel: startSpan("mcp.server", attrs: traceloop.workflow.name = server_name)
Instr->>OTel: startSpan("<tool>.tool", attrs: traceloop.workflow.name = server_name)
Instr-->>FastMCP: run tool logic
Instr->>OTel: end tool span
Instr->>OTel: end mcp.server span
sequenceDiagram
autonumber
participant App
participant Agent as OpenAI Agent
participant Hooks as OTel Hooks
participant SDK as traceloop SDK
participant OTel as OTel Span Processor
App->>Agent: run(query)
Agent-->>Hooks: on_span_start(AgentSpanData)
Hooks->>SDK: set_agent_name(agent_name)
note right of SDK: store "agent_name" in tracer context
Agent->>OTel: start child spans
OTel->>OTel: on_start reads context
OTel-->>Agent: set GEN_AI_AGENT_NAME attribute on spans
rect rgba(230,240,255,0.5)
note over Agent,Hooks: Handoff handling
Agent-->>Hooks: on_span_start(HandoffSpanData)
Hooks->>Hooks: add GEN_AI_AGENT_NAME to handoff attributes
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
packages/opentelemetry-instrumentation-openai-agents/mocked_recording_test.py
Outdated
Show resolved
Hide resolved
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 8e48bd9 in 2 minutes and 29 seconds. Click for details.
- Reviewed
876lines of code in11files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py:515
- Draft comment:
Review the logic for clearing the agent context in on_span_end. Resettingself._agent_namewhen it matches may risk clearing context in nested agent flows. Consider a more robust context management approach. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is speculative - it suggests there "may" be an issue with nested flows but doesn't demonstrate a concrete problem. The code already has logic specifically designed to handle complex scenarios (as noted in the comments). The comment doesn't provide evidence that the current approach is actually problematic. The change appears to be a thoughtful improvement over previous code. I could be wrong if there are specific nested agent flow scenarios where this logic fails, but the comment doesn't identify any. Without concrete examples of problematic scenarios, and given that the code already attempts to handle complex cases, this comment appears to be speculative rather than identifying a clear issue. The comment should be deleted as it is speculative and doesn't provide clear evidence of an actual problem with the implemented approach.
2. packages/traceloop-sdk/traceloop/sdk/decorators/base.py:148
- Draft comment:
Ensure that the usage of set_agent_name in _setup_span (lines ~145–150) is consistent with the revised logging approach; consider using a logger there as well if appropriate. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While using a logger instead of print is generally good practice, this seems like a debug print that was added temporarily. The comment is not incorrect, but it's a minor suggestion that may not be worth the reviewer's attention. The print statement isn't causing any real issues and could be intentional for development. The print statement could pollute logs in production and make it harder to properly monitor the application. Proper logging is important for production systems. While logging best practices are important, this seems like an intentionally added debug print that may be temporary. The comment creates unnecessary back-and-forth for a minor issue. Delete the comment as it's a minor suggestion that doesn't warrant interrupting the PR review process. The print statement isn't causing significant issues.
Workflow ID: wflow_6l6LiwDeW6f9ODpi
You can customize 
...elemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 3699ef5 in 2 minutes and 21 seconds. Click for details.
- Reviewed
43lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py:166
- Draft comment:
Removed call to _set_agent_name_attribute for tool spans. Confirm that tool spans correctly inherit the agent name via the parent context or other propagation mechanism. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that the tool spans correctly inherit the agent name via the parent context or another propagation mechanism. This is asking for confirmation of behavior, which violates the rule against asking the PR author to confirm their intention or ensure behavior is intended.
2. packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py:192
- Draft comment:
Removed call to _set_agent_name_attribute in response span creation. Verify that response spans still capture the intended agent name via their parent (agent) span. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that a specific behavior is still correct after a change. This falls under the rule of not asking the author to ensure behavior is intended or to double-check things. Therefore, this comment should be removed.
3. packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py:213
- Draft comment:
Removed call to _set_agent_name_attribute in chat span creation. Ensure that chat spans obtain the agent name appropriately from the active agent context. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment suggests there might be an issue with agent name propagation, but looking at the code: 1. Agent names are now set via set_agent_name() when agent spans start 2. This propagates through the context system to child spans 3. The removal of _set_agent_name_attribute appears intentional and consistent 4. There's no evidence this is causing any problems I could be missing some subtle issue with the OpenTelemetry context propagation system. Maybe there's a reason the old direct attribute setting was needed. The code shows a clear pattern of using context propagation instead of direct attribute setting. Without evidence of an actual problem, we should trust the intentional architectural change. This comment should be deleted. It questions an intentional architectural change without evidence of any actual problem.
4. packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py:545
- Draft comment:
Deleted the helper _set_agent_name_attribute. Confirm that removing this helper doesn’t break the propagation of the agent name in spans that may rely on explicit attribute assignment. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_OEXZAtrVsZJSLnQd
You can customize
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py (1)
66-99: Use instance-scoped server name in spans; avoid shared state.Resolve the server name from the ToolManager/related server, then set attributes from that value.
- async def traced_method(wrapped, _instance, args, kwargs): + async def traced_method(wrapped, _instance, args, kwargs): if not self._tracer: return await wrapped(*args, **kwargs) @@ - entity_name = tool_key if tool_key else "unknown_tool" + entity_name = tool_key if tool_key else "unknown_tool" + + # Resolve server name from nearby instances; fall back to weak map or legacy field + server_name = None + for attr in ("server", "_server", "app", "owner"): + srv = getattr(_instance, attr, None) + name = getattr(srv, "_traceloop_server_name", getattr(srv, "name", None)) if srv else None + if isinstance(name, str): + server_name = f"{name}.mcp" if not name.endswith(".mcp") else name + break + if server_name is None: + server_name = getattr(_instance, "_traceloop_server_name", None) + if server_name is None: + server_name = getattr(self, "_server_names", {}).get(_instance, None) if hasattr(self, "_server_names") else None + if server_name is None: + server_name = getattr(self, "_server_name", None) # last-resort # Create parent server.mcp span with self._tracer.start_as_current_span("mcp.server") as mcp_span: mcp_span.set_attribute(SpanAttributes.TRACELOOP_SPAN_KIND, "server") mcp_span.set_attribute(SpanAttributes.TRACELOOP_ENTITY_NAME, "mcp.server") - if self._server_name: - mcp_span.set_attribute(SpanAttributes.TRACELOOP_WORKFLOW_NAME, self._server_name) + if server_name: + mcp_span.set_attribute(SpanAttributes.TRACELOOP_WORKFLOW_NAME, server_name) @@ with self._tracer.start_as_current_span(span_name) as tool_span: tool_span.set_attribute(SpanAttributes.TRACELOOP_SPAN_KIND, TraceloopSpanKindValues.TOOL.value) tool_span.set_attribute(SpanAttributes.TRACELOOP_ENTITY_NAME, entity_name) - if self._server_name: - tool_span.set_attribute(SpanAttributes.TRACELOOP_WORKFLOW_NAME, self._server_name) + if server_name: + tool_span.set_attribute(SpanAttributes.TRACELOOP_WORKFLOW_NAME, server_name)
🧹 Nitpick comments (12)
packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py (1)
268-268: No-op whitespace change; consider avoiding churn.This blank line adds no functional value. Suggest dropping to reduce diff noise.
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
264-267: Remove print from library API; use logger or drop.Printing from SDK code is noisy and not thread-safe for logs. Use logging.debug or remove.
-def set_agent_name(agent_name: str) -> None: - print(f"Setting agent name in tracing: {agent_name}") - attach(set_value("agent_name", agent_name)) +def set_agent_name(agent_name: str) -> None: + logging.getLogger(__name__).debug("Setting agent name in tracing: %s", agent_name) + attach(set_value("agent_name", agent_name))packages/traceloop-sdk/traceloop/sdk/decorators/base.py (1)
149-152: Avoid stdout prints in decorators; rely on context only.The print adds noise in user apps. set_agent_name already updates context; logging at debug is enough.
- if tlp_span_kind == TraceloopSpanKindValues.AGENT: - print(f"Setting agent name: {entity_name}") - set_agent_name(entity_name) + if tlp_span_kind == TraceloopSpanKindValues.AGENT: + set_agent_name(entity_name)packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py (2)
85-89: Consider standardizing span.kind values.TRACELOOP_SPAN_KIND uses a controlled vocabulary; "server" is non-standard relative to TraceloopSpanKindValues. If feasible, use WORKFLOW for the top-level FastMCP span.
- mcp_span.set_attribute(SpanAttributes.TRACELOOP_SPAN_KIND, "server") + mcp_span.set_attribute(SpanAttributes.TRACELOOP_SPAN_KIND, TraceloopSpanKindValues.WORKFLOW.value)
42-47: Uninstrument is a no-op. Document limitation or add best-effort cleanup.At minimum, document in README that FastMCP hooks can’t be cleanly unwrapped due to wrapt limitations.
I can draft a short note for the docs/changelog if helpful.
packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_response_spans_basic.yaml (1)
67-69: Model mismatch vs test may cause VCR miss.Cassette uses
model: "gpt-4.1"while the corresponding test constructs an agent with"gpt-4o". Depending on VCR matchers, this can break playback.Either re-record with the intended model or align the test input to
"gpt-4.1".Also applies to: 110-128
packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.py (3)
20-21: Align test model with cassette to avoid VCR mismatches.Cassette shows
gpt-4.1; test usesgpt-4o. Align or re-record.Apply this diff if you want to align to cassette:
- model="gpt-4o" + model="gpt-4.1"
49-51: Use semantic constant for system attribute.Prefer
SpanAttributes.LLM_SYSTEMover raw key.- assert response_span.attributes["gen_ai.system"] == "openai" + assert response_span.attributes[SpanAttributes.LLM_SYSTEM] == "openai"
32-36: Optional: strengthen assertions.You can also assert span kinds to catch regressions.
- assert len(agent_spans) == 1, f"Expected 1 agent span, got {len(agent_spans)}" + assert len(agent_spans) == 1, f"Expected 1 agent span, got {len(agent_spans)}" + assert agent_spans[0].attributes[SpanAttributes.TRACELOOP_SPAN_KIND] == TraceloopSpanKindValues.AGENT.valueAlso applies to: 66-68, 82-84
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (3)
550-554: Use context to read agent name; drop unused arg.
current_agent_spanis unused, and relying on_agent_namewould still be racy. Read from OTel context instead.@@ - def _set_agent_name_attribute(self, attributes: Dict[str, Any], current_agent_span=None): - """Set the agent name attribute using current agent name.""" - if self._agent_name: - attributes[GEN_AI_AGENT_NAME] = self._agent_name + def _set_agent_name_attribute(self, attributes: Dict[str, Any]): + """Set the agent name attribute from OTel context.""" + name = context.get_value("agent_name") + if name: + attributes[GEN_AI_AGENT_NAME] = nameUpdate call sites:
- self._set_agent_name_attribute(tool_attributes, current_agent_span) + self._set_agent_name_attribute(tool_attributes) @@ - self._set_agent_name_attribute(response_attributes, current_agent_span) + self._set_agent_name_attribute(response_attributes) @@ - self._set_agent_name_attribute(response_attributes, current_agent_span) + self._set_agent_name_attribute(response_attributes)This also resolves the Ruff ARG002 warning.
Also applies to: 171-172, 197-198, 218-219
26-28: Type hints: keys aren’tstr.
self._otel_spans: Dict[str, Any]is misleading; keys are agent span objects. UseDict[Any, Any]or precise types if available.- self._otel_spans: Dict[str, Any] = {} # agents span -> otel span + self._otel_spans: Dict[Any, Any] = {} # agents span -> otel span
186-205: Parenting response/tool spans to the agent span may be unreliable.
_find_current_agent_span()usesget_current_span()and checks.name.endswith('.agent'), which often won’t hold once a child is current. Prefer tracking the agent span per trace (e.g., dict keyed bytrace_id) and using that as parent.Would you like a follow-up patch to introduce
self._agent_span_by_trace_idand wire it into parent resolution?Also applies to: 207-226
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py(3 hunks)packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp.py(1 hunks)packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_attributes.py(2 hunks)packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py(10 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_response_spans_basic.yaml(1 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_tool_spans.yaml(1 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.py(1 hunks)packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/decorators/base.py(2 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/__init__.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.pypackages/opentelemetry-instrumentation-mcp/tests/test_fastmcp.pypackages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.pypackages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.pypackages/traceloop-sdk/traceloop/sdk/tracing/__init__.pypackages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_attributes.pypackages/traceloop-sdk/traceloop/sdk/decorators/base.pypackages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.pypackages/traceloop-sdk/traceloop/sdk/tracing/tracing.py
**/cassettes/**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Never commit secrets or PII in VCR cassettes; scrub sensitive data
Files:
packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_tool_spans.yamlpackages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_response_spans_basic.yaml
🧬 Code graph analysis (5)
packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.py (3)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (2)
SpanAttributes(64-261)TraceloopSpanKindValues(301-306)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py (1)
function_tool_agent(85-98)
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py (2)
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/utils.py (1)
dont_throw(12-40)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
TraceloopSpanKindValues(301-306)
packages/traceloop-sdk/traceloop/sdk/tracing/__init__.py (1)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (2)
set_workflow_name(260-261)set_agent_name(264-266)
packages/traceloop-sdk/traceloop/sdk/decorators/base.py (2)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (2)
set_workflow_name(260-261)set_agent_name(264-266)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
TraceloopSpanKindValues(301-306)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (2)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/utils.py (1)
dont_throw(46-72)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
set_agent_name(264-266)
🪛 Flake8 (7.2.0)
packages/traceloop-sdk/traceloop/sdk/tracing/__init__.py
[error] 2-2: 'traceloop.sdk.tracing.tracing.set_workflow_name' imported but unused
(F401)
[error] 2-2: 'traceloop.sdk.tracing.tracing.set_agent_name' imported but unused
(F401)
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
550-550: Unused method argument: current_agent_span
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint
🔇 Additional comments (6)
packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_attributes.py (2)
65-66: LGTM: validates workflow name on tool span.Good coverage to ensure TRACLOOP_WORKFLOW_NAME is propagated.
170-172: LGTM: ensures workflow name persists on error spans.Catches regressions where errors might skip attribute setting.
packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp.py (1)
125-149: LGTM: assertions for server/tool workflow names.Solid checks that mcp.server and server-side tool spans carry the expected workflow name.
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
324-327: LGTM — agent name attached to spans (verified).rg output confirms GEN_AI_AGENT_NAME is set in packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (lines 324–326) and is asserted in packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.py.
packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_response_spans_basic.yaml (1)
49-56: Consider filtering organization/project and request IDs.Headers like
openai-organization,openai-project, andx-request-idaren’t secrets but can be considered sensitive metadata. Prefer filtering them to reduce leakage surface.You can extend VCR filters:
filter_headers=["authorization", "cookie", "openai-project", "openai-organization", "x-request-id"].Also applies to: 151-157, 62-63, 175-175
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (1)
138-145: Normalize span kind for handoff spans.
"handoff"isn’t part ofTraceloopSpanKindValues. UseUNKNOWNand add an explicit operation attribute.
[suggest_minor_issue]- handoff_attributes = { - SpanAttributes.TRACELOOP_SPAN_KIND: "handoff", + handoff_attributes = { + SpanAttributes.TRACELOOP_SPAN_KIND: TraceloopSpanKindValues.UNKNOWN.value, "gen_ai.system": "openai_agents" } + handoff_attributes["gen_ai.operation.name"] = "handoff"
...entelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
Show resolved
Hide resolved
...entelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
Show resolved
Hide resolved
...elemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
Show resolved
Hide resolved
...elemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
Outdated
Show resolved
Hide resolved
...ssettes/test_agent_name_propagation/test_agent_name_propagation_to_response_spans_basic.yaml
Show resolved
Hide resolved
...s/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_tool_spans.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py (3)
350-357: Bug: may break on OpenAI v0 objects
response.choicesassumes attribute access; for v0 we buildresponse_dictand need dict access. Use a version-agnosticchoicesvariable.Apply this diff:
- if should_emit_events(): - if response.choices is not None: - for choice in response.choices: - emit_event(_parse_choice_event(choice)) + if should_emit_events(): + choices = response.choices if is_openai_v1() else response_dict.get("choices") + if choices: + for choice in choices: + emit_event(_parse_choice_event(choice))
470-489: Shadowed loop variable ‘i’ hurts readabilityInner loop reuses
i, which already indexes messages. Rename inner index to avoid confusion.Apply this diff:
- if tool_calls: - for i, tool_call in enumerate(tool_calls): + if tool_calls: + for j, tool_call in enumerate(tool_calls): if is_openai_v1(): tool_call = model_as_dict(tool_call) function = tool_call.get("function") _set_span_attribute( span, - f"{prefix}.tool_calls.{i}.id", + f"{prefix}.tool_calls.{j}.id", tool_call.get("id"), ) _set_span_attribute( span, - f"{prefix}.tool_calls.{i}.name", + f"{prefix}.tool_calls.{j}.name", function.get("name"), ) _set_span_attribute( span, - f"{prefix}.tool_calls.{i}.arguments", + f"{prefix}.tool_calls.{j}.arguments", function.get("arguments"), )
1144-1191: Guard against None when iterating choices in streaming accumulation
for choice in item.get("choices")will raise ifchoicesis None. Cheap defensive fix.Apply this diff:
- for choice in item.get("choices"): + for choice in (item.get("choices") or []):packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py (1)
85-91: Consider using a defined span kind for Traceloop or a FastMCP‑specific attribute.
traceloop.span.kind="server"isn’t part of TraceloopSpanKindValues. Either useunknown, omit it, or add a FastMCP‑specific key (e.g.,fastmcp.span.kind="server"), to avoid confusing downstream processors that expect the enum.packages/traceloop-sdk/traceloop/sdk/tracing/__init__.py (1)
2-2: Fix Flake8 F401 by declaring explicit re‑exports and marking imports as intentional.
__init__.pyis re‑exporting API; silence F401 and add__all__.Apply this diff:
-from traceloop.sdk.tracing.tracing import set_workflow_name, set_agent_name +from traceloop.sdk.tracing.tracing import set_workflow_name, set_agent_name # noqa: F401 +__all__ = ["get_tracer", "set_workflow_name", "set_agent_name"]Also add
# noqa: F401to Line 1 if flagged by your linter.packages/traceloop-sdk/traceloop/sdk/decorators/base.py (1)
149-152: Remove debug print from library code.Use logging if needed; avoid stdout noise in SDKs.
Apply this diff:
- print(f"Setting agent name: {entity_name}") set_agent_name(entity_name)packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (1)
371-373: Make model settings tracking per-trace to avoid misattribution.
self._last_model_settingsis global and can be overwritten by concurrent traces, leading to wrong attributes on agent spans. Key it bytrace_id(store on response end, read on agent end, then delete).I can provide a focused patch if you confirm the desired key (trace_id vs. agents span id).
Also applies to: 484-486, 489-501
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py(3 hunks)packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp.py(1 hunks)packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_attributes.py(2 hunks)packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py(6 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_response_spans_basic.yaml(1 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_tool_spans.yaml(1 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.py(1 hunks)packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/decorators/base.py(2 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/__init__.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_attributes.pypackages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.pypackages/traceloop-sdk/traceloop/sdk/tracing/tracing.pypackages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.pypackages/traceloop-sdk/traceloop/sdk/decorators/base.pypackages/traceloop-sdk/traceloop/sdk/tracing/__init__.pypackages/opentelemetry-instrumentation-mcp/tests/test_fastmcp.pypackages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.pypackages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
**/cassettes/**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Never commit secrets or PII in VCR cassettes; scrub sensitive data
Files:
packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_tool_spans.yamlpackages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_response_spans_basic.yaml
🧬 Code graph analysis (5)
packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.py (3)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (2)
SpanAttributes(64-261)TraceloopSpanKindValues(301-306)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py (1)
function_tool_agent(85-98)
packages/traceloop-sdk/traceloop/sdk/decorators/base.py (2)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (3)
get_tracer(240-241)set_workflow_name(260-261)set_agent_name(264-265)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
TraceloopSpanKindValues(301-306)
packages/traceloop-sdk/traceloop/sdk/tracing/__init__.py (1)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (2)
set_workflow_name(260-261)set_agent_name(264-265)
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py (2)
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/utils.py (1)
dont_throw(12-40)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (1)
TraceloopSpanKindValues(301-306)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (2)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/utils.py (1)
dont_throw(46-72)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
set_agent_name(264-265)
🪛 Flake8 (7.2.0)
packages/traceloop-sdk/traceloop/sdk/tracing/__init__.py
[error] 2-2: 'traceloop.sdk.tracing.tracing.set_workflow_name' imported but unused
(F401)
[error] 2-2: 'traceloop.sdk.tracing.tracing.set_agent_name' imported but unused
(F401)
🪛 Ruff (0.12.2)
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
51-51: Unused function argument: instance
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (13)
packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/chat_wrappers.py (1)
268-268: Formatting-only change — OKNo functional impact; complies with Flake8 (single blank line).
packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py (1)
34-41: Hooking FastMCP.init is the right spot.Registering a post‑import wrapper on
FastMCP.__init__is appropriate for capturing server identity.packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp_attributes.py (2)
65-65: LGTM: workflow name asserted on tool span.This verifies propagation to server‑side tool spans as intended.
170-172: LGTM: workflow name asserted on error spans.Good coverage that attributes persist on error paths.
packages/opentelemetry-instrumentation-mcp/tests/test_fastmcp.py (1)
125-149: LGTM: validates workflow name on mcp.server and tool spans.Filtering for spans that actually carry the workflow attribute neatly distinguishes server‑side vs client‑side spans.
packages/traceloop-sdk/traceloop/sdk/decorators/base.py (2)
23-23: LGTM: import of set_agent_name for AGENT spans.Import is used below in
_setup_span.
142-148: Dual-write is intentional — AGENT spans include both workflow and agent attributes. Tracing sets GEN_AI_AGENT_NAME on span start (packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py:325), openai-agents hooks create AGENT spans with GEN_AI_AGENT_NAME (packages/opentelemetry-instrumentation-openai-agents/.../_hooks.py:91-95), and the decorator calls set_workflow_name for AGENT spans (packages/traceloop-sdk/traceloop/sdk/decorators/base.py:142-148). Tests verify agent-name propagation (packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.py).packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_response_spans_basic.yaml (1)
1-180: VCR cassette appears properly configured for testing.The cassette contains appropriate test interactions showing agent name propagation with no obvious sensitive data exposed. The structure follows VCR best practices with proper HTTP request/response recording for OpenAI API interactions.
packages/opentelemetry-instrumentation-openai-agents/tests/test_agent_name_propagation.py (3)
1-11: LGTM! Proper imports for agent name propagation testing.The imports correctly include the necessary OpenTelemetry semantic conventions (
GEN_AI_AGENT_NAME,GEN_AI_COMPLETION) and Traceloop span kinds for comprehensive testing of agent name propagation.
12-51: Comprehensive test for agent name propagation to response spans.The test properly:
- Creates a simple agent with a clear name
- Executes a query and collects spans
- Verifies exactly one agent span exists with the correct name format
- Confirms at least one response span is created
- Validates that the agent span has the correct
GEN_AI_AGENT_NAMEattribute- Ensures all response spans inherit the agent name through propagation
- Checks that response spans have appropriate LLM-specific attributes (
LLM_REQUEST_TYPE,gen_ai.system)This provides solid coverage for the agent name propagation feature.
53-85: Well-designed test for agent name propagation to tool spans.The test effectively validates:
- Agent span creation with proper naming convention ("WeatherAgent.agent")
- Tool span creation with correct naming ("get_weather.tool")
- Agent name propagation from agent to tool spans via
GEN_AI_AGENT_NAME- Tool-specific attributes including span kind and tool name
- Proper relationship between agent and tool spans
The test uses the
function_tool_agentfixture appropriately and provides comprehensive assertions for the tool span propagation scenario.packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
323-326: GEN_AI_AGENT_NAME is exported by the incubating GenAI semconv — no fallback needed. Verified that opentelemetry.semconv._incubating.attributes.gen_ai_attributes exports GEN_AI_AGENT_NAME (gen_ai.agent.name); keep the current code.packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (1)
115-151: TracerWrapper always installs span processor chaining default_span_processor_on_start
All instantiation paths inTracerWrapper—default, single custom, and multiple custom processors—override each processor’son_startto invokedefault_span_processor_on_start, guaranteeingGEN_AI_AGENT_NAMEis set on all spans.
...entelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
Show resolved
Hide resolved
...entelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
Show resolved
Hide resolved
...entelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py
Show resolved
Hide resolved
...elemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
Show resolved
Hide resolved
...s/tests/cassettes/test_agent_name_propagation/test_agent_name_propagation_to_tool_spans.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 1f311b9 in 2 minutes and 42 seconds. Click for details.
- Reviewed
218lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_openai_agents/test_agent_name_propagation_to_agent_spans.yaml:6
- Draft comment:
Agent span 'name' value ('Analytics Agent') differs from expected naming ('testAgent.agent') used in tests. Ensure consistency between cassette and test agent name propagation. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a test file, so consistency with test expectations is important. However, without seeing the actual test file, I can't verify if "testAgent.agent" is really the expected name. The comment is making assumptions about test expectations without clear evidence. The cassette appears to be recording real API interactions, so changing the recorded data could make the tests less realistic. I might be undervaluing test consistency - if the tests expect a specific agent name, the cassette should match. However, I also can't be certain what the tests actually expect. While test consistency is important, modifying recorded API responses to match assumptions about test expectations could lead to brittle or unrealistic tests. Without seeing the actual test code, we can't verify if this change is correct. Delete the comment. Without seeing the test code that uses this cassette, we don't have enough context to verify if this name change suggestion is correct.
2. packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py:435
- Draft comment:
This test duplicates agent name propagation checks from 'test_agent_spans'. Consider consolidating tests to reduce redundancy. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_i7hUhHmrYkuwxjfE
You can customize
...ai-agents/tests/cassettes/test_openai_agents/test_agent_name_propagation_to_agent_spans.yaml
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py (1)
446-451: Make span lookup more robust (avoid brittle count assertion)Relying on exactly one span by name can be brittle as instrumentation evolves. Prefer next(..., None) with an existence check.
- agent_spans = [s for s in spans if s.name == "testAgent.agent"] - assert len(agent_spans) == 1, f"Expected 1 agent span, got {len(agent_spans)}" - agent_span = agent_spans[0] + agent_span = next((s for s in spans if s.name == "testAgent.agent"), None) + assert agent_span is not None, "Agent span 'testAgent.agent' not found"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py(4 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_openai_agents/test_agent_name_propagation_to_agent_spans.yaml(1 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
🧰 Additional context used
📓 Path-based instructions (2)
**/cassettes/**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Never commit secrets or PII in VCR cassettes; scrub sensitive data
Files:
packages/opentelemetry-instrumentation-openai-agents/tests/cassettes/test_openai_agents/test_agent_name_propagation_to_agent_spans.yaml
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: nina-kollman
PR: traceloop/openllmetry#3388
File: packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py:20-20
Timestamp: 2025-09-18T14:36:24.669Z
Learning: In packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py, the team is aware that the global self._server_name field is unsafe for multi-server or concurrent usage but has consciously decided not to support multiple FastMCP servers currently.
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py (2)
packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py (2)
exporter(27-37)test_agent(72-81)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Lint
- GitHub Check: Test Packages (3.12)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.10)
🔇 Additional comments (1)
packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py (1)
436-456: Approve — agent-name propagation assertion is correct and test is unique.rg shows a single definition at packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py:436; no duplicates found.
...ai-agents/tests/cassettes/test_openai_agents/test_agent_name_propagation_to_agent_spans.yaml
Show resolved
Hide resolved
...ai-agents/tests/cassettes/test_openai_agents/test_agent_name_propagation_to_agent_spans.yaml
Show resolved
Hide resolved
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b9bfba5 in 1 minute and 5 seconds. Click for details.
- Reviewed
9lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py:457
- Draft comment:
Removed an extraneous trailing newline. Ensure this aligns with the project's newline style guidelines. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_FP04oTyv64GJ4kM5
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py (3)
448-456: Use fixture-driven name and improve diagnostics; avoid hardcoded "testAgent".Leverage
test_agent.nameto reduce brittleness and include span names on failure.- # Find the agent span - agent_spans = [s for s in spans if s.name == "testAgent.agent"] - assert len(agent_spans) == 1, f"Expected 1 agent span, got {len(agent_spans)}" + # Find the agent span + expected_agent_name = test_agent.name + expected_agent_span_name = f"{expected_agent_name}.agent" + agent_spans = [s for s in spans if s.name == expected_agent_span_name] + assert len(agent_spans) == 1, ( + f"Expected 1 agent span named '{expected_agent_span_name}', " + f"got {len(agent_spans)}: {[s.name for s in spans]}" + ) agent_span = agent_spans[0] @@ - assert agent_span.attributes[GEN_AI_AGENT_NAME] == "testAgent", ( - f"Expected agent name 'testAgent', got '{agent_span.attributes[GEN_AI_AGENT_NAME]}'" + assert agent_span.attributes[GEN_AI_AGENT_NAME] == expected_agent_name, ( + f"Expected agent name '{expected_agent_name}', got '{agent_span.attributes[GEN_AI_AGENT_NAME]}'" )
452-456: Also assert propagation to child spans (response/tool) to match PR objective.The PR claims propagation to child spans; add minimal checks so we catch regressions.
# Verify the agent span has the agent name attribute set via context propagation assert GEN_AI_AGENT_NAME in agent_span.attributes, f"Agent span should have {GEN_AI_AGENT_NAME} attribute" - assert agent_span.attributes[GEN_AI_AGENT_NAME] == "testAgent", ( - f"Expected agent name 'testAgent', got '{agent_span.attributes[GEN_AI_AGENT_NAME]}'" + assert agent_span.attributes[GEN_AI_AGENT_NAME] == expected_agent_name, ( + f"Expected agent name '{expected_agent_name}', got '{agent_span.attributes[GEN_AI_AGENT_NAME]}'" ) + + # Child spans should carry the agent name as well + response_spans = [s for s in spans if s.name == "openai.response"] + assert len(response_spans) >= 1, f"Expected at least 1 openai.response span, got {len(response_spans)}" + for rs in response_spans: + assert rs.attributes.get(GEN_AI_AGENT_NAME) == expected_agent_name, ( + f"Child span '{rs.name}' missing {GEN_AI_AGENT_NAME}='{expected_agent_name}'" + ) + + # If any tool spans exist, they should also carry the agent name + for ts in [s for s in spans if s.name.endswith('.tool')]: + assert ts.attributes.get(GEN_AI_AGENT_NAME) == expected_agent_name, ( + f"Child tool span '{ts.name}' missing {GEN_AI_AGENT_NAME}='{expected_agent_name}'" + )
438-438: Nit: move the GEN_AI_AGENT_NAME import to module top for consistency with other semconv imports.Keeps imports centralized and avoids duplicate imports during test collection.
-from opentelemetry.semconv._incubating.attributes.gen_ai_attributes import GEN_AI_AGENT_NAMEAnd at the top with the existing GEN_AI_COMPLETION import:
from opentelemetry.semconv._incubating.attributes.gen_ai_attributes import ( - GEN_AI_COMPLETION, + GEN_AI_COMPLETION, + GEN_AI_AGENT_NAME, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: nina-kollman
PR: traceloop/openllmetry#3388
File: packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py:20-20
Timestamp: 2025-09-18T14:36:24.669Z
Learning: In packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py, the team is aware that the global self._server_name field is unsafe for multi-server or concurrent usage but has consciously decided not to support multiple FastMCP servers currently.
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-openai-agents/tests/test_openai_agents.py (2)
packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py (2)
exporter(27-37)test_agent(72-81)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Lint
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.10)
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed fc3312b in 2 minutes and 11 seconds. Click for details.
- Reviewed
33lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_bZcUtG5Q2WdaoQOJ
You can customize
...elemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
Outdated
Show resolved
Hide resolved
...elemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed a104572 in 56 seconds. Click for details.
- Reviewed
39lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py:14
- Draft comment:
Good simplification: removing the test mode check and debug prints via os.getenv helps clean up the code. The try/except fallback for set_agent_name is clearer. Just confirm that always attempting to import (and using a no-op on ImportError) meets your test versus production needs. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_v590T4qUh0XPwheZ
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (2)
15-27: Critical: NameError in test-mode branch + noisy prints; switch to logger and fix gating.
print(f"ASAF - Setting agent name: {set_agent_name}")referencesset_agent_namebefore it’s defined at module scope, causing a NameError whenOPENAI_AGENTS_TEST_MODEis set. Also, direct prints in instrumentation are noisy; uselogger.debug. This aligns with prior feedback.Apply:
@@ -# Import set_agent_name only if not in test mode -if not os.getenv("OPENAI_AGENTS_TEST_MODE"): - try: - from traceloop.sdk.tracing import set_agent_name - print(f"NOMI - Setting agent name: {set_agent_name}") - except ImportError: - # If traceloop is not available, create a no-op function - def set_agent_name(name): pass -else: - # In test mode, create a no-op function - print(f"ASAF - Setting agent name: {set_agent_name}") - def set_agent_name(name): pass +# Configure set_agent_name with test-mode and import fallback; avoid prints +TEST_MODE = bool(os.getenv("OPENAI_AGENTS_TEST_MODE")) +if TEST_MODE: + def set_agent_name(name: str) -> None: # no-op in tests + return + logger.debug("OPENAI_AGENTS_TEST_MODE=true: set_agent_name is a no-op") +else: + try: + from traceloop.sdk.tracing import set_agent_name # type: ignore + logger.debug("set_agent_name wired to traceloop.sdk.tracing") + except Exception: + def set_agent_name(name: str) -> None: + return + logger.debug("traceloop-sdk not available; using no-op set_agent_name")Additionally, introduce a logger at the top of the module:
@@ -from typing import Dict, Any +from typing import Dict, Any +import logging @@ -from collections import OrderedDict +from collections import OrderedDict +logger = logging.getLogger(__name__)
90-92: Scope agent_name in OTEL context; attach/detach to prevent cross-trace leaks.Calling
set_agent_name(agent_name)attaches to context without any corresponding detach, so the agent name can bleed into unrelated spans/threads. Store the attach token per agent-span and detach it inon_span_end.Apply:
@@ def on_span_start(self, span): - # Set agent name in OpenTelemetry context for propagation to child spans - set_agent_name(agent_name) + # Set agent name in OpenTelemetry context for propagation to child spans + token = context.attach(context.set_value("agent_name", agent_name)) + # Track token per Agents SDK span to detach later + self._agent_ctx_tokens[span] = tokenAnd add a token map in the constructor:
@@ def __init__(self, tracer: Tracer): self._otel_spans: Dict[str, Any] = {} # agents span -> otel span self._span_contexts: Dict[str, Any] = {} # agents span -> context token + self._agent_ctx_tokens: Dict[Any, Any] = {} # agents span -> OTEL context token for agent_nameDetach in
on_span_end:@@ def on_span_end(self, span): if span in self._span_contexts: context.detach(self._span_contexts[span]) del self._span_contexts[span] + # Detach agent_name context, if set + token = getattr(self, "_agent_ctx_tokens", {}).pop(span, None) + if token is not None: + context.detach(token)
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (1)
151-151: Handoff span: confirm semantics for GEN_AI_AGENT_NAME and consider adding target agent.You set
GEN_AI_AGENT_NAMEto thefrom_agent. If consumers expect this key to reflect the “current/active” agent on that span, that’s fine; otherwise, consider adding a second attribute for the target (e.g.,gen_ai.handoff.to_agent.name) or also taggingto_agentwithGEN_AI_AGENT_NAMEon a separate “arrival” span. Also,TRACELOOP_SPAN_KINDcurrently uses the string "handoff"; consider aligning with a defined enum value or documenting this custom kind.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: nina-kollman
PR: traceloop/openllmetry#3388
File: packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py:20-20
Timestamp: 2025-09-18T14:36:24.669Z
Learning: In packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py, the team is aware that the global self._server_name field is unsafe for multi-server or concurrent usage but has consciously decided not to support multiple FastMCP servers currently.
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (3)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (2)
SpanAttributes(64-261)TraceloopSpanKindValues(301-306)packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/utils.py (1)
dont_throw(46-72)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
set_agent_name(264-265)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Lint
- GitHub Check: Build Packages (3.11)
🔇 Additional comments (2)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (2)
102-102: Good: use official GEN_AI_AGENT_NAME attribute key.Switching from a string literal to the semconv constant reduces typos and eases future refactors.
11-11: Verify incubating semconv imports match pinned OTEL versions.Ran the check — import failed in the sandbox: "No module named opentelemetry.semconv._incubating.attributes.gen_ai_attributes" (semconv _incubating not present). Affected file: packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py (line 11) — imports GEN_AI_COMPLETION and GEN_AI_AGENT_NAME; confirm the repo’s pinned OpenTelemetry semconv version exposes those symbols at this path or update the import(s)/add a fallback.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 1c7a3aa in 1 minute and 46 seconds. Click for details.
- Reviewed
62lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py:14
- Draft comment:
Directly importing 'set_agent_name' removes the fallback mechanism. Ensure that requiring traceloop.sdk.tracing is acceptable in all deployment contexts. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py:42
- Draft comment:
The fixture name 'mock_set_agent_name' shadows the global MagicMock variable of the same name, risking unexpected recursion. Consider renaming one to avoid this naming conflict. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Looking at the code, this isn't actually a problem - it's a common pattern in pytest to have a fixture return and manage a global mock. The fixture is intentionally named the same as the global mock it manages. The fixture resets and returns the global mock, which is exactly what we want. There's no recursion risk because the fixture is explicitly returning the global mock. Could there be edge cases where having the same name causes issues with pytest's fixture resolution? Could this make the code harder to understand for future maintainers? No, this is a standard pytest pattern. The fixture's purpose is clearly documented in its docstring, and the reset behavior shows it's intentionally managing the global mock. The comment should be deleted. The naming is intentional and follows common pytest patterns for managing global mocks through fixtures.
Workflow ID: wflow_htrg217RZTtrg0uf
You can customize
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py(4 hunks)packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opentelemetry-instrumentation-openai-agents/opentelemetry/instrumentation/openai_agents/_hooks.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: nina-kollman
PR: traceloop/openllmetry#3388
File: packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py:20-20
Timestamp: 2025-09-18T14:36:24.669Z
Learning: In packages/opentelemetry-instrumentation-mcp/opentelemetry/instrumentation/mcp/fastmcp_instrumentation.py, the team is aware that the global self._server_name field is unsafe for multi-server or concurrent usage but has consciously decided not to support multiple FastMCP servers currently.
🧬 Code graph analysis (1)
packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py (1)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
set_agent_name(264-265)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (1)
packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py (1)
27-40: Import-time mock ordering is wrong; use ModuleType for traceloop mocks and import the instrumentor after the mocks.
- Move the OpenAIAgentsInstrumentor import to after the mock block in packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py (lines 27–40).
- Replace MagicMock-based sys.modules entries with types.ModuleType instances and set only tracing.set_agent_name = MagicMock(...); register traceloop, traceloop.sdk, traceloop.sdk.tracing in sys.modules.
Verification: the provided ripgrep returned "No such file or directory" for opentelemetry/instrumentation/openai_agents — cannot confirm whether the instrumentor performs a top-level
from traceloop.sdk.tracing import set_agent_name; re-run the repo search or provide the correct path to verify.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 5370969 in 54 seconds. Click for details.
- Reviewed
50lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py:1
- Draft comment:
Good change: importing 'types' and using types.ModuleType for creating module mocks improves type safety and clarity over using MagicMock for modules. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py:28
- Draft comment:
The new approach using a global 'SET_AGENT_NAME_MOCK' assigned via MagicMock and configured in module hierarchy is clear and effective. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/opentelemetry-instrumentation-openai-agents/tests/conftest.py:47
- Draft comment:
Fixture 'mock_set_agent_name' now correctly resets and returns SET_AGENT_NAME_MOCK, ensuring clean state between tests. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_mtcqJzbxfatxIull
You can customize
nina-kollman
changed the title
2 participants
feat(instrumentation): ...orfix(instrumentation): ....Important
Enhances traceability by propagating agent names through spans and improving workflow name handling in
FastMCPInstrumentorandOpenTelemetryTracingProcessor.FastMCPInstrumentorinfastmcp_instrumentation.pycaptures server name and applies it as workflow name on server and tool spans.OpenTelemetryTracingProcessorin_hooks.pypropagates agent names into spans, preserved across handoffs, and injects into child spans.set_agent_name()function added totracing.pyfor agent identity propagation.test_fastmcp.pyandtest_fastmcp_attributes.pyto verify workflow-name and agent-name propagation.test_agent_name_propagation_to_agent_spans.yamlverifies agent name propagation.conftest.pyforset_agent_name()function.gen_ai.agent.nametoGEN_AI_AGENT_NAMEin_hooks.pyand other files for consistency.This description was created by
for 5370969. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Tests