-
-
Notifications
You must be signed in to change notification settings - Fork 427
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 3 - Use OpenTelemetry in Sentry Performance API #3416
base: feat/potel-2-promote-span-attributes
Are you sure you want to change the base?
POTEL 3 - Use OpenTelemetry in Sentry Performance API #3416
Conversation
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- POTEL 3 - Use OpenTelemetry in Sentry Performance API ([#3416](https://github.com/getsentry/sentry-java/pull/3416)) If none of the above apply, you can opt out of this check by adding |
@@ -196,4 +200,14 @@ public void setAppStartTransaction(final boolean appStartTransaction) { | |||
public boolean isAppStartTransaction() { | |||
return isAppStartTransaction; | |||
} | |||
|
|||
@ApiStatus.Internal | |||
public @Nullable ISpanFactory getSpanFactory() { |
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.
This is required to override the span factory when creating Sentry transactions in OTel SpanExporter
to avoid endless loops.
Performance metrics 🚀
|
…-use-otel-in-sentry-api
@NotNull IScopes scopes, | ||
@NotNull SpanOptions spanOptions, | ||
@Nullable ISpan parentSpan) { | ||
// TODO [POTEL] forward to SentryTracer.createChild? |
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.
In case of DefaultSpanFactory
, couldn't we just call parentSpan.createChild()
?
final @NotNull IScopes scopes, | ||
final @NotNull SpanOptions spanOptions, | ||
final @Nullable ISpan parentSpan) { | ||
final @NotNull SpanBuilder spanBuilder = getTracer().spanBuilder(name); |
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.
TODO: forward sampling decision to OTEL
} | ||
// TODO [POTEL] start timestamp | ||
final @NotNull Span span = spanBuilder.startSpan(); | ||
return new OtelSpanWrapper(span, scopes); |
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.
Shouldn't we be able to read the OtelSpanWrapper
from the spanStorage
at this point?
As spanBuilder.startSpan()
calls spanProcessor.onStart()
before returning the span.
|
||
@Override | ||
public void setDescription(@Nullable String description) { | ||
// TODO [POTEL] need to find a way to transfer data from this wrapper to SpanExporter |
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.
Possibly in SentrySpanExporter.createAndFinishSpanForOtelSpan()
if the wrapper is stored in SentryWeakSpanStorage
// TODO [POTEL] should check if enabled but multi init with different options makes testing hard | ||
// atm | ||
// if (scopes.getOptions().isEnableSpotlight()) { | ||
final @Nullable String spotlightUrl = scopes.getOptions().getSpotlightConnectionUrl(); |
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.
Maybe something like this for a little less nesting?
final @Nullable String optionsSpotlightUrl = scopes.getOptions().getSpotlightConnectionUrl();
final @NotNull String spotlightUrl = optionsSpotlightUrl != null ? optionsSpotlightUrl : "http://localhost:8969/stream";
if(containsSpotlightUrl(fullUrl, spotlightUrl)) {
return true;
}
if(containsSpotlightUrl(httpUrl, spotlightUrl)) {
return true;
}
📜 Description
💡 Motivation and Context
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps