Conversation
📝 WalkthroughWalkthroughThis PR updates dependencies across the monorepo, introduces pnpm package overrides at the root level, and refactors sample application code to align with AI SDK conventions by replacing Changes
Poem
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 71fb01b in 2 minutes and 47 seconds. Click for details.
- Reviewed
803lines of code in16files - Skipped
2files when reviewing. - Skipped posting
16draft 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. package.json:35
- Draft comment:
Added 'js-yaml' dependency and bumped 'lerna' to v9.0.3 for CVE remediation; verify that these changes remain compatible with existing build scripts and tooling. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify compatibility with existing build scripts and tooling, which is against the rules. It is not making a specific code suggestion or asking for a specific test to be written. Therefore, it should be removed.
2. packages/instrumentation-anthropic/package.json:57
- Draft comment:
Added 'qs' dependency (^6.14.1) to address vulnerabilities; ensure its usage is correct. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is related to a dependency change, specifically the addition of theqsdependency. The comment asks to ensure its usage is correct, which violates the rule against asking the PR author to ensure behavior is intended or correct. Therefore, this comment should be removed.
3. packages/instrumentation-bedrock/package.json:49
- Draft comment:
Updated '@aws-sdk/client-bedrock-runtime' from ^3.848.0 to ^3.969.0; confirm that the new version functions correctly within the instrumentation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to confirm that the new version functions correctly, which violates the rule against asking for confirmation or verification of behavior. It is related to dependency changes, which should not be commented on unless there's a specific issue or suggestion.
4. packages/instrumentation-bedrock/tests/meta.test.ts:211
- Draft comment:
Refactored streaming response handling by accumulating the finalParsedResponse and moving assertions outside the loop. Consider adding an explicit failure if no final chunk is received to catch unexpected behavior. - 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 comment has two parts: 1) It describes what was done (refactoring), which is informative but not actionable. 2) It suggests "Consider adding an explicit failure if no final chunk is received to catch unexpected behavior." This is a suggestion using "Consider", which makes it sound optional rather than required. The rules state not to make speculative comments or suggestions that aren't clearly required. The current code has anif (finalParsedResponse)guard that silently skips assertions if there's no final chunk. Whether this is a bug or intentional behavior isn't clear from the test alone. The comment is suggesting a potential improvement but not identifying a definite issue. This seems like a "nice to have" suggestion rather than a clear code change requirement. The suggestion to add an explicit failure could be valid - if the test expects a final chunk with metrics, it should fail if that doesn't happen. However, the comment uses "Consider" which makes it optional, and it's not clear if the absence of a final chunk is actually a test failure scenario or if it's acceptable. This might be a legitimate test robustness improvement rather than just a speculative comment. While the suggestion could improve test robustness, the comment doesn't demonstrate that the current behavior is wrong or that there's a bug. It's a quality suggestion ("Consider adding...") rather than identifying a clear issue. The rules state comments should be about clear code changes required, not optional improvements. Without evidence that the current behavior is incorrect, this falls into the category of a speculative or optional suggestion. The comment is primarily informative (describing the refactoring) and includes an optional suggestion ("Consider adding...") rather than identifying a required code change. Per the rules, comments should identify clear issues requiring changes, not make optional suggestions. This should be deleted.
5. packages/instrumentation-cohere/package.json:55
- Draft comment:
Added '@smithy/config-resolver' (^4.4.0) and ensured 'qs' is up to date; verify these dependencies do not conflict with AWS SDK modules. - 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 the dependencies do not conflict with AWS SDK modules. This falls under the rule of not asking the author to ensure compatibility or verify behavior, which is not allowed.
6. packages/instrumentation-langchain/package.json:41
- Draft comment:
Bumped versions for '@langchain/core', '@langchain/community', and 'langchain' to newer releases; ensure any breaking changes from these updates are handled. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about dependency version changes, which is not allowed according to the rules. It also asks the author to ensure that breaking changes are handled, which is not specific enough to be useful.
7. packages/instrumentation-llamaindex/package.json:55
- Draft comment:
Updated 'llamaindex' dependency from ^0.11.19 to ^0.12.1; confirm that the new version integrates properly with the instrumentation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to confirm that the new version of a dependency integrates properly, which is against the rules. It does not provide a specific suggestion or point out a specific issue with the code.
8. packages/instrumentation-mcp/package.json:49
- Draft comment:
Updated '@modelcontextprotocol/sdk' from ^1.0.4 to ^1.25.2; review for any breaking API changes in the new version. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about a dependency change, specifically updating a library version. The comment suggests reviewing for breaking changes, which is not allowed according to the rules. The rules specify not to comment on pure dependency changes or ask the author to ensure compatibility.
9. packages/instrumentation-qdrant/package.json:57
- Draft comment:
Updated '@qdrant/js-client-rest' from ^1.15.0 to ^1.16.2; verify that this version works seamlessly with Qdrant instrumentation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify that the new version of a dependency works seamlessly, which is against the rules. It does not provide a specific code suggestion or ask for a specific test to be written. It is also related to dependency changes, which we should not comment on unless we recognize the library and have a specific suggestion.
10. packages/sample-app/package.json:58
- Draft comment:
Multiple dependency upgrades (e.g., '@ai-sdk/openai', '@aws-sdk/client-bedrock-runtime', langchain) have been applied. Verify that the sample application remains fully functional with these new versions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify the functionality of the application after dependency upgrades. This falls under the rule of not asking the author to ensure the behavior is intended or to double-check things. Therefore, this comment should be removed.
11. packages/sample-app/src/sample_mcp.ts:55
- Draft comment:
Using explicit type annotations and casting the server as 'any' in registerTool calls improves type safety but may hide underlying issues; consider upgrading MCP server typings to avoid casting. - 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 comment acknowledges that the changes "improve type safety" which suggests the changes are actually good. Then it says "but may hide underlying issues" which is speculative - there's no evidence of actual underlying issues. Finally, it suggests "consider upgrading MCP server typings" which is a vague suggestion that doesn't provide clear actionable guidance. The author clearly made a deliberate choice to useas anycasting (they even added eslint-disable comments), so this comment is essentially asking them to reconsider their approach without strong evidence that it's wrong. This violates the rule about not making speculative comments and not asking the author to confirm their intention. The comment does acknowledge the improvement in type safety, and suggesting better typings could be considered constructive feedback. Perhaps the author wasn't aware of better alternatives, and this could prompt them to investigate if there's a way to avoid theanycasting. While suggesting better typings could theoretically be helpful, the comment is too vague and speculative. It says "may hide underlying issues" without identifying any actual issues, and "consider upgrading MCP server typings" is not actionable - it's unclear what specific action the author should take. The author clearly made a deliberate choice here (evidenced by the eslint-disable comments), and without strong evidence that this approach is problematic, this comment should be removed. This comment should be deleted because it's speculative ("may hide underlying issues"), not clearly actionable ("consider upgrading" is vague), and appears to question the author's deliberate design choice without providing strong evidence of a problem. The comment even acknowledges the changes improve type safety.
12. packages/sample-app/src/sample_nested_agents.ts:13
- Draft comment:
Tool definitions have been updated to use 'inputSchema' instead of 'parameters' and 'stopWhen: stepCountIs(...)' instead of 'maxSteps'. This aligns with the new API; ensure that the behavior remains consistent with previous implementations. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is purely informative, explaining changes in tool definitions and suggesting to ensure consistent behavior. It doesn't provide a specific code suggestion or ask for a test to be written. It violates the rule against making purely informative comments.
13. packages/sample-app/src/sample_vercel_ai_agent.ts:50
- Draft comment:
Updated tool definitions to use 'inputSchema' and replaced 'maxSteps' with 'stopWhen: stepCountIs(...)', improving consistency with the latest API; the explicit type annotations improve clarity. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, providing a summary of the changes made in the PR. It does not offer any suggestions, ask for tests, or confirm intentions. It violates the rule against making purely informative comments.
14. packages/sample-app/src/sample_vercel_ai_agent_simple.ts:14
- Draft comment:
The simple agent sample now employs explicit type annotations and uses the updated API ('inputSchema' and 'stopWhen'). This enhances type safety and consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, praising the use of explicit type annotations and updated API. It doesn't provide any actionable feedback or suggestions for improvement.
15. packages/sample-app/src/sample_vercel_ai_tools.ts:14
- Draft comment:
Refactored tool definitions to adopt 'inputSchema' and use 'stopWhen: stepCountIs(...)' instead of the deprecated 'parameters' and 'maxSteps'. The changes are consistent and improve clarity. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, praising the refactoring and noting the consistency and clarity improvements. It doesn't provide any actionable feedback or suggestions for improvement.
16. packages/traceloop-sdk/package.json:90
- Draft comment:
Dependencies and peer dependencies in the SDK have been updated across the board. Ensure that these new versions remain compatible with dependent packages and that peer dependency requirements are met. - 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_DR1nW3hEIsK1uN3U
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sample-app/src/sample_mcp.ts (1)
54-68: Investigate SDK type definitions forregisterTool—the cast may indicate a version mismatch.The
(server as any).registerToolcast is unnecessary. The MCP SDK properly definesregisterToolas a generic method that accepts bothZodTypeAnyandRecord<string, ZodTypeAny>forinputSchema, matching the code's usage. The cast likely indicates the installed SDK version has incomplete type exports or a TypeScript configuration issue.Check that
@modelcontextprotocol/sdkversion andtsconfigare aligned with the type definitions. Once resolved, the cast can be removed.The explicit handler type annotation
{ a: number; b: number }is redundant but harmless—the SDK's type inference should infer this correctly.
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 48-62: The pnpm overrides block (pnpm.overrides) contains a
non-existent version and several outdated pins; change "diff": "8.0.3" to
"8.0.2" and update the other override entries to current stable releases (e.g.,
bump zod, glob, cookie, playwright, form-data, tmp as noted) by querying
registry versions with npm view <package> versions before changing; ensure you
only modify the values in the overrides object and keep body-parser and undici
as-is since they intentionally address CVEs.
In `@packages/instrumentation-bedrock/tests/meta.test.ts`:
- Around line 252-274: The test currently skips validating token counts when
finalParsedResponse is falsy; add an explicit assertion that finalParsedResponse
was received (e.g., assert.ok(finalParsedResponse, 'Expected finalParsedResponse
containing amazon-bedrock-invocationMetrics')) and then remove the surrounding
if (finalParsedResponse) block so the subsequent assertions against
ATTR_GEN_AI_USAGE_PROMPT_TOKENS, ATTR_GEN_AI_USAGE_COMPLETION_TOKENS, and
SpanAttributes.LLM_USAGE_TOTAL_TOKENS always run against
finalParsedResponse["amazon-bedrock-invocationMetrics"].
In `@packages/instrumentation-langchain/package.json`:
- Line 41: Update the `@langchain/core` dependency in package.json from "^0.3.80"
to "^0.3.81" (or newer) to include the fix for CVE-2025-68664; after changing
the version, run the package manager install (npm/yarn/pnpm) to update the
lockfile and verify the updated package is present, and run tests or npm audit
to ensure no remaining vulnerabilities.
📜 Review details
Configuration used: defaults
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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
package.jsonpackages/instrumentation-anthropic/package.jsonpackages/instrumentation-bedrock/package.jsonpackages/instrumentation-bedrock/tests/meta.test.tspackages/instrumentation-cohere/package.jsonpackages/instrumentation-langchain/package.jsonpackages/instrumentation-llamaindex/package.jsonpackages/instrumentation-mcp/package.jsonpackages/instrumentation-qdrant/package.jsonpackages/sample-app/package.jsonpackages/sample-app/src/sample_mcp.tspackages/sample-app/src/sample_nested_agents.tspackages/sample-app/src/sample_vercel_ai_agent.tspackages/sample-app/src/sample_vercel_ai_agent_simple.tspackages/sample-app/src/sample_vercel_ai_tools.tspackages/sample-app/tsconfig.jsonpackages/traceloop-sdk/package.json
🧰 Additional context used
📓 Path-based instructions (5)
packages/instrumentation-*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place each provider integration in its own package under packages/instrumentation-[provider]/
Files:
packages/instrumentation-langchain/package.jsonpackages/instrumentation-anthropic/package.jsonpackages/instrumentation-llamaindex/package.jsonpackages/instrumentation-cohere/package.jsonpackages/instrumentation-bedrock/tests/meta.test.tspackages/instrumentation-qdrant/package.jsonpackages/instrumentation-bedrock/package.jsonpackages/instrumentation-mcp/package.json
packages/*/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Use workspace:* for intra-repo package dependencies in package.json
Files:
packages/instrumentation-langchain/package.jsonpackages/instrumentation-anthropic/package.jsonpackages/instrumentation-llamaindex/package.jsonpackages/instrumentation-cohere/package.jsonpackages/traceloop-sdk/package.jsonpackages/sample-app/package.jsonpackages/instrumentation-qdrant/package.jsonpackages/instrumentation-bedrock/package.jsonpackages/instrumentation-mcp/package.json
packages/traceloop-sdk/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
When adding a new instrumentation package, add it to the main SDK dependencies
Files:
packages/traceloop-sdk/package.json
packages/instrumentation-*/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/instrumentation-*/**/*.{ts,tsx}: Instrumentation classes must extend InstrumentationBase and register hooks using InstrumentationModuleDefinition
Instrumentations must create spans with appropriate AI/LLM semantic attributes for calls they wrap
Instrumentations must extract request/response data and token usage from wrapped calls
Instrumentations must capture and record errors appropriately
Do not implement anonymous telemetry collection in instrumentation packages; telemetry is collected only in the SDK
Files:
packages/instrumentation-bedrock/tests/meta.test.ts
packages/{instrumentation-*,traceloop-sdk}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Import AI/LLM semantic attribute constants from
@traceloop/ai-semantic-conventionsrather than hardcoding strings
Files:
packages/instrumentation-bedrock/tests/meta.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 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
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 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/instrumentation-langchain/package.jsonpackages/sample-app/src/sample_mcp.tspackages/instrumentation-anthropic/package.jsonpackages/instrumentation-llamaindex/package.jsonpackages/instrumentation-cohere/package.jsonpackages/sample-app/src/sample_vercel_ai_tools.tspackages/traceloop-sdk/package.jsonpackages/sample-app/package.jsonpackages/instrumentation-bedrock/tests/meta.test.tspackages/sample-app/tsconfig.jsonpackages/sample-app/src/sample_vercel_ai_agent.tspackages/sample-app/src/sample_vercel_ai_agent_simple.tspackages/sample-app/src/sample_nested_agents.tspackages/instrumentation-mcp/package.json
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/package.json : When adding a new instrumentation package, add it to the main SDK dependencies
Applied to files:
packages/instrumentation-langchain/package.jsonpackages/instrumentation-anthropic/package.jsonpackages/instrumentation-llamaindex/package.jsonpackages/instrumentation-cohere/package.jsonpackages/traceloop-sdk/package.jsonpackages/instrumentation-qdrant/package.jsonpackages/instrumentation-bedrock/package.jsonpackages/instrumentation-mcp/package.json
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Use ts-mocha as the TypeScript test runner for package tests
Applied to files:
packages/instrumentation-langchain/package.jsonpackages/instrumentation-anthropic/package.json
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 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/instrumentation-langchain/package.jsonpackages/instrumentation-anthropic/package.jsonpackages/instrumentation-llamaindex/package.jsonpackages/instrumentation-cohere/package.jsonpackages/sample-app/src/sample_vercel_ai_tools.tspackages/traceloop-sdk/package.jsonpackages/sample-app/src/sample_vercel_ai_agent.tspackages/sample-app/src/sample_vercel_ai_agent_simple.tspackages/sample-app/src/sample_nested_agents.ts
📚 Learning: 2025-12-30T22:05:07.422Z
Learnt from: galzilber
Repo: traceloop/openllmetry-js PR: 853
File: packages/sample-app/src/sample_security_evaluators.ts:37-46
Timestamp: 2025-12-30T22:05:07.422Z
Learning: In packages/sample-app/src/**/*.ts, allow demonstration code to omit defensive error handling and bounds checking for API responses. This pattern applies to sample/demo code only and should not be interpreted as a best practice for production code. Reviewers should still require basic guards for critical paths (security, integrity) and clearly document that the code is a demonstration with comments indicating assumptions.
Applied to files:
packages/sample-app/src/sample_mcp.tspackages/sample-app/src/sample_vercel_ai_tools.tspackages/sample-app/src/sample_vercel_ai_agent.tspackages/sample-app/src/sample_vercel_ai_agent_simple.tspackages/sample-app/src/sample_nested_agents.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/traceloop-sdk/src/lib/node-server-sdk.ts : Register new instrumentation packages during SDK initialization
Applied to files:
packages/instrumentation-anthropic/package.jsonpackages/instrumentation-cohere/package.jsonpackages/sample-app/src/sample_vercel_ai_tools.tspackages/traceloop-sdk/package.jsonpackages/sample-app/src/sample_vercel_ai_agent.tspackages/sample-app/src/sample_vercel_ai_agent_simple.tspackages/sample-app/src/sample_nested_agents.ts
📚 Learning: 2025-07-28T14:30:22.017Z
Learnt from: galkleinman
Repo: traceloop/openllmetry-js PR: 620
File: packages/instrumentation-llamaindex/package.json:43-45
Timestamp: 2025-07-28T14:30:22.017Z
Learning: Lerna automatically converts `workspace:*` dependencies to actual published versions during the `lerna publish` command, so `workspace:*` in package.json files is safe and expected in Lerna-managed monorepos.
Applied to files:
package.json
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 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/package.jsonpackages/sample-app/tsconfig.jsonpackages/sample-app/src/sample_vercel_ai_agent.tspackages/sample-app/src/sample_vercel_ai_agent_simple.tspackages/sample-app/src/sample_nested_agents.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentations must extract request/response data and token usage from wrapped calls
Applied to files:
packages/instrumentation-bedrock/tests/meta.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 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/instrumentation-bedrock/tests/meta.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Instrumentations must capture and record errors appropriately
Applied to files:
packages/instrumentation-bedrock/tests/meta.test.ts
📚 Learning: 2025-08-24T22:08:07.023Z
Learnt from: CR
Repo: traceloop/openllmetry-js PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T22:08:07.023Z
Learning: Applies to packages/instrumentation-*/**/*.{ts,tsx} : Do not implement anonymous telemetry collection in instrumentation packages; telemetry is collected only in the SDK
Applied to files:
packages/instrumentation-bedrock/tests/meta.test.ts
🔇 Additional comments (39)
packages/sample-app/tsconfig.json (2)
6-14: LGTM!The updated compiler options are appropriate for a sample application:
module: "esnext"withmoduleResolution: "bundler"aligns with modern bundler-based tooling- Disabling declarations and source maps is reasonable since this isn't a published library
- Relaxing
noUnusedLocals/noUnusedParametersis acceptable for sample code that may contain illustrative snippets
17-17: No action needed. The sample-app package does not contain test files (notest/directory exists, and no.test.tsor.spec.tsfiles are present). The change to theincludepattern is therefore not a concern.Likely an incorrect or invalid review comment.
packages/sample-app/src/sample_vercel_ai_tools.ts (5)
14-38: LGTM! Tool schema migration is correct.The
getWeathertool correctly usesinputSchemaand the execute handler type annotation matches the schema definition.
40-72: LGTM!calculateDistancetool correctly migrated.The multi-line type annotation for the destructured parameters is clear and matches the
inputSchemadefinition.
74-120: LGTM!searchRestaurantstool correctly migrated.The optional
cuisineparameter is properly typed withcuisine?: string.
141-141: LGTM! Step-based termination updated.Replacing
maxStepswithstopWhen: stepCountIs(5)aligns with the AI SDK v5 API for controlling multi-step tool execution.
3-3: LGTM! Import updated for AI SDK v5 API.The addition of
stepCountIsaligns with the updated Vercel AI SDK v5 API for step-based execution control. The implementation correctly uses the pattern withstopWhen: stepCountIs(5)for managing agentic loop iterations.packages/sample-app/src/sample_vercel_ai_agent.ts (7)
3-3: LGTM! Import updated consistently.
50-80: LGTM!searchKnowledgetool correctly migrated.
82-113: LGTM!analyzeTrendstool correctly migrated.The array type
technologies: string[]is properly annotated.
115-141: LGTM!generateReporttool correctly migrated.
143-169: LGTM!saveToMemorytool correctly migrated.
171-199: LGTM!recallFromMemorytool correctly migrated.
268-268: LGTM! Step-based termination updated.
stopWhen: stepCountIs(10)provides appropriate step limits for this multi-tool research agent.packages/sample-app/src/sample_vercel_ai_agent_simple.ts (4)
3-3: LGTM! Import updated consistently.
14-54: LGTM!calculatetool correctly migrated.The enum type annotation
operation: "add" | "subtract" | "multiply" | "divide"correctly matches the zod enum definition.
57-111: LGTM!createTasktool correctly migrated.The optional
taskName?: stringcorrectly matches the.optional()modifier in the schema.
148-148: LGTM! Step-based termination updated.packages/sample-app/src/sample_nested_agents.ts (9)
3-3: LGTM! Import updated consistently.
13-28: LGTM!searchToolcorrectly migrated.
30-53: LGTM!searchAgentToolcorrectly migrated with nested step control.The nested
generateTextcall appropriately usesstopWhen: stepCountIs(3)for the delegated search agent.
55-68: LGTM!analyzeToolcorrectly migrated.
70-93: LGTM!analysisAgentToolcorrectly migrated with nested step control.
95-119: LGTM!researchAgentToolcorrectly migrated.The higher step count of 5 is appropriate since this agent delegates to both search and analysis sub-agents.
121-131: LGTM!formatToolcorrectly migrated.
133-154: LGTM!summaryAgentToolcorrectly migrated.
168-185: LGTM! Orchestrator workflow correctly updated.The top-level orchestrator appropriately uses
stopWhen: stepCountIs(5)for coordinating research and summary agents.packages/sample-app/src/sample_mcp.ts (1)
71-85: Consistent with the add tool pattern.Same approach as the first tool registration. The explicit type annotation
{ x: number; y: number }maintains internal type safety.packages/instrumentation-qdrant/package.json (1)
54-54: LGTM!Minor version bump for
@qdrant/js-client-restfrom ^1.15.0 to ^1.16.2 is a safe update for CVE remediation. The devDependency is used for testing and won't affect production builds.packages/instrumentation-llamaindex/package.json (1)
57-57: Approve, but verify tests pass with this update.The
llamaindexbump from ^0.11.19 to ^0.12.1 crosses a minor version boundary in the 0.x range, which may include breaking API changes per semver conventions. Since this is a devDependency used for testing, ensure the test suite passes after this update.packages/instrumentation-bedrock/package.json (1)
49-49: LGTM!The
@aws-sdk/client-bedrock-runtimeupdate from ^3.848.0 to ^3.969.0 stays within the same major version and should maintain backward compatibility. This is appropriate for CVE remediation.packages/instrumentation-mcp/package.json (1)
49-49: No action needed.The
@modelcontextprotocol/sdkversion update is compatible with the instrumentation. The code already declares version support for[">=1.0.0"], which encompasses both the old and new versions. The instrumentation patches only stable public methods (Client.connect,Client.request,Server.request) with defensive checks, and the SDK is a devDependency used only for development and testing, not production runtime.packages/instrumentation-cohere/package.json (1)
54-54: [Your rewritten review comment text here]
[Exactly ONE classification tag]packages/instrumentation-anthropic/package.json (1)
57-57: Verify if explicit devDependency is needed alongside pnpm overrides.The
qspackage is added here as a devDependency, but the AI summary indicates rootpackage.jsonalready has pnpm overrides forqs ^6.14.1. Typically, pnpm overrides should force the desired version for all transitive dependencies without needing explicit additions in each package.Is this explicit devDependency required for the override to take effect in this workspace package, or can it be removed in favor of relying solely on the root-level override?
packages/sample-app/package.json (1)
57-89: LGTM - Sample app dependency updates.The dependency updates align with the CVE remediation objective and are consistent with version bumps in other packages across the monorepo. Since this is a private sample app, these updates primarily ensure the samples demonstrate usage with secure, up-to-date library versions.
packages/instrumentation-bedrock/tests/meta.test.ts (1)
210-223: LGTM - Streaming test refactored for correctness.Moving assertions outside the streaming loop is the right approach. The test now correctly waits for all chunks to be processed before validating span attributes against the final invocation metrics.
packages/traceloop-sdk/package.json (1)
94-126: LGTM - SDK devDependency updates for CVE remediation.The version bumps are consistent with updates across the monorepo and align with the CVE remediation objective. The addition of
zodas a devDependency matches its usage in the sample-app.package.json (2)
48-62: Good use of pnpm overrides for CVE remediation.Using
pnpm.overridesto pin transitive dependency versions is the correct approach for this monorepo to ensure all packages use secure versions regardless of what their direct dependencies specify. This is preferable to manually updating each package's dependencies.
35-36: Lerna 9 upgrade appears compatible with current setup.Lerna 9.0.3 introduces breaking changes (Node 18 support dropped;
bootstrap,add,linkcommands removed), but this repo is well-positioned for the upgrade: Node 20 is already in use (.nvmrc), no deprecated commands are used in CI workflows, and the modern pnpm + nx approach is compatible. Run the release workflows in CI to confirm the upgrade works end-to-end.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "pnpm": { | ||
| "overrides": { | ||
| "zod": "^3.25.76", | ||
| "form-data": "^4.0.4", | ||
| "glob": "^10.5.0", | ||
| "jws": "^4.0.1", | ||
| "undici": "^7.18.2", | ||
| "diff": "^8.0.3", | ||
| "cookie": "^0.7.0", | ||
| "qs": "^6.14.1", | ||
| "body-parser": "^2.2.1", | ||
| "tmp": "^0.2.4", | ||
| "playwright": "^1.55.1", | ||
| "@smithy/config-resolver": "^4.4.0" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files package.jsonRepository: traceloop/openllmetry-js
Length of output: 80
🏁 Script executed:
cat -n package.json | head -80Repository: traceloop/openllmetry-js
Length of output: 2297
🏁 Script executed:
git log --oneline -5 package.json 2>/dev/null | head -10Repository: traceloop/openllmetry-js
Length of output: 81
🌐 Web query:
zod latest version npm registry 2025 2026
💡 Result:
Zod latest published version (npm) as of Jan 15, 2026: 4.3.5 (published Jan 4, 2026). [1][2]
Sources:
- Yarn package listing (zod — "latest" = 4.3.5, Jan 4, 2026). [1]
- Release history / registry view (shows recent 4.x releases and canaries). [2]
🌐 Web query:
undici CVE security vulnerabilities 2025
💡 Result:
Recent 2025 CVEs for undici (HTTP/1.1 client for Node.js):
-
CVE-2025-22150 — Use of insufficiently random values: undici used Math.random() for multipart/form-data boundaries, allowing prediction and request tampering in specific attacker-controlled scenarios. Patched in 5.28.5, 6.21.1, 7.2.3. [1][3]
-
CVE-2025-47279 — Memory leak / DoS via bad/invalid certificate data when repeatedly calling a webhook-like endpoint. Patched in 5.29.0, 6.21.2, 7.5.0. [2][4]
References:
- GitHub Advisory (CVE-2025-22150). [1]
- GitHub Advisory (CVE-2025-47279). [2]
- NVD entry for CVE-2025-22150. [3]
- NVD entry for CVE-2025-47279. [4]
Would you like upgrade/mitigation commands or a script to detect affected versions in your projects?
🌐 Web query:
body-parser 2.2.1 npm package
💡 Result:
Summary: body-parser@2.2.1 is a patch release that fixes a DoS/resource-consumption issue in 2.2.0 (patched CVE: CVE-2025-13466). Install: npm install body-parser@2.2.1. Sources: GitHub security advisory (patched in 2.2.1). [1] Additional package metadata (current/latest versions and security notes). [2][3]
Sources
- ExpressJS security advisory (GHSA-wqch-xfxh-vrr4) showing 2.2.0 affected and 2.2.1 as the patch. [1]
- Snyk package/version pages (notes on 2.2.1 and later versions). [2]
- Package listing / metadata (jsDelivr / Socket showing recent versions). [3]
🌐 Web query:
playwright 1.55.1 latest version npm 2026
💡 Result:
1.55.1 is not the latest. The latest stable Playwright release on npm is 1.56.1 (published Oct 17, 2025). [1][2]
You can verify locally with: npm view playwright version. [2]
Sources:
[1] Release listings for playwright (shows v1.56.1).
[2] npmjs playwright package page.
🌐 Web query:
form-data qs cookie diff npm latest versions 2026
💡 Result:
Latest npm "latest" versions (checked Jan 15, 2026):
- form-data — 4.0.5. [1]
- qs — 6.14.1. [2]
- cookie — 1.0.2. [3]
- diff (jsdiff) — 8.0.2. [4]
Sources: [1] npm/socket package listing for form-data. [2] qs package versions listing. [3] cookie package listing. [4] diff (jsdiff) npm page.
🌐 Web query:
jws package npm latest version 2025 2026
💡 Result:
The latest published version of the npm package "jws" appears to be 4.0.1 (superseding 4.0.0). [1][2]
Sources:
- socket.dev package page showing version 4.0.1 as latest. [1]
- npmjs package page (shows 4.0.0). [2]
🌐 Web query:
glob npm latest version 2025 2026
💡 Result:
The latest published version of the npm package "glob" is 13.0.0 (released Nov 19, 2025). [1][2][3]
Sources:
- Release history listing showing 13.0.0 (Nov 19, 2025). [1]
- Yarn package page (latest: 13.0.0, Nov 19, 2025). [2]
- Snyk package overview indicating latest version 13.0.0. [3]
🌐 Web query:
tmp npm latest version 2026
💡 Result:
As of January 15, 2026 the latest published version of the npm package "tmp" is 0.2.5. [1][2]
Sources:
- npm package page for tmp. [1]
- Package overview on Socket (tmp — latest 0.2.5). [2]
🌐 Web query:
diff npm package version 8.0.3 does not exist jsdiff
💡 Result:
Short answer: The package published as "diff" (the jsdiff library) has no 8.0.3 release on npm — the latest published 8.x version shown on npm is 8.0.2. Use diff@8.0.2 or run npm view to confirm locally.
Sources:
- npm package page (diff) — shows version 8.0.2. [1]
- GitHub repository (kpdecker/jsdiff). [2]
Commands you can run to verify:
- npm view diff versions
- npm info diff version
References:
[1] https://www.npmjs.com/package/diff
[2] https://github.com/kpdecker/jsdiff
🌐 Web query:
undici version 7.18.2 release date when published
💡 Result:
Undici v7.18.2 was published on January 6, 2026. [1][2]
Sources:
[1] Yarn package page (undici) — shows 7.18.2 dated January 6, 2026.
[2] SourceForge mirror listing for v7.18.2 — modified 2026-01-06.
Address non-existent diff version and update significantly outdated packages.
The pnpm overrides include a version that does not exist: diff@8.0.3 should be diff@8.0.2. Additionally, several packages are significantly outdated:
zod: Currently pinned to ^3.25.76; latest is 4.3.5 (Jan 2026)glob: Currently pinned to ^10.5.0; latest is 13.0.0 (Nov 2025)cookie: Currently pinned to ^0.7.0; latest is 1.0.2playwright: Currently pinned to ^1.55.1; latest is 1.56.1 (Oct 2025)form-data,tmp: Each one patch behind latest
Packages that are current or adequately address known CVEs:
body-parser@2.2.1correctly patches CVE-2025-13466undici@7.18.2covers CVE-2025-22150 and CVE-2025-47279
Update the overrides to use current versions, starting with diff@8.0.2 instead of the non-existent 8.0.3. Run npm view <package> versions for each override to confirm latest stable releases before updating.
🤖 Prompt for AI Agents
In `@package.json` around lines 48 - 62, The pnpm overrides block (pnpm.overrides)
contains a non-existent version and several outdated pins; change "diff":
"8.0.3" to "8.0.2" and update the other override entries to current stable
releases (e.g., bump zod, glob, cookie, playwright, form-data, tmp as noted) by
querying registry versions with npm view <package> versions before changing;
ensure you only modify the values in the overrides object and keep body-parser
and undici as-is since they intentionally address CVEs.
| if (finalParsedResponse) { | ||
| assert.strictEqual( | ||
| attributes[ATTR_GEN_AI_USAGE_PROMPT_TOKENS], | ||
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | ||
| "inputTokenCount" | ||
| ], | ||
| ); | ||
| assert.strictEqual( | ||
| attributes[ATTR_GEN_AI_USAGE_COMPLETION_TOKENS], | ||
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | ||
| "outputTokenCount" | ||
| ], | ||
| ); | ||
| assert.strictEqual( | ||
| attributes[SpanAttributes.LLM_USAGE_TOTAL_TOKENS], | ||
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | ||
| "inputTokenCount" | ||
| ] + | ||
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | ||
| "outputTokenCount" | ||
| ], | ||
| ); | ||
| } |
There was a problem hiding this comment.
Test may silently pass if final metrics chunk is missing.
The conditional if (finalParsedResponse) allows the test to pass without validating token counts if no chunk contains amazon-bedrock-invocationMetrics. This could mask regressions if the API response format changes or if streaming fails to deliver the final metrics chunk.
Consider adding an explicit assertion to ensure the metrics chunk was received:
Suggested fix
+ // Ensure we received the final metrics chunk
+ assert.ok(
+ finalParsedResponse,
+ "Expected final chunk with amazon-bedrock-invocationMetrics",
+ );
+
// Token counts should match the final invocation metrics
- if (finalParsedResponse) {
- assert.strictEqual(
+ assert.strictEqual(Then remove the closing } on line 274 and unindent the assertions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (finalParsedResponse) { | |
| assert.strictEqual( | |
| attributes[ATTR_GEN_AI_USAGE_PROMPT_TOKENS], | |
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | |
| "inputTokenCount" | |
| ], | |
| ); | |
| assert.strictEqual( | |
| attributes[ATTR_GEN_AI_USAGE_COMPLETION_TOKENS], | |
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | |
| "outputTokenCount" | |
| ], | |
| ); | |
| assert.strictEqual( | |
| attributes[SpanAttributes.LLM_USAGE_TOTAL_TOKENS], | |
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | |
| "inputTokenCount" | |
| ] + | |
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | |
| "outputTokenCount" | |
| ], | |
| ); | |
| } | |
| // Ensure we received the final metrics chunk | |
| assert.ok( | |
| finalParsedResponse, | |
| "Expected final chunk with amazon-bedrock-invocationMetrics", | |
| ); | |
| // Token counts should match the final invocation metrics | |
| assert.strictEqual( | |
| attributes[ATTR_GEN_AI_USAGE_PROMPT_TOKENS], | |
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | |
| "inputTokenCount" | |
| ], | |
| ); | |
| assert.strictEqual( | |
| attributes[ATTR_GEN_AI_USAGE_COMPLETION_TOKENS], | |
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | |
| "outputTokenCount" | |
| ], | |
| ); | |
| assert.strictEqual( | |
| attributes[SpanAttributes.LLM_USAGE_TOTAL_TOKENS], | |
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | |
| "inputTokenCount" | |
| ] + | |
| finalParsedResponse["amazon-bedrock-invocationMetrics"][ | |
| "outputTokenCount" | |
| ], | |
| ); |
🤖 Prompt for AI Agents
In `@packages/instrumentation-bedrock/tests/meta.test.ts` around lines 252 - 274,
The test currently skips validating token counts when finalParsedResponse is
falsy; add an explicit assertion that finalParsedResponse was received (e.g.,
assert.ok(finalParsedResponse, 'Expected finalParsedResponse containing
amazon-bedrock-invocationMetrics')) and then remove the surrounding if
(finalParsedResponse) block so the subsequent assertions against
ATTR_GEN_AI_USAGE_PROMPT_TOKENS, ATTR_GEN_AI_USAGE_COMPLETION_TOKENS, and
SpanAttributes.LLM_USAGE_TOTAL_TOKENS always run against
finalParsedResponse["amazon-bedrock-invocationMetrics"].
| }, | ||
| "dependencies": { | ||
| "@langchain/core": "^0.3.70", | ||
| "@langchain/core": "^0.3.80", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/instrumentation-langchain/package.jsonRepository: traceloop/openllmetry-js
Length of output: 2373
🌐 Web query:
@langchain/core 0.3.80 security fix CVE
💡 Result:
Short answer: @langchain/core v0.3.80 includes a security fix for a template-injection vulnerability (CVE-2025-65106). Upgrade to v0.3.80 or later; note a separate, critical serialization-injection CVE (CVE-2025-68664) was fixed in 0.3.81, so if you need protection from that issue upgrade to >=0.3.81. [1][2]
Sources:
[1] OpenCVE / CVE-2025-65106 (patches in 0.3.80).
[2] Snyk / LangChain advisories and version recommendations (template injection fix in 0.3.80; serialization fix in 0.3.81).
🌐 Web query:
@langchain/community 0.3.59 security fix
💡 Result:
Do you mean the npm package @langchain/community version 0.3.59 and want details about a security fix in that release (or instructions to upgrade)?
🌐 Web query:
langchain 0.3.37 security fix changelog
💡 Result:
Summary — langchain v0.3.37 security fix
- Fix: patched a serialization-injection vulnerability that could let attacker-controlled JSON with internal "lc" keys be treated as LangChain objects during deserialization (enabling secret extraction / prompt injection). [1]
- Technical changes: improved escaping in toJSON/serialization, tightened defaults for load()/loads() (e.g., secretsFromEnv defaulted to false), added restrictions such as allowed_objects / maxDepth and blocking unsafe template usage (Jinja2) by default. [1][2]
- Recommendation: upgrade any affected JS/npm langchain/@langchain/core versions older than the patched ranges (langchain < 0.3.37 or
@langchain/core< 0.3.80) to the fixed releases. [1][3]
Sources: [1] GitHub security advisory for LangChain.js; [2] The Hacker News coverage; [3] Chainguard advisory.
@langchain/core version 0.3.80 does not address CVE-2025-68664 (serialization-injection); upgrade to 0.3.81 or later.
The version for @langchain/core should be updated to ^0.3.81 or later. While v0.3.80 fixes CVE-2025-65106 (template-injection), it leaves CVE-2025-68664 (critical serialization-injection vulnerability) unpatched. Version 0.3.81 addresses this additional critical CVE. The langchain v0.3.37 update appropriately addresses the serialization-injection vulnerability on its side.
🤖 Prompt for AI Agents
In `@packages/instrumentation-langchain/package.json` at line 41, Update the
`@langchain/core` dependency in package.json from "^0.3.80" to "^0.3.81" (or
newer) to include the fix for CVE-2025-68664; after changing the version, run
the package manager install (npm/yarn/pnpm) to update the lockfile and verify
the updated package is present, and run tests or npm audit to ensure no
remaining vulnerabilities.
Important
Update dependencies across multiple packages to remediate CVEs and improve test handling of streaming responses.
lernato^9.0.3and addjs-yamltopackage.json.qstodevDependenciesininstrumentation-anthropic/package.jsonandinstrumentation-cohere/package.json.@aws-sdk/client-bedrock-runtimeto^3.969.0ininstrumentation-bedrock/package.jsonandsample-app/package.json.@langchain/coreto^0.3.80and@langchain/communityto^0.3.59ininstrumentation-langchain/package.jsonandsample-app/package.json.llamaindexto^0.12.1ininstrumentation-llamaindex/package.jsonandsample-app/package.json.@modelcontextprotocol/sdkto^1.25.2ininstrumentation-mcp/package.jsonandsample-app/package.json.@qdrant/js-client-restto^1.16.2ininstrumentation-qdrant/package.json.aito^5.0.52insample-app/package.jsonandtraceloop-sdk/package.json.cheerioto^1.1.2insample-app/package.json.meta.test.tsininstrumentation-bedrock/teststo handle streaming responses and assertions after processing all chunks.This description was created by
for 71fb01b. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.