-
Notifications
You must be signed in to change notification settings - Fork 46
fix(agent): Add gen_ai.agent.name span attribute #737
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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. WalkthroughAdds a new span attribute Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Traceloop as Traceloop SDK
participant Ctx as Tracing Context
participant SpanProc as Span Processor
participant OpenAI as OpenAI API
App->>Traceloop: withAgent("plan_trip.agent"){...}
Traceloop->>Ctx: set AGENT_NAME_KEY="plan_trip.agent"
Traceloop->>SpanProc: start agent span
Note right of SpanProc #ffeead: onSpanStart reads AGENT_NAME_KEY\nsets gen_ai.agent.name attribute
SpanProc-->Traceloop: agent span started
App->>Traceloop: withLLMCall(...) -> start LLM span
Traceloop->>SpanProc: start llm span (uses active context)
Note right of SpanProc #d0f0c0: reads AGENT_NAME_KEY from context\nsets gen_ai.agent.name on LLM span
Traceloop->>OpenAI: POST /v1/chat/completions
OpenAI-->>Traceloop: 200 OK
Traceloop-->>App: result
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
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (2)packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
packages/traceloop-sdk/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (4)📚 Learning: 2025-08-24T22:08:07.023ZApplied to files:
📚 Learning: 2025-08-24T22:08:07.023ZApplied to files:
📚 Learning: 2025-08-24T22:08:07.023ZApplied to files:
📚 Learning: 2025-08-24T22:08:07.023ZApplied to files:
🧬 Code graph analysis (1)packages/traceloop-sdk/src/lib/tracing/decorators.ts (2)
🔇 Additional comments (3)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to acdc019 in 1 minute and 43 seconds. Click for details.
- Reviewed
144lines of code in6files - Skipped
0files when reviewing. - Skipped posting
6draft 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/ai-semantic-conventions/src/SemanticAttributes.ts:30
- Draft comment:
LLM_AGENT_NAME has been added appropriately to the semantic attributes. This aligns with the naming conventions for agent spans. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/sample-app/src/sample_vercel_ai_tools.ts:116
- Draft comment:
planTrip now uses withAgent instead of withWorkflow. This change correctly reflects that this function is acting as an agent. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/traceloop-sdk/src/lib/tracing/decorators.ts:104
- Draft comment:
Consider replacing the literal string 'gen_ai.agent.name' with the constant SpanAttributes.LLM_AGENT_NAME to ensure consistency. - 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% The suggestion to use a constant makes sense from a code quality perspective to ensure consistency and avoid typos. However, I don't see clear evidence that SpanAttributes.LLM_AGENT_NAME actually exists or that it contains the same value. Without being able to verify the constant exists and has the correct value, this suggestion could introduce bugs. I could be wrong about the constant not existing - it may be a well-known standard that I'm not aware of. The suggestion to use constants is generally good practice. While using constants is good practice, we need to be certain the constant exists and has the correct value before suggesting its use. The risk of introducing bugs outweighs the benefit here. Delete this comment since we don't have strong evidence that SpanAttributes.LLM_AGENT_NAME exists or contains the correct value.
4. packages/traceloop-sdk/src/lib/tracing/manual.ts:176
- Draft comment:
The agent name is correctly captured from context and set in both withVectorDBCall and withLLMCall. The implementation looks consistent. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. packages/traceloop-sdk/src/lib/tracing/tracing.ts:7
- Draft comment:
AGENT_NAME_KEY is defined correctly with createContextKey and matches the established naming convention. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. packages/traceloop-sdk/src/lib/tracing/decorators.ts:5
- Draft comment:
Typo notice: The identifier 'ASSOCATION_PROPERTIES_KEY' appears to have a misspelling. Consider changing it to 'ASSOCIATION_PROPERTIES_KEY' if that is the intended spelling. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_RySnYvdbphc4K5KY
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (6)
packages/ai-semantic-conventions/src/SemanticAttributes.ts(1 hunks)packages/sample-app/src/sample_vercel_ai_tools.ts(1 hunks)packages/traceloop-sdk/src/lib/tracing/decorators.ts(3 hunks)packages/traceloop-sdk/src/lib/tracing/manual.ts(3 hunks)packages/traceloop-sdk/src/lib/tracing/span-processor.ts(2 hunks)packages/traceloop-sdk/src/lib/tracing/tracing.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import AI/LLM semantic attribute constants from @traceloop/ai-semantic-conventions rather than hardcoding strings
Files:
packages/traceloop-sdk/src/lib/tracing/tracing.tspackages/traceloop-sdk/src/lib/tracing/manual.tspackages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/traceloop-sdk/src/lib/tracing/decorators.ts
packages/traceloop-sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/traceloop-sdk/**/*.{ts,tsx}: Use the provided decorators (@workflow, @task, @agent) for workflow/task/agent spans instead of re-implementing them
For manual LLM operations, use trace.withLLMSpan from @traceloop/node-server-sdk
Files:
packages/traceloop-sdk/src/lib/tracing/tracing.tspackages/traceloop-sdk/src/lib/tracing/manual.tspackages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/traceloop-sdk/src/lib/tracing/decorators.ts
packages/ai-semantic-conventions/src/SemanticAttributes.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Define all AI/LLM span attribute constants in packages/ai-semantic-conventions/src/SemanticAttributes.ts
Files:
packages/ai-semantic-conventions/src/SemanticAttributes.ts
🧠 Learnings (5)
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx} : Import AI/LLM semantic attribute constants from traceloop/ai-semantic-conventions rather than hardcoding strings
Applied to files:
packages/traceloop-sdk/src/lib/tracing/tracing.tspackages/ai-semantic-conventions/src/SemanticAttributes.tspackages/traceloop-sdk/src/lib/tracing/manual.tspackages/sample-app/src/sample_vercel_ai_tools.tspackages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/traceloop-sdk/src/lib/tracing/decorators.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/ai-semantic-conventions/src/SemanticAttributes.ts : Define all AI/LLM span attribute constants in packages/ai-semantic-conventions/src/SemanticAttributes.ts
Applied to files:
packages/ai-semantic-conventions/src/SemanticAttributes.tspackages/traceloop-sdk/src/lib/tracing/span-processor.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentations must create spans with appropriate AI/LLM semantic attributes for calls they wrap
Applied to files:
packages/ai-semantic-conventions/src/SemanticAttributes.tspackages/traceloop-sdk/src/lib/tracing/manual.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : For manual LLM operations, use trace.withLLMSpan from traceloop/node-server-sdk
Applied to files:
packages/traceloop-sdk/src/lib/tracing/manual.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : Use the provided decorators (workflow, task, agent) for workflow/task/agent spans instead of re-implementing them
Applied to files:
packages/traceloop-sdk/src/lib/tracing/manual.tspackages/sample-app/src/sample_vercel_ai_tools.tspackages/traceloop-sdk/src/lib/tracing/span-processor.tspackages/traceloop-sdk/src/lib/tracing/decorators.ts
🧬 Code graph analysis (3)
packages/traceloop-sdk/src/lib/tracing/manual.ts (2)
packages/traceloop-sdk/src/lib/tracing/tracing.ts (2)
AGENT_NAME_KEY(7-7)getTracer(12-14)packages/ai-semantic-conventions/src/SemanticAttributes.ts (1)
SpanAttributes(17-62)
packages/traceloop-sdk/src/lib/tracing/span-processor.ts (2)
packages/traceloop-sdk/src/lib/tracing/tracing.ts (1)
AGENT_NAME_KEY(7-7)packages/ai-semantic-conventions/src/SemanticAttributes.ts (1)
SpanAttributes(17-62)
packages/traceloop-sdk/src/lib/tracing/decorators.ts (1)
packages/traceloop-sdk/src/lib/tracing/tracing.ts (1)
AGENT_NAME_KEY(7-7)
🪛 GitHub Actions: CI
packages/traceloop-sdk/src/lib/tracing/span-processor.ts
[error] 1-1: Prettier formatting check failed. The command 'pnpm prettier --check .' exited with code 1. Run 'pnpm prettier --write .' to fix code style issues in this file.
🔇 Additional comments (10)
packages/traceloop-sdk/src/lib/tracing/tracing.ts (1)
7-7: LGTM - Agent context key added successfully.The addition of
AGENT_NAME_KEYfollows the existing pattern and properly extends the public context API for agent tracing support.packages/ai-semantic-conventions/src/SemanticAttributes.ts (2)
30-30: LGTM - Semantic attribute follows convention.The
LLM_AGENT_NAMEattribute with value"gen_ai.agent.name"properly follows the GenAI semantic conventions and aligns with the agent tracing infrastructure being introduced.
30-30: ✅ Verified - Follows OpenTelemetry GenAI semantic conventions.The
gen_ai.agent.nameattribute is defined in the official OpenTelemetry GenAI semantic conventions as "Human-readable name of the GenAI agent provided by the application" with examples like "Math Tutor; Fiction Writer". The implementation correctly follows the standard.packages/traceloop-sdk/src/lib/tracing/decorators.ts (2)
4-4: LGTM - Import follows established pattern.The import of
AGENT_NAME_KEYcorrectly follows the established pattern for context keys in this file.
53-56: LGTM - Agent context setup is correct.The logic properly sets the agent name in the context for
AGENTtype entities, following the same pattern used for workflow names.packages/sample-app/src/sample_vercel_ai_tools.ts (1)
117-117: LGTM - Switch to agent decorator is appropriate.The change from
withWorkflowtowithAgentaligns with the new agent tracing capabilities and maintains the same function signature and behavior.packages/traceloop-sdk/src/lib/tracing/span-processor.ts (1)
14-14: LGTM - Import follows established pattern.The import of
AGENT_NAME_KEYis consistent with the other context key imports.packages/traceloop-sdk/src/lib/tracing/manual.ts (3)
2-2: LGTM - Import follows established pattern.The import of
AGENT_NAME_KEYis consistent with other context key imports in the file.
148-153: LGTM - Agent name propagation implemented correctly.The agent name is properly retrieved from the entity context and set on the span using the correct semantic attribute constant.
171-181: LGTM - Consistent agent name propagation.The changes properly propagate agent name to LLM call spans and use consistent context handling throughout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed a8d41ef in 29 seconds. Click for details.
- Reviewed
30lines 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/traceloop-sdk/src/lib/tracing/decorators.ts:104
- Draft comment:
Good change: using SpanAttributes.LLM_AGENT_NAME instead of a hardcoded string improves consistency. Ensure that the constant is correctly defined and documented in the semantic conventions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment starts with a positive note about the change, which is not necessary. The second part of the comment asks the author to ensure that the constant is correctly defined and documented, which is not allowed according to the rules. The comment does not provide a specific suggestion or point out a specific issue with the code.
2. packages/traceloop-sdk/src/lib/tracing/span-processor.ts:146
- Draft comment:
Removal of debug logging for agentName cleans up the output. Confirm no vital debugging info is lost. - 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 no vital debugging information is lost, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code.
Workflow ID: wflow_0FiIunuwNNa15YFT
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/traceloop-sdk/test/agent_decorator.test.ts (4)
40-56: Gate Polly logging to keep CI noise downSet logging based on env and default to false in CI.
- logging: true, + logging: process.env.POLLY_LOGGING === "true",
71-78: Broaden secret/header scrubbing before persisting recordingsMake the filter case-insensitive and scrub other sensitive headers.
- server.any().on("beforePersist", (_req, recording) => { - recording.request.headers = recording.request.headers.filter( - ({ name }: { name: string }) => name !== "authorization", - ); - }); + server.any().on("beforePersist", (_req, recording) => { + const SENSITIVE = new Set(["authorization", "x-api-key", "openai-organization"]); + recording.request.headers = recording.request.headers.filter( + ({ name }: { name: string }) => !SENSITIVE.has(name.toLowerCase()), + ); + });
101-150: Assert agent span also carries gen_ai.agent.name and parent/child relationStrengthen the test to validate the new attribute on the agent span and the linkage to the chat span.
assert.ok(agentSpan); + assert.strictEqual( + agentSpan.attributes[`${SpanAttributes.LLM_AGENT_NAME}`], + "plan_trip", + ); assert.strictEqual( agentSpan.attributes[`${SpanAttributes.TRACELOOP_WORKFLOW_NAME}`], "plan_trip", ); @@ assert.ok(chatSpan); + // Ensure chat span is a child of the agent span + assert.strictEqual( + chatSpan.parentSpanId, + agentSpan.spanContext.spanId, + );
170-209: Also assert agent name attribute and parent/child for decoration pathMirror the assertions from the withAgent test.
assert.ok(agentSpan); + assert.strictEqual( + agentSpan.attributes[`${SpanAttributes.LLM_AGENT_NAME}`], + "travel_planner", + ); @@ assert.ok(chatSpan); + assert.strictEqual( + chatSpan.parentSpanId, + agentSpan.spanContext.spanId, + );
📜 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 (5)
packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-create-spans-for-agents-using-decoration-syntax_1932039671/recording.har(1 hunks)packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-create-spans-for-agents-using-withAgent-syntax_3895564654/recording.har(1 hunks)packages/traceloop-sdk/src/lib/tracing/decorators.ts(3 hunks)packages/traceloop-sdk/src/lib/tracing/span-processor.ts(2 hunks)packages/traceloop-sdk/test/agent_decorator.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-create-spans-for-agents-using-decoration-syntax_1932039671/recording.har
- packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-create-spans-for-agents-using-withAgent-syntax_3895564654/recording.har
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/traceloop-sdk/src/lib/tracing/span-processor.ts
- packages/traceloop-sdk/src/lib/tracing/decorators.ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import AI/LLM semantic attribute constants from @traceloop/ai-semantic-conventions rather than hardcoding strings
Files:
packages/traceloop-sdk/test/agent_decorator.test.ts
packages/traceloop-sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/traceloop-sdk/**/*.{ts,tsx}: Use the provided decorators (@workflow, @task, @agent) for workflow/task/agent spans instead of re-implementing them
For manual LLM operations, use trace.withLLMSpan from @traceloop/node-server-sdk
Files:
packages/traceloop-sdk/test/agent_decorator.test.ts
🧠 Learnings (1)
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : Use the provided decorators (workflow, task, agent) for workflow/task/agent spans instead of re-implementing them
Applied to files:
packages/traceloop-sdk/test/agent_decorator.test.ts
🧬 Code graph analysis (1)
packages/traceloop-sdk/test/agent_decorator.test.ts (2)
packages/traceloop-sdk/test/test-setup.ts (2)
getSharedExporter(37-39)initializeSharedTraceloop(23-35)packages/ai-semantic-conventions/src/SemanticAttributes.ts (1)
SpanAttributes(17-62)
🪛 GitHub Actions: CI
packages/traceloop-sdk/test/agent_decorator.test.ts
[warning] 1-1: Code style issues found in the above file. Run Prettier with --write to fix.
🔇 Additional comments (7)
packages/traceloop-sdk/test/agent_decorator.test.ts (7)
27-29: Good: semantic attributes imported from the right packageUsing SpanAttributes from @traceloop/ai-semantic-conventions aligns with the guidelines.
33-36: Polly registration looks correctAdapters and FS persister registration are in the right order.
58-69: Init sequence is solidOpenAI is initialized after Polly; shared tracer init avoids cross-suite conflicts.
84-100: Nice: withAgent usage exercises association properties and child LLM spanGood coverage for the new agent context.
216-233: Manual LLM instrumentation path is exercised correctlyGood use of reportRequest/reportResponse and attribute checks; agent name propagation is validated.
Given the guidelines, confirm that using traceloop.withLLMCall here is intentional to test the SDK wrapper (vs. trace.withLLMSpan from @traceloop/node-server-sdk).
1-279: Prettier applied — formatting fixedRan Prettier on packages/traceloop-sdk/test/agent_decorator.test.ts; the file was formatted successfully and no further changes are required.
211-279: Keep parentSpanContext; use ReadableSpan property for spanContextDo not replace completionSpan.parentSpanContext?.spanId with parentSpanId (parentSpanId was removed in recent OTel JS SDK); only change the spanContext accessor to the property form. (newreleases.io)
- assert.strictEqual( - completionSpan.parentSpanContext?.spanId, - agentSpan.spanContext().spanId, - ); + assert.strictEqual( + completionSpan.parentSpanContext?.spanId, + agentSpan.spanContext.spanId, + );Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed a11c838 in 27 seconds. Click for details.
- Reviewed
22lines of code in1files - 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/traceloop-sdk/test/agent_decorator.test.ts:168
- Draft comment:
Formatting change: The agentSpan assignment is now split over multiple lines, which improves readability. Ensure this matches the project's style guidelines. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/test/agent_decorator.test.ts:283
- Draft comment:
Trailing newline added at the end of the file. This adheres to best practices. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_rJLIiV9yuO8rkK09
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 014b1a6 in 30 seconds. Click for details.
- Reviewed
422lines 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/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-create-spans-for-agents-using-decoration-syntax_1932039671/recording.har:1
- Draft comment:
Removed outdated HAR recording for agent span tests. Ensure that updated recordings (with the new gen_ai.agent.name attribute) are in place to prevent test regressions. - 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.
2. packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-create-spans-for-agents-using-withAgent-syntax_3895564654/recording.har:1
- Draft comment:
Removed obsolete HAR recording for agent span tests (withAgent syntax). Verify that new test fixtures correctly capture the updated agent name attribute behavior. - 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_GXeN6vSZrehRfqFH
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
…to nk/gen_ai_agent
galkleinman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| LLM_RESPONSE_MODEL: "gen_ai.response.model", | ||
| LLM_USAGE_PROMPT_TOKENS: "gen_ai.usage.prompt_tokens", | ||
| LLM_USAGE_COMPLETION_TOKENS: "gen_ai.usage.completion_tokens", | ||
| LLM_AGENT_NAME: "gen_ai.agent.name", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://opentelemetry.io/docs/specs/semconv/registry/attributes/gen-ai/#gen-ai-agent-name
is it also incubating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it is, i'll at least stick to their consts naming... LLM_AGENT_NAMe doesn't make a lot of sense imo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@galkleinman Yes it is, I will rename it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed b22a9a6 in 35 seconds. Click for details.
- Reviewed
101lines of code in5files - Skipped
0files when reviewing. - Skipped posting
5draft 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/ai-semantic-conventions/src/SemanticAttributes.ts:31
- Draft comment:
Remove the extra blank line before GEN_AI_AGENT_NAME for cleaner formatting. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/src/lib/tracing/decorators.ts:105
- Draft comment:
Updated agent name attribute correctly to GEN_AI_AGENT_NAME; ensure related docs reflect this change. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/traceloop-sdk/src/lib/tracing/manual.ts:149
- Draft comment:
Consistently update agent name attribute to GEN_AI_AGENT_NAME in manual instrumentation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. packages/traceloop-sdk/src/lib/tracing/span-processor.ts:153
- Draft comment:
Using GEN_AI_AGENT_NAME for setting agent name in span processor is correct. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. packages/traceloop-sdk/test/agent_decorator.test.ts:129
- Draft comment:
Test assertions updated to use GEN_AI_AGENT_NAME are correct. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_eIRxnUSKn5x90CnD
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/traceloop-sdk/src/lib/tracing/manual.ts (1)
143-153: Fix startActiveSpan options and ensure span ends on rejectionSecond arg to startActiveSpan must be SpanOptions; currently passing attributes directly. Also end the span on promise rejection.
Apply this diff:
return getTracer().startActiveSpan( `${vendor}.${type}`, - { [SpanAttributes.LLM_REQUEST_TYPE]: type }, + { attributes: { [SpanAttributes.LLM_REQUEST_TYPE]: type } }, entityContext, (span: Span) => { // Set agent name if there's an active agent context const agentName = entityContext.getValue(AGENT_NAME_KEY); if (agentName) { span.setAttribute(SpanAttributes.GEN_AI_AGENT_NAME, agentName as string); } const res = fn.apply(thisArg, [{ span: new VectorSpan(span) }]); if (res instanceof Promise) { - return res.then((resolvedRes) => { - span.end(); - return resolvedRes; - }); + return res.finally(() => { + span.end(); + }); }
📜 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 (5)
packages/ai-semantic-conventions/src/SemanticAttributes.ts(2 hunks)packages/traceloop-sdk/src/lib/tracing/decorators.ts(3 hunks)packages/traceloop-sdk/src/lib/tracing/manual.ts(3 hunks)packages/traceloop-sdk/src/lib/tracing/span-processor.ts(2 hunks)packages/traceloop-sdk/test/agent_decorator.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ai-semantic-conventions/src/SemanticAttributes.ts
- packages/traceloop-sdk/src/lib/tracing/decorators.ts
- packages/traceloop-sdk/test/agent_decorator.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import AI/LLM semantic attribute constants from @traceloop/ai-semantic-conventions rather than hardcoding strings
Files:
packages/traceloop-sdk/src/lib/tracing/manual.tspackages/traceloop-sdk/src/lib/tracing/span-processor.ts
packages/traceloop-sdk/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/traceloop-sdk/**/*.{ts,tsx}: Use the provided decorators (@workflow, @task, @agent) for workflow/task/agent spans instead of re-implementing them
For manual LLM operations, use trace.withLLMSpan from @traceloop/node-server-sdk
Files:
packages/traceloop-sdk/src/lib/tracing/manual.tspackages/traceloop-sdk/src/lib/tracing/span-processor.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentations must create spans with appropriate AI/LLM semantic attributes for calls they wrap
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : For manual LLM operations, use trace.withLLMSpan from traceloop/node-server-sdk
Applied to files:
packages/traceloop-sdk/src/lib/tracing/manual.tspackages/traceloop-sdk/src/lib/tracing/span-processor.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx} : Import AI/LLM semantic attribute constants from traceloop/ai-semantic-conventions rather than hardcoding strings
Applied to files:
packages/traceloop-sdk/src/lib/tracing/manual.tspackages/traceloop-sdk/src/lib/tracing/span-processor.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/**/*.{ts,tsx} : Use the provided decorators (workflow, task, agent) for workflow/task/agent spans instead of re-implementing them
Applied to files:
packages/traceloop-sdk/src/lib/tracing/manual.tspackages/traceloop-sdk/src/lib/tracing/span-processor.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
PR: traceloop/openllmetry-js#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentations must create spans with appropriate AI/LLM semantic attributes for calls they wrap
Applied to files:
packages/traceloop-sdk/src/lib/tracing/manual.tspackages/traceloop-sdk/src/lib/tracing/span-processor.ts
🧬 Code graph analysis (2)
packages/traceloop-sdk/src/lib/tracing/manual.ts (2)
packages/traceloop-sdk/src/lib/tracing/tracing.ts (2)
AGENT_NAME_KEY(7-7)getTracer(12-14)packages/ai-semantic-conventions/src/SemanticAttributes.ts (1)
SpanAttributes(18-64)
packages/traceloop-sdk/src/lib/tracing/span-processor.ts (2)
packages/traceloop-sdk/src/lib/tracing/tracing.ts (1)
AGENT_NAME_KEY(7-7)packages/ai-semantic-conventions/src/SemanticAttributes.ts (1)
SpanAttributes(18-64)
🔇 Additional comments (3)
packages/traceloop-sdk/src/lib/tracing/span-processor.ts (2)
15-16: LGTM: context key importImporting AGENT_NAME_KEY from ./tracing is correct and aligns with our context-handling design.
152-155: LGTM: set gen_ai.agent.name on span startCorrectly reads AGENT_NAME_KEY from context and uses SpanAttributes.GEN_AI_AGENT_NAME (no hardcoded string). Matches the semantic conventions.
packages/traceloop-sdk/src/lib/tracing/manual.ts (1)
2-2: LGTM: AGENT_NAME_KEY importUsing the shared context key avoids duplication and hardcoded strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 732c0b2 in 31 seconds. Click for details.
- Reviewed
53lines of code in3files - 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/ai-semantic-conventions/src/SemanticAttributes.ts:14
- Draft comment:
Minor formatting change: redundant blank line removed before defining SpanAttributes. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/src/lib/tracing/decorators.ts:102
- Draft comment:
Multiline formatting for setAttribute call improves readability; update looks fine. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/traceloop-sdk/src/lib/tracing/manual.ts:148
- Draft comment:
Consistent multiline formatting for setAttribute in withVectorDBCall; no issues. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_J0d6uF5t376mcbZJ
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Important
Adds
GEN_AI_AGENT_NAMEspan attribute for agent context in tracing, with tests and HAR recordings for validation.GEN_AI_AGENT_NAMEattribute toSpanAttributesinSemanticAttributes.ts.withEntity()indecorators.tsto setGEN_AI_AGENT_NAMEfor agent spans.withVectorDBCall()andwithLLMCall()inmanual.tsto setGEN_AI_AGENT_NAMEif agent context is active.onSpanStart()inspan-processor.tsto setGEN_AI_AGENT_NAMEfor active agent context.agent_decorator.test.tsto test agent spans, decorator usage, and manual LLM instrumentation.recordings/Test-Agent-Decorator_2969879889/.withWorkflowtowithAgentinsample_vercel_ai_tools.ts.This description was created by
for 732c0b2. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Tests