feat(opentelemetry): Widen peer dependencies to support Otel v2 #16246
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report 📦
|
3 tasks
andreiborza
force-pushed
the
ab/sentry-otel-widen-deps
branch
2 times, most recently
from
7bd99e1 to
5cff48b
Compare
andreiborza
requested review from
mydea,
Lms24 and
chargome
and removed request for
Lms24
mydea
reviewed
May 12, 2025
.github/workflows/build.yml
Outdated
| @@ -751,6 +754,42 @@ jobs: | |||
| directory: dev-packages/node-integration-tests | |||
| token: ${{ secrets.CODECOV_TOKEN }} | |||
|
|
|||
| job_opentelemetry_v2_tests: | |||
There was a problem hiding this comment.
h: do we need this? Can we not just run this normally through the node unit tests?
There was a problem hiding this comment.
Ah, yea maybe that would make it easier to revert the change too. I'll update.
There was a problem hiding this comment.
Removed and instead relying on node unit tests.
mydea
reviewed
May 12, 2025
| // looking up the parent span id according to the new API | ||
| // In OTel v1, the parent span id is accessed as `parentSpanId` | ||
| // In OTel v2, the parent span id is accessed as `spanId` on the `parentSpanContext` | ||
| const parentSpanId = |
There was a problem hiding this comment.
l: Can we use the getParentSpanId() util here too? :)
There was a problem hiding this comment.
I was thinking about that but then I'd need to pull it into core which means more api surface? I preferred not to, but can change if you think that's better?
There was a problem hiding this comment.
ah right, other package - all good then!
andreiborza
force-pushed
the
ab/sentry-otel-widen-deps
branch
from
68950bf to
5b8503d
Compare
mydea
approved these changes
May 12, 2025
chargome
approved these changes
May 12, 2025
| await spanProcessor.forceFlush(); | ||
| } | ||
|
|
||
| await provider?.forceFlush(); |
There was a problem hiding this comment.
Has this changed for both api versions?
There was a problem hiding this comment.
Yep, this was already supported in v1.
There was a problem hiding this comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In preparation for supporting OpenTelemetry v2 we widened the peer dependency range of
@sentry/opentelemetryto allow v2 dependencies.Note:
@sentry/nodestill requires the v1 packages of OpenTelemetry, this is just preparatory work to allow us to bump this down the road.Parts of
@sentry/opentelemetry(and@sentry/core) have been reworked to be compatible with both OpenTelemetry v1 and v2.Unit tests in
@sentry/opentelemetryare running with v1 dependencies while a new package was added todev-packages/with a copy of all unit tests from@sentry/opentelemetryto run with v2 dependencies, adjusted to breaking changes introduced with OpenTelemetry v2.