Skip to content

Commit 762ee2d

Browse files
authored
Also use port when checking if a request is made to Sentry DSN (#4231)
* Also use port when checking if a request is made to Sentry DSN * changelog
1 parent 033bc88 commit 762ee2d

File tree

7 files changed

+304
-17
lines changed

7 files changed

+304
-17
lines changed

CHANGELOG.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
## Unreleased
44

5+
### Behavioural Changes
6+
7+
- Use `java.net.URI` for parsing URLs in `UrlUtils` ([#4210](https://github.com/getsentry/sentry-java/pull/4210))
8+
- This could affect grouping for issues with messages containing URLs that fall in known corner cases that were handled incorrectly previously (e.g. email in URL path)
9+
510
### Fixes
611

712
- Add support for setting in-app-includes/in-app-excludes via AndroidManifest.xml ([#4240](https://github.com/getsentry/sentry-java/pull/4240))
@@ -15,10 +20,11 @@
1520
- Set `sentry.capture-open-telemetry-events=true` in Springs `application.properties` to enable it
1621
- Set `sentry.captureOpenTelemetryEvents: true` in Springs `application.yml` to enable it
1722

18-
### Behavioural Changes
23+
### Internal
1924

20-
- Use `java.net.URI` for parsing URLs in `UrlUtils` ([#4210](https://github.com/getsentry/sentry-java/pull/4210))
21-
- This could affect grouping for issues with messages containing URLs that fall in known corner cases that were handled incorrectly previously (e.g. email in URL path)
25+
- Also use port when checking if a request is made to Sentry DSN ([#4231](https://github.com/getsentry/sentry-java/pull/4231))
26+
- For our OpenTelemetry integration we check if a span is for a request to Sentry
27+
- We now also consider the port when performing this check
2228

2329
### Dependencies
2430

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OpenTelemetryAttributesExtractor.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,20 @@ private static Map<String, String> collectHeaders(
105105
return headers;
106106
}
107107

108+
@SuppressWarnings("deprecation")
108109
public @Nullable String extractUrl(
109110
final @NotNull Attributes attributes, final @NotNull SentryOptions options) {
110111
final @Nullable String urlFull = attributes.get(UrlAttributes.URL_FULL);
111112
if (urlFull != null) {
112113
return urlFull;
113114
}
114115

116+
final @Nullable String deprecatedUrl =
117+
attributes.get(io.opentelemetry.semconv.SemanticAttributes.HTTP_URL);
118+
if (deprecatedUrl != null) {
119+
return deprecatedUrl;
120+
}
121+
115122
final String urlString = buildUrlString(attributes, options);
116123
if (!urlString.isEmpty()) {
117124
return urlString;

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelInternalSpanDetectionUtil.java

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import io.opentelemetry.api.common.Attributes;
44
import io.opentelemetry.api.trace.SpanKind;
5-
import io.opentelemetry.semconv.UrlAttributes;
65
import io.sentry.DsnUtil;
76
import io.sentry.IScopes;
87
import java.util.Arrays;
@@ -17,6 +16,8 @@ public final class OtelInternalSpanDetectionUtil {
1716

1817
private static final @NotNull List<SpanKind> spanKindsConsideredForSentryRequests =
1918
Arrays.asList(SpanKind.CLIENT, SpanKind.INTERNAL);
19+
private static final @NotNull OpenTelemetryAttributesExtractor attributesExtractor =
20+
new OpenTelemetryAttributesExtractor();
2021

2122
@SuppressWarnings("deprecation")
2223
public static boolean isSentryRequest(
@@ -27,14 +28,8 @@ public static boolean isSentryRequest(
2728
return false;
2829
}
2930

30-
final @Nullable String httpUrl =
31-
attributes.get(io.opentelemetry.semconv.SemanticAttributes.HTTP_URL);
32-
if (DsnUtil.urlContainsDsnHost(scopes.getOptions(), httpUrl)) {
33-
return true;
34-
}
35-
36-
final @Nullable String fullUrl = attributes.get(UrlAttributes.URL_FULL);
37-
if (DsnUtil.urlContainsDsnHost(scopes.getOptions(), fullUrl)) {
31+
String url = attributesExtractor.extractUrl(attributes, scopes.getOptions());
32+
if (DsnUtil.urlContainsDsnHost(scopes.getOptions(), url)) {
3833
return true;
3934
}
4035

@@ -43,10 +38,7 @@ public static boolean isSentryRequest(
4338
final @NotNull String spotlightUrl =
4439
optionsSpotlightUrl != null ? optionsSpotlightUrl : "http://localhost:8969/stream";
4540

46-
if (containsSpotlightUrl(fullUrl, spotlightUrl)) {
47-
return true;
48-
}
49-
if (containsSpotlightUrl(httpUrl, spotlightUrl)) {
41+
if (containsSpotlightUrl(url, spotlightUrl)) {
5042
return true;
5143
}
5244
}

sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OpenTelemetryAttributesExtractorTest.kt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import io.opentelemetry.api.common.AttributeKey
44
import io.opentelemetry.sdk.internal.AttributesMap
55
import io.opentelemetry.sdk.trace.data.SpanData
66
import io.opentelemetry.semconv.HttpAttributes
7+
import io.opentelemetry.semconv.SemanticAttributes
78
import io.opentelemetry.semconv.ServerAttributes
89
import io.opentelemetry.semconv.UrlAttributes
910
import io.sentry.Scope
@@ -202,6 +203,19 @@ class OpenTelemetryAttributesExtractorTest {
202203
assertEquals("https://sentry.io/some/path", url)
203204
}
204205

206+
@Test
207+
fun `returns deprecated URL if present`() {
208+
givenAttributes(
209+
mapOf(
210+
SemanticAttributes.HTTP_URL to "https://sentry.io/some/path"
211+
)
212+
)
213+
214+
val url = whenExtractingUrl()
215+
216+
assertEquals("https://sentry.io/some/path", url)
217+
}
218+
205219
@Test
206220
fun `returns reconstructed URL if attributes present`() {
207221
givenAttributes(
Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
package io.sentry.opentelemetry
2+
3+
import io.opentelemetry.api.common.AttributeKey
4+
import io.opentelemetry.api.trace.SpanKind
5+
import io.opentelemetry.sdk.internal.AttributesMap
6+
import io.opentelemetry.semconv.HttpAttributes
7+
import io.opentelemetry.semconv.SemanticAttributes
8+
import io.opentelemetry.semconv.ServerAttributes
9+
import io.opentelemetry.semconv.UrlAttributes
10+
import io.sentry.IScopes
11+
import io.sentry.SentryOptions
12+
import org.mockito.kotlin.mock
13+
import org.mockito.kotlin.whenever
14+
import kotlin.test.Test
15+
import kotlin.test.assertFalse
16+
import kotlin.test.assertTrue
17+
18+
class OtelInternalSpanDetectionUtilTest {
19+
20+
private class Fixture {
21+
val scopes = mock<IScopes>()
22+
val attributes = AttributesMap.create(100, 100)
23+
val options = SentryOptions.empty()
24+
var spanKind: SpanKind = SpanKind.INTERNAL
25+
26+
init {
27+
whenever(scopes.options).thenReturn(options)
28+
}
29+
}
30+
31+
private val fixture = Fixture()
32+
33+
@Test
34+
fun `detects split url as internal (span kind client)`() {
35+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
36+
givenSpanKind(SpanKind.CLIENT)
37+
givenAttributes(
38+
mapOf(
39+
HttpAttributes.HTTP_REQUEST_METHOD to "GET",
40+
UrlAttributes.URL_SCHEME to "https",
41+
UrlAttributes.URL_PATH to "/path/to/123",
42+
UrlAttributes.URL_QUERY to "q=123456&b=X",
43+
ServerAttributes.SERVER_ADDRESS to "io.sentry",
44+
ServerAttributes.SERVER_PORT to 8081L
45+
)
46+
)
47+
48+
thenRequestIsConsideredInternal()
49+
}
50+
51+
@Test
52+
fun `detects full url as internal (span kind client)`() {
53+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
54+
givenSpanKind(SpanKind.CLIENT)
55+
givenAttributes(
56+
mapOf(
57+
UrlAttributes.URL_FULL to "https://io.sentry:8081"
58+
)
59+
)
60+
61+
thenRequestIsConsideredInternal()
62+
}
63+
64+
@Test
65+
fun `detects deprecated url as internal (span kind client)`() {
66+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
67+
givenSpanKind(SpanKind.CLIENT)
68+
givenAttributes(
69+
mapOf(
70+
SemanticAttributes.HTTP_URL to "https://io.sentry:8081"
71+
)
72+
)
73+
74+
thenRequestIsConsideredInternal()
75+
}
76+
77+
@Test
78+
fun `detects split url as internal (span kind internal)`() {
79+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
80+
givenSpanKind(SpanKind.INTERNAL)
81+
givenAttributes(
82+
mapOf(
83+
HttpAttributes.HTTP_REQUEST_METHOD to "GET",
84+
UrlAttributes.URL_SCHEME to "https",
85+
UrlAttributes.URL_PATH to "/path/to/123",
86+
UrlAttributes.URL_QUERY to "q=123456&b=X",
87+
ServerAttributes.SERVER_ADDRESS to "io.sentry",
88+
ServerAttributes.SERVER_PORT to 8081L
89+
)
90+
)
91+
92+
thenRequestIsConsideredInternal()
93+
}
94+
95+
@Test
96+
fun `detects full url as internal (span kind internal)`() {
97+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
98+
givenSpanKind(SpanKind.INTERNAL)
99+
givenAttributes(
100+
mapOf(
101+
UrlAttributes.URL_FULL to "https://io.sentry:8081"
102+
)
103+
)
104+
105+
thenRequestIsConsideredInternal()
106+
}
107+
108+
@Test
109+
fun `detects deprecated url as internal (span kind internal)`() {
110+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
111+
givenSpanKind(SpanKind.INTERNAL)
112+
givenAttributes(
113+
mapOf(
114+
SemanticAttributes.HTTP_URL to "https://io.sentry:8081"
115+
)
116+
)
117+
118+
thenRequestIsConsideredInternal()
119+
}
120+
121+
@Test
122+
fun `does not detect full url as internal (span kind server)`() {
123+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
124+
givenSpanKind(SpanKind.SERVER)
125+
givenAttributes(
126+
mapOf(
127+
UrlAttributes.URL_FULL to "https://io.sentry:8081"
128+
)
129+
)
130+
131+
thenRequestIsNotConsideredInternal()
132+
}
133+
134+
@Test
135+
fun `does not detect full url as internal (span kind producer)`() {
136+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
137+
givenSpanKind(SpanKind.PRODUCER)
138+
givenAttributes(
139+
mapOf(
140+
UrlAttributes.URL_FULL to "https://io.sentry:8081"
141+
)
142+
)
143+
144+
thenRequestIsNotConsideredInternal()
145+
}
146+
147+
@Test
148+
fun `does not detect full url as internal (span kind consumer)`() {
149+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
150+
givenSpanKind(SpanKind.CONSUMER)
151+
givenAttributes(
152+
mapOf(
153+
UrlAttributes.URL_FULL to "https://io.sentry:8081"
154+
)
155+
)
156+
157+
thenRequestIsNotConsideredInternal()
158+
}
159+
160+
@Test
161+
fun `detects full spotlight url as internal`() {
162+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
163+
givenSpotlightEnabled(true)
164+
givenSpanKind(SpanKind.CLIENT)
165+
givenAttributes(
166+
mapOf(
167+
UrlAttributes.URL_FULL to "http://localhost:8969/stream"
168+
)
169+
)
170+
171+
thenRequestIsConsideredInternal()
172+
}
173+
174+
@Test
175+
fun `detects full spotlight url as internal with custom spotlight url`() {
176+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
177+
givenSpotlightEnabled(true)
178+
givenSpotlightUrl("http://localhost:8090/stream")
179+
givenSpanKind(SpanKind.CLIENT)
180+
givenAttributes(
181+
mapOf(
182+
UrlAttributes.URL_FULL to "http://localhost:8090/stream"
183+
)
184+
)
185+
186+
thenRequestIsConsideredInternal()
187+
}
188+
189+
@Test
190+
fun `does not detect mismatching full spotlight url as internal`() {
191+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
192+
givenSpotlightEnabled(true)
193+
givenSpanKind(SpanKind.CLIENT)
194+
givenAttributes(
195+
mapOf(
196+
UrlAttributes.URL_FULL to "http://localhost:8080/stream"
197+
)
198+
)
199+
200+
thenRequestIsNotConsideredInternal()
201+
}
202+
203+
@Test
204+
fun `does not detect mismatching full customized spotlight url as internal`() {
205+
givenDsn("https://publicKey:secretKey@io.sentry:8081/path/id?sample.rate=0.1")
206+
givenSpotlightEnabled(true)
207+
givenSpotlightUrl("http://localhost:8090/stream")
208+
givenSpanKind(SpanKind.CLIENT)
209+
givenAttributes(
210+
mapOf(
211+
UrlAttributes.URL_FULL to "http://localhost:8091/stream"
212+
)
213+
)
214+
215+
thenRequestIsNotConsideredInternal()
216+
}
217+
218+
private fun givenAttributes(map: Map<AttributeKey<out Any>, Any>) {
219+
map.forEach { k, v ->
220+
fixture.attributes.put(k, v)
221+
}
222+
}
223+
224+
private fun givenDsn(dsn: String) {
225+
fixture.options.dsn = dsn
226+
}
227+
228+
private fun givenSpotlightEnabled(enabled: Boolean) {
229+
fixture.options.isEnableSpotlight = enabled
230+
}
231+
232+
private fun givenSpotlightUrl(url: String) {
233+
fixture.options.spotlightConnectionUrl = url
234+
}
235+
236+
private fun givenSpanKind(spanKind: SpanKind) {
237+
fixture.spanKind = spanKind
238+
}
239+
240+
private fun thenRequestIsConsideredInternal() {
241+
assertTrue(checkIfInternal())
242+
}
243+
244+
private fun thenRequestIsNotConsideredInternal() {
245+
assertFalse(checkIfInternal())
246+
}
247+
248+
private fun checkIfInternal(): Boolean {
249+
return OtelInternalSpanDetectionUtil.isSentryRequest(fixture.scopes, fixture.spanKind, fixture.attributes)
250+
}
251+
}

sentry/src/main/java/io/sentry/DsnUtil.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ public static boolean urlContainsDsnHost(@Nullable SentryOptions options, @Nulla
3131
return false;
3232
}
3333

34-
return url.toLowerCase(Locale.ROOT).contains(dsnHost.toLowerCase(Locale.ROOT));
34+
final @NotNull String lowerCaseHost = dsnHost.toLowerCase(Locale.ROOT);
35+
final int dsnPort = sentryUri.getPort();
36+
37+
if (dsnPort > 0) {
38+
return url.toLowerCase(Locale.ROOT).contains(lowerCaseHost + ":" + dsnPort);
39+
} else {
40+
return url.toLowerCase(Locale.ROOT).contains(lowerCaseHost);
41+
}
3542
}
3643
}

0 commit comments

Comments
 (0)