Add library instrumentation for ktor server #4983
Conversation
anuraaga
requested review from
iNikem,
jkwatson,
laurit,
mateuszrzeszutek,
pavolloffay,
trask and
tylerbenson
as code owners
| import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming | ||
| import kotlinx.coroutines.withContext | ||
|
|
||
| class KtorServerTracing private constructor( |
There was a problem hiding this comment.
anuraaga
force-pushed
the
ktor-server
branch
from
e9accdc
to
46a0d54
Compare
anuraaga
changed the title
| pipeline.environment.monitor.subscribe(Routing.RoutingCallStarted) { call -> | ||
| val context = call.attributes.getOrNull(contextKey) | ||
| if (context != null) { | ||
| ServerSpanNaming.updateServerSpanName(context, ServerSpanNaming.Source.SERVLET, { _, arg -> arg.route.parent.toString() }, call) |
There was a problem hiding this comment.
Should this set http.route attribute as well?
There was a problem hiding this comment.
Yeah I think so - do you think we should do it here or should ServerSpanName, or a similar helper, set them both?
There was a problem hiding this comment.
I believe we discussed this already somewhere... I don't remember our decision :(
There was a problem hiding this comment.
http.route is going to be set by ServerSpanNaming (well, once I find enough time to implement that...)
|
|
||
| internal val additionalExtractors = mutableListOf<AttributesExtractor<in ApplicationRequest, in ApplicationResponse>>() | ||
|
|
||
| internal var statusExtractor: |
There was a problem hiding this comment.
I don't understand why is this a function and not just an instance of SpanStatusExtractor?
There was a problem hiding this comment.
This is the same pattern as many of the tracing builders. It's because it's very common to want to customize one specific case and then delegate to default for others.
| pipeline.environment.monitor.subscribe(Routing.RoutingCallStarted) { call -> | ||
| val context = call.attributes.getOrNull(contextKey) | ||
| if (context != null) { | ||
| ServerSpanNaming.updateServerSpanName(context, ServerSpanNaming.Source.SERVLET, { _, arg -> arg.route.parent.toString() }, call) |
There was a problem hiding this comment.
http.route is going to be set by ServerSpanNaming (well, once I find enough time to implement that...)
For #4972
This seemed like a fun instrumentation to try to get more practice with Kotlin. It wasn't fun when trying to integrate it with Spock :P