Update the OpenTelemetry dependencies - #1497
Conversation
008f1fc to
8e4b32e
Compare
There was a problem hiding this comment.
Pull request overview
This pull request updates the project鈥檚 bundled OpenTelemetry dependency set (including test expectations to match updated span attribute keys/names) and adds tooling to dump and compare integration-test span outputs across runs, helping detect silent telemetry shape changes.
Changes:
- Update OpenTelemetry-related dependencies (and lockfile) to newer versions intended to resolve reported CVEs and align with stable semantic conventions.
- Update integration tests/helpers to assert on new stable span attribute keys and updated span names (e.g., DB and Mongo/Redis changes).
- Add span dump + comparison utilities and add optional
ARCHIVE_SPANSsupport to test-app Docker Compose setups to retain spans across examples.
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/restify/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/nextjs/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/nestjs/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/koa3-mysql/tests/spec/app_spec.rb | Update SQL span attribute assertions to stable DB keys. |
| test/koa3-mysql/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/koa-mysql/tests/spec/app_spec.rb | Update SQL span attribute assertions to stable DB keys. |
| test/koa-mysql/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/koa-mongo/tests/spec/app_spec.rb | Update Mongo span name and collection attribute assertions for newer instrumentation output. |
| test/koa-mongo/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/helpers/span.rb | Add archive_all to append per-example spans into a consolidated archive file when enabled. |
| test/helpers/redis_helper.rb | Update Redis span matching to stable semantic attribute keys. |
| test/helpers/integration_helper.rb | Archive spans before clearing per example; archive remaining spans after suite. |
| test/fastify/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-yoga/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-redis/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-rabbitmq/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-prisma-postgres/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-prisma-mongo/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-postgres/tests/spec/app_spec.rb | Update SQL span attribute assertions to stable DB keys. |
| test/express-postgres/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-mongoose/tests/spec/app_spec.rb | Update Mongoose/Mongo span name assertion for newer instrumentation output. |
| test/express-mongoose/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-knex/tests/spec/app_spec.rb | Update Knex SQL span attribute assertions to stable DB keys. |
| test/express-knex/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| test/express-apollo/docker-compose.yml | Pass through ARCHIVE_SPANS for optional span archiving during suite runs. |
| scripts/compare_span_dumps.js | New tool to compare span dumps across runs by instrumentation library, names, and attribute keys. |
| script/dump_test_app_spans | New tool to run a test app suite and dump all spans produced during the run. |
| package.json | Update bundled OpenTelemetry dependency constraints/pins. |
| package-lock.json | Update resolved dependency graph for updated OpenTelemetry versions. |
| .changesets/update-the-opentelemetry-dependencies.md | Add changeset documenting the OpenTelemetry dependency update and referenced CVEs. |
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| resolves two reported vulnerabilities: CVE-2026-59892 in | ||
| `@opentelemetry/propagator-jaeger`, which this integration does not use but did | ||
| bundle through `@opentelemetry/sdk-node`, and CVE-2026-54285 in | ||
| `@opentelemetry/core`, which applies to the W3C baggage propagation that this | ||
| integration does use. |
There was a problem hiding this comment.
Make each of the CVEs a bullet point for readability.
There was a problem hiding this comment.
Done. Each CVE is its own bullet now.
| # script/dump_test_app_spans express-redis tmp/spans/before | ||
| # # change the dependencies | ||
| # script/dump_test_app_spans express-redis tmp/spans/after | ||
| # node scripts/compare_span_dumps.js tmp/spans/before tmp/spans/after |
There was a problem hiding this comment.
I'm not sure what to do with these scripts. On the one hand, the bobot did find them useful, to be able to compare before/after behaviour and confidently upgrade the OpenTelemetry integrations. On the other hand, that's what our tests are supposed to be for, and right now these scripts are dead code.
This whole workflow needs to be rethought beyond these scripts. These test apps check only a fraction of the span attributes and shape expectations that we have. For span attributes, it's mostly checking them on behalf of the agent, which has specific logic in its extractor that depends on them. Then there's the parent-child relationships, what constitutes a trace versus what is nested into another trace, which is arguably more important to AppSignal than what span attributes are present.
There's many libraries we support for which we don't have a test app here, or we don't have one on test-setups, or both. It's also unclear to me how much of this would even matter in a future collector mode version of the integration, though the collector also has extractors of its own.
I'm thinking to drop this whole thing for now -- the scripts and the ARCHIVE_SPANS logic they add to the helpers -- and come back to the whole problem space with fresh eyes later on.
There was a problem hiding this comment.
Dropped, all of it. The Add a way to compare the spans a test app produces commit is gone from this pull request, which takes script/dump_test_app_spans, scripts/compare_span_dumps.js, Span.archive_all, the after(:suite) hook and the ARCHIVE_SPANS passthrough in all sixteen compose files with it. The dependency bump is a separate commit and is untouched.
Agreed on the reasoning. The scripts caught the MongoDB span rename during this upgrade, but the suite caught that too, by failing on Span.find_by_name!("mongodb.find"). What the suite did not catch was the consequence, which is that the agent derived the operation from that name prefix. That assumption lives in a Rust file in another repository and is written down nowhere on this side, so a green suite here and a green suite there were both compatible with every MongoDB event being named wrong. That is the gap worth designing for, and it is not one these scripts close.
The `@opentelemetry/propagator-jaeger` version that resolved here carries CVE-2026-59892, and the `@opentelemetry/core` version carries CVE-2026-54285, which applies to the baggage propagation we do use. From version 0.221.0 only the stable HTTP, network and database attributes are emitted, and the agent reads the stable name first for every attribute it needs, so these spans report as before. `@opentelemetry/instrumentation-mongodb` also renamed its spans to `<operation> <collection>`, which the agent read the operation from, so this needs appsignal/appsignal-agent#1294. `@opentelemetry/core`, `sdk-trace-base` and `sdk-metrics` are pinned exactly because `sdk-node` pins them exactly, and any other range resolves a second copy.
Stacked on #1496.
Also depends on https://github.com/appsignal/appsignal-agent/pull/1294
being released. This pull request should be merged after that release.
Update the OpenTelemetry dependencies
The
@opentelemetry/propagator-jaegerversion that resolved herecarries CVE-2026-59892, and the
@opentelemetry/coreversion carriesCVE-2026-54285, which applies to the baggage propagation we do use.
From version 0.221.0 only the stable HTTP, network and database
attributes are emitted, and the agent reads the stable name first for
every attribute it needs, so these spans report as before.
@opentelemetry/instrumentation-mongodbalso renamed its spans to<operation> <collection>, which the agent read the operation from, sothis needs appsignal/appsignal-agent#1294.
@opentelemetry/core,sdk-trace-baseandsdk-metricsare pinned exactly becausesdk-nodepins them exactly, and any other range resolves a second copy.