-
-
Notifications
You must be signed in to change notification settings - Fork 434
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
POTEL 21 - Drop OpenTelemetry spans for internal Sentry requests #3508
Conversation
…pes-merge-2-add-scopes
…ainScopes to rootScopes
|
Performance metrics 🚀
|
if (containsSpotlightUrl(httpUrl, spotlightUrl)) { | ||
return true; | ||
} | ||
// } |
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.
Leftover that can be deleted
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.
Ideally we reintroduce the if
above, then it's needed again. We can retest after doing the alpha release.
@@ -134,51 +129,9 @@ private boolean isSpanTooOld(final @NotNull SpanData span, final @NotNull Sentry | |||
} | |||
|
|||
private @NotNull List<SpanData> filterOutSentrySpans(final @NotNull Collection<SpanData> spans) { |
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.
Are there instances where SentrySampler
is not invoked and spans need to be filtered here in the exporter?
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.
Theoretically it's possible the span is modified during runtime and then has different attributes in SpanExporter
.
#skip-changelog
📜 Description
Requests to Sentry cause OpenTelemetry auto instrumentation to create spans. We now drop them in
SentrySampler
usingSamplingDecision.DROP
.💡 Motivation and Context
#3499
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps