fix(security): bump @opentelemetry sdk-node/exporter-prometheus to 0.217.0 (with ReadableSpan migration)#702
fix(security): bump @opentelemetry sdk-node/exporter-prometheus to 0.217.0 (with ReadableSpan migration)#702langwatch-agent wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughThe PR bumps ChangesOpenTelemetry compatibility update
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6e88c4f to
51e5472
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
javascript/src/agents/judge/judge-span-collector.ts (1)
75-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared
getParentSpanIdhelper.This compatibility shim is now identical to
javascript/src/agents/judge/span-utils.ts. Importing the shared export here instead of keeping a third copy will keep future OpenTelemetry shape changes from drifting across files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@javascript/src/agents/judge/judge-span-collector.ts` around lines 75 - 81, The local getParentSpanId compatibility shim in judge-span-collector is duplicating the shared logic already implemented in span-utils; replace the inline helper with an import of the shared getParentSpanId export and update the collector to use that shared function so OpenTelemetry shape handling stays centralized and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@javascript/src/agents/judge/judge-span-collector.ts`:
- Around line 75-81: The local getParentSpanId compatibility shim in
judge-span-collector is duplicating the shared logic already implemented in
span-utils; replace the inline helper with an import of the shared
getParentSpanId export and update the collector to use that shared function so
OpenTelemetry shape handling stays centralized and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75cd1363-c866-459e-9501-408e03ce0d8b
⛔ Files ignored due to path filters (2)
javascript/package-lock.jsonis excluded by!**/package-lock.jsonjavascript/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
javascript/package.jsonjavascript/src/agents/judge/judge-span-collector.tsjavascript/src/agents/judge/judge-span-digest-formatter.tsjavascript/src/agents/judge/span-utils.ts
51e5472 to
46c113c
Compare
|
CI fully green. Ready for human review and merge. Resolves the HIGH @opentelemetry/sdk-node (#428) and exporter-prometheus (#427) advisories via the 0.217.0 bump, with the ReadableSpan parent-access migration (now version-agnostic: reads both parentSpanContext.spanId and the legacy parentSpanId via a cast, since the tree carries two sdk-trace-base versions). typecheck + tsup build + 150 judge tests green. The MODERATE @opentelemetry/core <2.8.0 alerts (#442/#450) are intentionally not included; see the PR description for why (old OTel 1.x subtree from the langwatch SDK dep). |
46c113c to
901e0f3
Compare
|
Rebased onto main. Two execution tests added on main since this PR opened (scenario-role-attributes, scenario-scope-attribute) construct a NodeTracerProvider with a SimpleSpanProcessor; after the sdk-node 0.217 bump those two symbols resolve to different @opentelemetry/sdk-trace-base copies in the tree (the langwatch SDK pulls an older 1.x subtree), so tsc flagged a SpanProcessor type mismatch. Cast to the provider's expected type to keep tsc happy without disturbing the 1.x subtree. Re-dogfooded: typecheck, tsup build (incl DTS), judge slice (150) and the two edited tests all pass; frozen install clean. Mergeable again. |
…217.0 Clears HIGH alerts #427 (exporter-prometheus) and #428 (sdk-node) by bumping @opentelemetry/sdk-node to 0.217.0. App-side changes the bump requires: - ReadableSpan.parentSpanId became parentSpanContext in the newer SDK; the judge span utilities read the parent id version-agnostically (span-utils, judge-span-collector, judge-span-digest-formatter). - The tree carries multiple @opentelemetry/sdk-trace-base copies (the langwatch SDK pulls an older 1.x subtree), so in the role/scope execution tests SimpleSpanProcessor and NodeTracerProvider's expected SpanProcessor resolve to different copies. Cast to the provider's expected type to keep tsc happy without touching the 1.x subtree (build and runtime are fine). Dogfooded: typecheck, tsup build (incl DTS), judge unit slice (150), and the edited execution tests all pass; frozen install clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
901e0f3 to
addd7c7
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
javascript/src/execution/__tests__/scenario-role-attributes.test.ts (1)
57-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate cast block — extract into shared test helper.
The identical
spanProcessorscast (with the same explanatory comment) appears twice here and again inscenario-scope-attribute.test.ts. Since this is a workaround tied to a specific dependency-tree quirk, consolidating it into a shared helper (e.g.,createTestTracerProvider(exporter)) would keep all three sites in sync if the cast ever needs to change or be removed once the duplicatesdk-trace-basecopies are resolved.♻️ Suggested helper extraction
// e.g. in a shared test-utils file export function createTestSpanProcessors(exporter: InMemorySpanExporter) { // Multiple `@opentelemetry/sdk-trace-base` copies coexist in the tree, so // SimpleSpanProcessor and NodeTracerProvider's expected SpanProcessor can // resolve to different copies. Cast to the type the provider expects. return [ new SimpleSpanProcessor(exporter), ] as unknown as NonNullable< ConstructorParameters<typeof NodeTracerProvider>[0] >["spanProcessors"]; }Also applies to: 191-198
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@javascript/src/execution/__tests__/scenario-role-attributes.test.ts` around lines 57 - 64, The same spanProcessors cast and explanatory comment are duplicated in this test and scenario-scope-attribute.test.ts, so extract that workaround into a shared helper instead of repeating it. Move the NodeTracerProvider/SimpleSpanProcessor cast logic into a reusable test utility such as createTestTracerProvider or createTestSpanProcessors, then update both tests to call it so any future change to the OpenTelemetry workaround stays in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@javascript/src/execution/__tests__/scenario-role-attributes.test.ts`:
- Around line 57-64: The same spanProcessors cast and explanatory comment are
duplicated in this test and scenario-scope-attribute.test.ts, so extract that
workaround into a shared helper instead of repeating it. Move the
NodeTracerProvider/SimpleSpanProcessor cast logic into a reusable test utility
such as createTestTracerProvider or createTestSpanProcessors, then update both
tests to call it so any future change to the OpenTelemetry workaround stays in
one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38c683d6-4c85-4039-930d-213871d042e7
⛔ Files ignored due to path filters (2)
javascript/package-lock.jsonis excluded by!**/package-lock.jsonjavascript/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
javascript/package.jsonjavascript/src/agents/judge/judge-span-collector.tsjavascript/src/agents/judge/judge-span-digest-formatter.tsjavascript/src/agents/judge/span-utils.tsjavascript/src/execution/__tests__/scenario-role-attributes.test.tsjavascript/src/execution/__tests__/scenario-scope-attribute.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- javascript/src/agents/judge/judge-span-collector.ts
- javascript/src/agents/judge/judge-span-digest-formatter.ts
- javascript/src/agents/judge/span-utils.ts
- javascript/package.json
|
🐕 PR Hound review brief — assigned reviewer @0xdeafcafe Review mode: Targeted Review Inspection targets (max 3):
Author questions:
|
|
Automated low-risk assessment This PR was evaluated against the repository's Low-Risk Pull Requests procedure and does not qualify as low risk.
This PR requires a manual review before merging. |
Raises the OpenTelemetry SDK in the javascript package to close the HIGH OTel advisories, and adapts the judge span code to the new SDK (path C: the bump breaks typecheck, the fix ships in the same PR).
Alerts resolved
<0.217.0-> 0.217.0 (fix(security): patch path-to-regexp DoS in openai-realtime-demo (CVE-2026-4926) #428)<0.217.0-> 0.217.0 (fix(security): bump lodash-es override to >=4.18.0 (CVE-2026-4800) #427)Changes
@opentelemetry/sdk-node0.212.0 -> 0.217.0 (pulls exporter-prometheus 0.217.0). Both javascript locks regenerated; the only non-OTel addition isimport-in-the-middle(OTel's instrumentation helper).ReadableSpan.parentSpanId(string) with the typedparentSpanContext(SpanContext). The threegetParentSpanIdhelpers (span-utils,judge-span-collector,judge-span-digest-formatter) now readparentSpanContext.spanId, with the flatparentSpanIdkept as a fallback for older span implementations.Verification
tsc --noEmitpasses (it failed before the migration withProperty 'parentSpanId' does not exist on type 'ReadableSpan').tsupbuild succeeds.pnpm install --frozen-lockfileandnpm ciboth clean.Not included
<2.8.0(chore(deps): bump actions/cache from 3.5.0 to 5.0.5 #442 npm, fix(deps): bump fast-uri to >=3.1.2 for high severity CVEs #450 pnpm): a blanket core>=2.8.0override breaks the build, because an older OTel 1.30.1 subtree (pulled transitively via the langwatch SDK dependency) importsgetEnv/TracesSamplerValuesthat core 2.x removed. Flooring core to 2.8.0 needs that 1.x subtree migrated to 2.x first, which is a separate, larger change.