Add asynchronous tracing to Spring Boot Autoconfigure WithSpanAspect

[HaloFour]

Updates the WithSpanAspect in Spring Boot Autoconfigure module to support asynchronous return types. Refactors the MethodSpanStrategies implementations from OpenTelemetry Annotations module to enable sharing between instrumentation projects.

Open question: Should the async method strategy machinery belong in the instrumentation-api module or would it be better suited in a separate AOP module?

[trask]

👍

[trask]

maybe .async -> .future or .promise

.async feels a bit general

[HaloFour]

I'd probably gravitate to reactive but that's probably as general as async is. Of your suggestions I think future probably fits better in the Java ecosystem.

[mateuszrzeszutek]

WDYT about renaming MethodSpanStrategy/-ies to AsyncSpanEndStrategy/-ies? (or something similar that emphasises the asynchronous nature of this code)

[trask]

WDYT about renaming MethodSpanStrategy/-ies to AsyncSpanEndStrategy/-ies? (or something similar that emphasises the asynchronous nature of this code)

that sounds nice, and if we don't find an AsyncSpanEndStrategy, we can handle the default synchronous case without a strategy, e.g.

  public Object end(Context context, Class<?> returnType, Object returnValue) {
    ...
    AsyncSpanEndStrategy asyncSpanEndStrategy = asyncSpanEndStrategies.resolveStrategy(returnType);
    if (asyncSpanEndStrategy != null) {
      return asyncSpanEndStrategy.end(this, context, returnValue);
    }
    end(context);
    return returnValue;
  }

[HaloFour]

That sounds good to me. If the interface is AsyncSpanEndStrategy does it make sense for the package name to remain async then?

[trask]

If the interface is AsyncSpanEndStrategy does it make sense for the package name to remain async then?

Ya, let's keep it async unless anyone else has thoughts.

[trask]

@HaloFour looks like it needs a ./gradlew spotlessApply

[trask]

🚢

[HaloFour]

Shweet! Looking at the changes I think that maybe Jdk8AsyncEndStrategy could have been named Jdk8AsyncSpanEndStrategy instead. Refactor fumble on my part. I could open a follow up PR to change it. At least the class is package-private so it doesn't really matter.

[trask]

I could open a follow up PR to change it. At least the class is package-private so it doesn't really matter.

👍 (not a problem even if it was public, as none of the instrumentation APIs have been declared stable yet)