-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
chore: Bump otel 2.x #15518
base: develop
Are you sure you want to change the base?
chore: Bump otel 2.x #15518
Conversation
8d2fad2
to
1a1fbdf
Compare
size-limit report 📦
|
❌ 18 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Looks like the unit test failures are mainly due to imports that have changed (for ex. because they are now types instead of classes) or fields that no longer exist on certain objects.
The pattern I see in E2E tests, and that is in a couple of unit tests, is that parent_span_id
seems to not be set anymore. I think this is because of this change: open-telemetry/opentelemetry-js#5450.
span.parentSpanId
-> span.parentSpanContext?.spanId
@@ -56,7 +55,7 @@ export class SentryNodeFetchInstrumentation extends InstrumentationBase<SentryNo | |||
private _propagationDecisionMap: LRUMap<string, boolean>; | |||
|
|||
public constructor(config: SentryNodeFetchInstrumentationOptions = {}) { | |||
super('@sentry/instrumentation-node-fetch', VERSION, config); | |||
super('@sentry/instrumentation-node-fetch', '2.0.0-dev.0', config); |
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.
considering this is using @sentry/X
I think it's reasonable enough to set our own SDK version here.
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.
Yea was thinking the same. Will update.
c0a29c3
to
3a56757
Compare
Would this update be considered a breaking change? I guess this could break compatibility with all otel integrations customers might be using? |
3a56757
to
793cc46
Compare
86ef801
to
dd500bd
Compare
dd500bd
to
f2314d4
Compare
This is a rough draft to bump Otel to the first pre-release version of 2.0.0. Tests are failing partly due to api changes and removed exports. I wasn't sure how to best tackle these and lack the knowledge to migrate some of these.
Followed migration guide: https://github.com/trentm/opentelemetry-js/blob/tm-sdk2-migration-docs-take2/doc/upgrade-to-2.x.md
Things to note:
register
function to register a propagator and context manager. I replacedBasicTracerProvider
withNodeTracerProvider
of@opentelemetry/sdk-trace-node
private
previously)activeSpanProcessor
with no provided alternatives. I opted to callforceFlush
on the provider instead which maps through the span processors and flushes them.@opentelemetry/core
, for now I hardcoded the version where it was needed to pass into instrumentations. Maybe we should use our own versions here?provider.addSpanProcessor()
sentry-docs#13011 - BasicTracerProvider#addSpanProcessor(...) was removed. We use the NodeTracerProvider#addSpanProcessor in the docs for combining existing otel setups with sentry: https://docs.sentry.io/platforms/javascript/guides/node/opentelemetry/custom-setup/