Skip to content
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

ClientCore tracing zero dependencies, no plugins #43346

Merged
merged 32 commits into from
Jan 7, 2025

Conversation

lmolkova
Copy link
Member

@lmolkova lmolkova commented Dec 11, 2024

This is an early prototype of possible tracing implementation in clientcore following slf4j shim philosophy:

  • it has an ultra thin API wrapper around Otel API, mimicking all the API that we're going to use
  • it's based on reflection making otel dependency fully optional (it never appears as a dependency, but when on the classpath it just works)
  • no plugin necessary

TODOs in this PR:

  • Configurability by users and libs
  • proper instantiation
  • full test coverage
  • optimization to back off and turn tracing off if something goes wrong
  • internal logging
  • benchmark - compare with direct calls
    • check if there is a better way than MethodHandle.invoke
  • agent and shading+relocation might need some additional tricks in reflection code, but seem doable
  • url sanitization
  • explicit inproc context propagation
    • polish API
  • suppression
  • code snippets and docs
  • W3C trace-context propagation over the wire

TODO (follow up PRs):

  • Relationship between HttpLogOptions and TelemetryOptions?
    • Maybe merge HttpLoggingPolicy and InstrumentationPolicy?
  • Move logging to telemetry package
  • Throw away client-request-id - replace with W3C trace context default impl
  • HTTP instrumentation policy
    • time-to-last-byte
  • Metrics
  • [Distant future maybe] Maven magic to compile with optional dependency on OTel, but not list it in the effective pom

TODO later, when necessary:

  • we still want to support SPI for tracers/meters, OTel is the default implementation, but another one can be implemented and plugged in
  • custom (non-w3c) context propagation
  • support links

@lmolkova
Copy link
Member Author

lmolkova commented Dec 25, 2024

Benchmarks (JAva 21):

Using ReflectiveInvoker.invokeWithArguments

Benchmark                                    Mode  Cnt     Score      Error  Units
TracingShimBenchmarks.directTracing          avgt   15   318.135 ±   84.421  ns/op
TracingShimBenchmarks.directTracingDisabled  avgt   15    34.609 ±    3.394  ns/op
TracingShimBenchmarks.shimTracing            avgt   15  3951.097 ± 1423.879  ns/op
TracingShimBenchmarks.shimTracingDisabled    avgt   15     2.499 ±    0.014  ns/op

using MethodHandle.invoke directly

invoke on method handle
Benchmark                                    Mode  Cnt    Score   Error  Units
TracingShimBenchmarks.directTracing          avgt   15  263.963 ± 0.616  ns/op
TracingShimBenchmarks.directTracingDisabled  avgt   15   15.714 ± 0.104  ns/op
TracingShimBenchmarks.shimTracing            avgt   15  337.979 ± 1.364  ns/op
TracingShimBenchmarks.shimTracingDisabled    avgt   15    2.473 ± 0.009  ns/op

Profiles:

image

( ReflectiveInvoker.invokeWithArguments )

image

( MethodHandle.invoke )

@lmolkova lmolkova changed the title PoC: Clientcore tracing zero dependencies, no plugins ClientCore tracing zero dependencies, no plugins Dec 27, 2024
@lmolkova lmolkova marked this pull request as ready for review December 27, 2024 00:20
sdk/clientcore/core/pom.xml Outdated Show resolved Hide resolved
@lmolkova
Copy link
Member Author

/azp run java - clientcore

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

io.clientcore:core

@lmolkova lmolkova force-pushed the client-core-tracing-shim branch from 46f32b3 to 5c170ea Compare January 7, 2025 00:04
@lmolkova lmolkova merged commit db428f2 into Azure:main Jan 7, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants