Feat: OkHttp callback for Customising the Span #1478
Conversation
maciejwalkowiak
requested review from
bruno-garcia and
marandaneto
as code owners
| 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 = { } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Agreed with naming. I think Unit can stay.
|
as @bruno-garcia came up with this callback after talking to a few people, let's get his review before merging it, looks good to me. |
| @@ -39,6 +41,7 @@ class SentryOkHttpInterceptor( | |||
| } | |||
| throw e | |||
| } finally { | |||
| span?.apply(beforeSpan) | |||
There was a problem hiding this comment.
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;
There was a problem hiding this comment.
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.
📜 Description
OkHttp callback for Customising the Span
💡 Motivation and Context
Fixes #1365
💚 How did you test it?
Unit tests.
📝 Checklist