opentelemetry-js-contrib
opentelemetry-js-contrib copied to clipboard
fix: Make Spans initiated in Instrumentation for HTTP Frameworks of Kind Server
Which problem is this PR solving?
The Span Kind in the all the HTTP framework instrumentations is not specified, making it internal by default.
A lot of providers, including Azure, is not reporting exceptions events in the spans if marked as internal.
I think that this behavior is intentional.
AFAIK in this context only http spans are supposed to be SpanKind.SERVER (these are generated by @opentelemetry/instrumentation-http). The extra spans generated by @opentelemetry/instrumentation-express, @opentelemetry/instrumentation-fastify, @opentelemetry/instrumentation-restify are all SpanKind.INTERNAL becaue they don't directly communicate with something else.
So any kind of a middleware span is an internal span (it's neither server nor client, it's in the middle -> internal).
From the spec:
- SERVER Indicates that the span covers server-side handling of a synchronous RPC or other remote request. This span is often the child of a remote CLIENT span that was expected to wait for a response.
- CLIENT Indicates that the span describes a request to some remote service. This span is usually the parent of a remote SERVER span and does not end until the response is received. [...]
- INTERNAL - Default value. Indicates that the span represents an internal operation within an application, as opposed to an operations with remote parents or children.
this applies here, the middleware span does not have any remote parents or children, only the http span does.
I think that this behavior is intentional.
I agree.
@XVincentX Does pichlermarc's explanation above make sense?
Can you provide more details on:
A lot of providers, including Azure, is not reporting exceptions events in the spans if marked as internal.
@trentm I am no expert enough in OpenTelemetry to say whether that is correct or not - I thought it was a bug, but I believe your assessment.
Can you provide more details
https://github.com/Azure/azure-sdk-for-js/blob/f4d9a79bc07cc538b0867ede7760da15d0cb523a/sdk/monitor/monitor-opentelemetry-exporter/src/utils/spanUtils.ts#L336
I believe this may be an issue that's better suited for the Azure SDK repo :thinking: It's seems related to the mapping of OTel to Azure Monitor, which is out of the realm of our expertise.
cc @hectorhdzg IIRC you're working on the JS Azure SDK, right? :thinking:
@XVincentX can you describe your scenario and what exactly are you trying to achieve in Azure Monitor side? are you expecting to see all instrumentation errors in your exception table in Application Insights? I don't believe any instrumentation different to HTTP currently call recordException in the Span, so even making the span internal you will not get exception telemetry as you are expecting.