-
Notifications
You must be signed in to change notification settings - Fork 850
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.
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
Ordering of tracing "interceptors" #2923
Comments
Another point for the summary is we've had user requests to have the span available in interceptors (e.g., to inject into log messages in the interceptor). Think grpc is ok. Confusingly enough, server interceptors are excuted in reverse while client ones are executed in order. Controlling the ordering is probably a reasonable reason for us to switch aws sdk from using the SPI to bytecode manipulation, oh well. |
🤯 |
From the perspective of OkHttp specifically, I think there are certain drawbacks to either order. This may or may not apply to interceptors of other clients depending on their flexibility:
The only way I see to not have the downsides of either case would be to actually have two nested spans - one has the request/response details that the application code requested/received, and the inner one (or ones) would correspond to actual request/response that was passed over the network. I don't have a suggestion for how to fit it in the current specification, but perhaps this is an idea worth considering. |
Discussed with @anuraaga in today's SIG meeting about #2894 (comment), and he convinced me that interceptors are different from "on response" callbacks, and deserve different treatment.
In summary:
So suggest the rule of thumb that we apply (as the default behavior), is for CLIENT spans to capture everything from the beginning of the request to once the response is completed (which includes passing through interceptors which are allowed to modify the response object).
We should review existing instrumentation that applies to "interceptors", e.g.
opentelemetry-java-instrumentation/instrumentation/grpc-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_5/GrpcClientBuilderBuildInstrumentation.java
Line 48 in 9637d29
And we should add tests to verify this behavior, similar to the tests added for okhttp3 in #2894.
The text was updated successfully, but these errors were encountered: