Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add description to OkHttp spans #3320

Merged
merged 5 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Add description to OkHttp spans ([#3320](https://github.com/getsentry/sentry-java/pull/3320))
- Update normalization of metrics keys, tags and values ([#3332](https://github.com/getsentry/sentry-java/pull/3332))
- Enable backpressure management by default ([#3284](https://github.com/getsentry/sentry-java/pull/3284))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
private var clientErrorResponse: Response? = null
private val isReadingResponseBody = AtomicBoolean(false)
private val isEventFinished = AtomicBoolean(false)
private val url: String
private val method: String

init {
val urlDetails = UrlUtils.parse(request.url.toString())
val url = urlDetails.urlOrFallback
url = urlDetails.urlOrFallback
val host: String = request.url.host
val encodedPath: String = request.url.encodedPath
val method: String = request.method
method = request.method

// We start the call span that will contain all the others
val parentSpan = if (Platform.isAndroid()) hub.transaction else hub.span
Expand Down Expand Up @@ -113,7 +115,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
fun startSpan(event: String) {
// Find the parent of the span being created. E.g. secureConnect is child of connect
val parentSpan = findParentSpan(event)
val span = parentSpan?.startChild("http.client.$event") ?: return
val span = parentSpan?.startChild("http.client.$event", "$method $url") ?: return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I'm wondering if we need to do anything to make PII scrubbing work here? (I don't think so).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright I just did a quick manual check. The span description definitely is processed and e.g. query params do get removed. All good!
image

if (event == RESPONSE_BODY_EVENT) {
// We save this event is reading the response body, so that it will not be auto-finished
isReadingResponseBody.set(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,28 +168,35 @@ class SentryOkHttpEventListenerTest {
}
1 -> {
assertEquals("http.client.proxy_select", span.operation)
assertEquals("GET ${request.url}", span.description)
assertNotNull(span.data["proxies"])
}
2 -> {
assertEquals("http.client.dns", span.operation)
assertEquals("GET ${request.url}", span.description)
assertNotNull(span.data["domain_name"])
assertNotNull(span.data["dns_addresses"])
}
3 -> {
assertEquals("http.client.connect", span.operation)
assertEquals("GET ${request.url}", span.description)
}
4 -> {
assertEquals("http.client.connection", span.operation)
assertEquals("GET ${request.url}", span.description)
}
5 -> {
assertEquals("http.client.request_headers", span.operation)
assertEquals("GET ${request.url}", span.description)
}
6 -> {
assertEquals("http.client.response_headers", span.operation)
assertEquals("GET ${request.url}", span.description)
assertEquals(201, span.data[SpanDataConvention.HTTP_STATUS_CODE_KEY])
}
7 -> {
assertEquals("http.client.response_body", span.operation)
assertEquals("GET ${request.url}", span.description)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ class SentryOkHttpEventTest {
assertNotNull(span)
assertTrue(spans.containsKey("span"))
assertEquals("http.client.span", span.operation)
assertEquals("${fixture.mockRequest.method} ${fixture.mockRequest.url}", span.description)
assertFalse(span.isFinished)
}

Expand Down Expand Up @@ -196,6 +197,7 @@ class SentryOkHttpEventTest {
sut.finishSpan("span") {
if (called == 0) {
assertEquals("http.client.span", it.operation)
assertEquals("${fixture.mockRequest.method} ${fixture.mockRequest.url}", it.description)
} else {
assertEquals(sut.callRootSpan, it)
}
Expand Down