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 5 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

* Feat: OkHttp callback for Customising the Span (#1478)

# 5.0.0-beta.4

* Fix: Enrich Transactions with Context Data (#1469)
Expand Down
4 changes: 2 additions & 2 deletions sentry-android-okhttp/api/sentry-android-okhttp.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ public final class io/sentry/android/okhttp/BuildConfig {

public final class io/sentry/android/okhttp/SentryOkHttpInterceptor : okhttp3/Interceptor {
public fun <init> ()V
public fun <init> (Lio/sentry/IHub;)V
public synthetic fun <init> (Lio/sentry/IHub;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Lio/sentry/IHub;Lkotlin/jvm/functions/Function1;)V
public synthetic fun <init> (Lio/sentry/IHub;Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun intercept (Lokhttp3/Interceptor$Chain;)Lokhttp3/Response;
}

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 beforeSpan: (span: ISpan) -> Unit = { }
) : Interceptor {

override fun intercept(chain: Interceptor.Chain): Response {
Expand Down Expand Up @@ -39,6 +41,7 @@ class SentryOkHttpInterceptor(
}
throw e
} finally {
span?.apply(beforeSpan)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way we can drop the span? Similar to how we do beforeSend and beforeBreadcrumb?

We would also need to pass some context to the callback, like the request (and resposne?) objects.

Some context: We're trying to add a way for a user to do:

if (request.url == "/health" || response.status == 200) {
return null;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point about dropping the span. url and status can be obtained from span but in less convenient way than with plain request and response. I'll change the implementation.

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,
beforeSpan: ((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 (beforeSpan != null) {
interceptor = SentryOkHttpInterceptor(hub, beforeSpan)
}
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(beforeSpan = {
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)
}
}