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.

Sign up for GitHub

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

Jump to bottom

Feat: OkHttp callback for Customising the Span #1478

Merged
merged 7 commits into from
May 25, 2021
Merged

Feat: OkHttp callback for Customising the Span #1478

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0a24fae
Feat: OkHttp callback for Customising the Span
maciejwalkowiak May 17, 2021
0d2610e
Changelog.
maciejwalkowiak May 17, 2021
9894775
Rename parameter name.
maciejwalkowiak May 19, 2021
73ad945
Merge branch 'main' into gh-1365
maciejwalkowiak May 19, 2021
7f532ba
Changelog.
maciejwalkowiak May 19, 2021
65dbc6f
Accept request and response in BeforeSpanCallback.
maciejwalkowiak May 24, 2021
6c95fbe
Merge branch 'main' into gh-1365
maciejwalkowiak May 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Unreleased

* Fix: Enrich Transactions with Context Data (#1469)
* Feat: OkHttp callback for Customising the Span (#1478)

# 5.0.0-beta.3

Expand Down
5 changes: 4 additions & 1 deletion sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ package io.sentry.android.okhttp
import io.sentry.Breadcrumb
import io.sentry.HubAdapter
import io.sentry.IHub
import io.sentry.ISpan
import io.sentry.SpanStatus
import java.io.IOException
import okhttp3.Interceptor
import okhttp3.Response

class SentryOkHttpInterceptor(
private val hub: IHub = HubAdapter.getInstance()
private val hub: IHub = HubAdapter.getInstance(),
private val spanCustomizer: (span: ISpan) -> Unit = { }
Copy link
Contributor

Choose a reason for hiding this comment

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

what's about using a name that sort of matches our beforeSend and beforeBreadcrumb callbacks?
eg beforeSpan? cc @bruno-garcia

I believe this could also be an interface already available in the sentry package, it may be reused in other integrations, I'd not add to options though, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

2nd thing is that using Unit isn't the best definition if using it on Java, but as OkHttp is already written in Kotlin, I guess it's just fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed with naming. I think Unit can stay.

) : Interceptor {

override fun intercept(chain: Interceptor.Chain): Response {
Expand Down Expand Up @@ -39,6 +41,7 @@ class SentryOkHttpInterceptor(
}
throw e
} finally {
span?.apply(spanCustomizer)
span?.finish()
val breadcrumb = Breadcrumb.http(request.url.toString(), request.method, code)
request.body?.contentLength().ifHasValidLength {
Expand Down
21 changes: 19 additions & 2 deletions sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.Breadcrumb
import io.sentry.IHub
import io.sentry.ISpan
import io.sentry.SentryOptions
import io.sentry.SentryTraceHeader
import io.sentry.SentryTracer
Expand All @@ -32,7 +33,7 @@ class SentryOkHttpInterceptorTest {

class Fixture {
val hub = mock<IHub>()
val interceptor = SentryOkHttpInterceptor(hub)
var interceptor = SentryOkHttpInterceptor(hub)
val server = MockWebServer()
val sentryTracer = SentryTracer(TransactionContext("name", "op"), hub)

Expand All @@ -44,7 +45,8 @@ class SentryOkHttpInterceptorTest {
isSpanActive: Boolean = true,
httpStatusCode: Int = 201,
responseBody: String = "success",
socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN
socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN,
spanCustomizer: ((span: ISpan) -> Unit)? = null
): OkHttpClient {
if (isSpanActive) {
whenever(hub.span).thenReturn(sentryTracer)
Expand All @@ -54,6 +56,9 @@ class SentryOkHttpInterceptorTest {
.setSocketPolicy(socketPolicy)
.setResponseCode(httpStatusCode))
server.start()
if (spanCustomizer != null) {
interceptor = SentryOkHttpInterceptor(hub, spanCustomizer)
}
return OkHttpClient.Builder().addInterceptor(interceptor).build()
}
}
Expand Down Expand Up @@ -159,4 +164,16 @@ class SentryOkHttpInterceptorTest {
assertEquals(SpanStatus.INTERNAL_ERROR, httpClientSpan.status)
assertTrue(httpClientSpan.throwable is IOException)
}

@Test
fun `customizer modifies span`() {
val sut = fixture.getSut(spanCustomizer = {
it.description = "overwritten description"
})
val request = getRequest()
sut.newCall(request).execute()
assertEquals(1, fixture.sentryTracer.children.size)
val httpClientSpan = fixture.sentryTracer.children.first()
assertEquals("overwritten description", httpClientSpan.description)
}
}