-
-
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
Changes from all commits
305baf5
95f5e1b
27f2398
ce3c14f
ce615f4
22ddc00
305c217
da927bc
8279276
9bfc086
b998e50
739827a
69f2d63
792d482
9bcbce6
3f25a4b
d6fb40a
7752bcc
1e329c5
b0d89ae
cdd414a
98da9ff
2d26033
bbb6700
c714b21
a474402
ae93e33
b01298b
b64e688
d06fc50
f0af5c3
2f02001
bf4a7bf
62cb91a
b1630ea
136b9ce
94d54ef
017599d
f4c2b3c
61c9d4a
04f3892
840c194
ab1c3a6
23506c5
c9b6f8b
696a809
847200d
8e86d3b
06db228
37ab4d0
faef2f8
c57b2d3
5b15128
9d48538
9b900c7
b3919bd
36ed84a
8f56e3f
81982bc
fba451c
c787e91
8c25d81
ffe1bef
a31fc08
789ae1e
f0c8cf2
0d84b57
ce1f729
720b07c
20f8a86
2134179
5ae32b7
6ddb833
507ca74
f5caf82
ce75759
f3f75a2
0c7b917
0c0c76c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
package io.sentry.opentelemetry; | ||
|
||
import io.opentelemetry.api.common.Attributes; | ||
import io.opentelemetry.api.trace.SpanKind; | ||
import io.opentelemetry.semconv.SemanticAttributes; | ||
import io.sentry.DsnUtil; | ||
import io.sentry.IScopes; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Locale; | ||
import org.jetbrains.annotations.ApiStatus; | ||
import org.jetbrains.annotations.NotNull; | ||
import org.jetbrains.annotations.Nullable; | ||
|
||
@ApiStatus.Internal | ||
public final class OtelInternalSpanDetectionUtil { | ||
|
||
private static final @NotNull List<SpanKind> spanKindsConsideredForSentryRequests = | ||
Arrays.asList(SpanKind.CLIENT, SpanKind.INTERNAL); | ||
|
||
@SuppressWarnings("deprecation") | ||
public static boolean isSentryRequest( | ||
final @NotNull IScopes scopes, | ||
final @NotNull SpanKind spanKind, | ||
final @NotNull Attributes attributes) { | ||
if (!spanKindsConsideredForSentryRequests.contains(spanKind)) { | ||
return false; | ||
} | ||
|
||
final @Nullable String httpUrl = attributes.get(SemanticAttributes.HTTP_URL); | ||
if (DsnUtil.urlContainsDsnHost(scopes.getOptions(), httpUrl)) { | ||
return true; | ||
} | ||
|
||
final @Nullable String fullUrl = attributes.get(SemanticAttributes.URL_FULL); | ||
if (DsnUtil.urlContainsDsnHost(scopes.getOptions(), fullUrl)) { | ||
return true; | ||
} | ||
|
||
// TODO [POTEL] should check if enabled but multi init with different options makes testing hard | ||
// atm | ||
// if (scopes.getOptions().isEnableSpotlight()) { | ||
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; | ||
} | ||
// } | ||
|
||
return false; | ||
} | ||
|
||
private static boolean containsSpotlightUrl( | ||
final @Nullable String requestUrl, final @NotNull String spotlightUrl) { | ||
if (requestUrl == null) { | ||
return false; | ||
} | ||
|
||
return requestUrl.toLowerCase(Locale.ROOT).contains(spotlightUrl.toLowerCase(Locale.ROOT)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,9 @@ | |
|
||
import static io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME; | ||
import static io.sentry.opentelemetry.InternalSemanticAttributes.IS_REMOTE_PARENT; | ||
import static io.sentry.opentelemetry.OtelInternalSpanDetectionUtil.isSentryRequest; | ||
|
||
import io.opentelemetry.api.common.Attributes; | ||
import io.opentelemetry.api.trace.SpanKind; | ||
import io.opentelemetry.api.trace.StatusCode; | ||
import io.opentelemetry.sdk.common.CompletableResultCode; | ||
import io.opentelemetry.sdk.trace.data.SpanData; | ||
|
@@ -14,7 +14,6 @@ | |
import io.sentry.Baggage; | ||
import io.sentry.DateUtils; | ||
import io.sentry.DefaultSpanFactory; | ||
import io.sentry.DsnUtil; | ||
import io.sentry.IScopes; | ||
import io.sentry.ISpan; | ||
import io.sentry.ITransaction; | ||
|
@@ -37,7 +36,6 @@ | |
import java.util.Collection; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Locale; | ||
import java.util.Map; | ||
import java.util.concurrent.CopyOnWriteArrayList; | ||
import java.util.function.Predicate; | ||
|
@@ -54,9 +52,6 @@ public final class SentrySpanExporter implements SpanExporter { | |
new SpanDescriptionExtractor(); | ||
private final @NotNull IScopes scopes; | ||
|
||
private final @NotNull List<SpanKind> spanKindsConsideredForSentryRequests = | ||
Arrays.asList(SpanKind.CLIENT, SpanKind.INTERNAL); | ||
|
||
private final @NotNull List<String> attributeKeysToRemove = | ||
Arrays.asList( | ||
InternalSemanticAttributes.IS_REMOTE_PARENT.getKey(), | ||
|
@@ -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 commentThe reason will be displayed to describe this comment to others. Learn more. Are there instances where There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
return spans.stream().filter((span) -> !isSentryRequest(span)).collect(Collectors.toList()); | ||
} | ||
|
||
@SuppressWarnings("deprecation") | ||
private boolean isSentryRequest(final @NotNull SpanData spanData) { | ||
final @NotNull SpanKind kind = spanData.getKind(); | ||
if (!spanKindsConsideredForSentryRequests.contains(kind)) { | ||
return false; | ||
} | ||
|
||
final @Nullable String httpUrl = spanData.getAttributes().get(SemanticAttributes.HTTP_URL); | ||
if (DsnUtil.urlContainsDsnHost(scopes.getOptions(), httpUrl)) { | ||
return true; | ||
} | ||
|
||
final @Nullable String fullUrl = spanData.getAttributes().get(SemanticAttributes.URL_FULL); | ||
if (DsnUtil.urlContainsDsnHost(scopes.getOptions(), fullUrl)) { | ||
return true; | ||
} | ||
|
||
// TODO [POTEL] should check if enabled but multi init with different options makes testing hard | ||
// atm | ||
// if (scopes.getOptions().isEnableSpotlight()) { | ||
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; | ||
} | ||
// } | ||
|
||
return false; | ||
} | ||
|
||
private boolean containsSpotlightUrl( | ||
final @Nullable String requestUrl, final @NotNull String spotlightUrl) { | ||
if (requestUrl == null) { | ||
return false; | ||
} | ||
|
||
return requestUrl.toLowerCase(Locale.ROOT).contains(spotlightUrl.toLowerCase(Locale.ROOT)); | ||
return spans.stream() | ||
.filter((span) -> !isSentryRequest(scopes, span.getKind(), span.getAttributes())) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
private List<SpanData> maybeSend(final @NotNull List<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.
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.