Ordering of tracing "interceptors"

[trask]

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:

• interceptors are configured "globally", and are not a replacement for "on response" callbacks, which are used to implement call-site specific behavior
• interceptors are part of the request/response "pipeline"
• interceptors can mutate the response, so the response is not really "complete" until it passes back through the interceptors

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.

• grpc client - looks like it needs to be updated (since grpc interceptors are run in reverse order):
  

  opentelemetry-java-instrumentation/instrumentation/grpc-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_5/GrpcClientBuilderBuildInstrumentation.java

  Line 48 in 9637d29

  interceptors.add(0, GrpcSingletons.CLIENT_INTERCEPTOR);

• aws-sdk 2.2: @anuraaga I don't see how to control ordering of these since they are added via SPI?
• aws-sdk-1.11, okhttp-2.2, okhttp-3.0, jaxws-2.0-cxf-3.0, netty

And we should add tests to verify this behavior, similar to the tests added for okhttp3 in #2894.

[anuraaga]

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.

[trask]

Confusingly enough, server interceptors are excuted in reverse while client ones are executed in order.

🤯

[agoallikmaa]

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:

• If interceptors are within the scope of the client span:
  • Important request properties may be changed by the interceptors (host, path, method - pretty much anything), meaning that CLIENT and SERVER span details won't match. Same for response properties.
  • Interceptors may also invoke the rest of the chain any number of times - meaning it may make multiple requests or none at all. This may cause multiple SERVER spans to exist for the same CLIENT span, or no SERVER span to exist and there is no indication from the CLIENT span that actually no real request went out.
• If interceptors are outside the scope of the client span:
  • If interceptors cause additional spans to be created, those will be siblings to the the CLIENT span, even though they can be considered to be part of the request.
  • Not possible to access span context in interceptors for purposes such as logging.

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.

[trask]

Closing, can revisit later if anyone has renewed interest.