sentry-java icon indicating copy to clipboard operation
sentry-java copied to clipboard

Performance powered by OpenTelemetry (POTel)

Open adinauer opened this issue 1 year ago • 1 comments

We want to use OpenTelemetry for the Sentry Java SDK. It should be possible to use OpenTelemetry API as well as Sentry API and still have both end up in Sentry.

H:

  • [x] Restore instrumenter code in Scopes https://github.com/getsentry/sentry-java/pull/3697
  • [x] Test updating op / description (op and description might have been overriden in SentrySpanExporter:204)

M:

  • [ ] Offer createSpan in Sentry API
  • [ ] Offer createInactiveSpan in Sentry API
  • [ ] Make SentryTracer.makeCurrent work
  • [ ] Implement createSpan for default span factory
  • [ ] Implement some magic to enable OTel span factory automatically when OTel is available (+ opt out of magic?)
  • [ ] Add a more complex service using Spring Boot to https://github.com/getsentry/opentelemetry-demo and showcase errors and other features Sentry has to offer (copied from #2419)
  • [ ] End2End Tests on Running application (copied from #2419)
  • [ ] Add a provider for SentryPropagator, see https://github.com/getsentry/sentry-java/issues/2254#issuecomment-1575492805

Figure out whether to include these in v8:

  • [ ] Add setAttribute methods and deprecate old API (maybe deprecate in v8, remove in v9 likely)
    • [ ] tbd when we actually want to do this
  • [ ] Make old API write to span attributes, tbd if we actually want to do this and how this should work in detail
  • [ ] should we clear out old finished spans after a while in SentrySpanExporter?

L:

  • [ ] is detecting a version mismatch between application and agent possible to warn the user
  • [ ] Create transaction in DefaultSpanFactory.createSpan
  • [ ] Fix classloader for sentry-opentelemetry-bootstrap module to actually be bootstrap classloader
    • [ ] This is currently loaded via the agent classloader due to implementation(projects.sentryOpentelemetry.sentryOpentelemetryBootstrap) in sentry-opentelemetry-agentcustomization/build.gradle
    • [ ] Fixing this causes ClassNotFound exception in OtelSpanWrapper due to ReadWriteSpan not being available in the bootstrap classloader. We have to figure out how to handle this. OTel probably already has a workaround for this. Likely using shading and proxy objects. needs investigation
  • [ ] Maybe optimize context forking happening too often when already wrapped but rewrapped by OTel ArrayBaseContext
  • [ ] Make measurements work
  • [ ] Make metrics work
  • [ ] Check if we want to support span Contexts by writing to root span or mark it internal as it currently wouldn't work for non root spans
  • [ ] What should transaction.finish do for OTel? Finish all child spans? Root span could hold a list of children (weakly)
  • [ ] Make scheduleFinish work
  • [ ] Make finish callback work
  • [ ] Make finish options work (dropIfNoChildren, Hint)
  • [ ] Make forceFinish work
  • [ ] Make getSpans work
  • [ ] Implement getEventId on OTel transaction wrapper
    • [ ] In theory we shouldn't be using getEventId() as only the SentryTracer instance created by SentrySpanExporter should be queried for its event id. In theory a customer could call getEventId on the OtelTransactionSpanForwarder returned by startTransaction. This ID wouldn't match the real event ID. We'd have to force the ID used by SentryTracer, e.g. via SpanOptions or similar or simply use span ID (+ maybe trace ID concatenated).
  • [ ] Once the more complex demo has been merged, revert the adservice close to original state and showcase how easy it is to use Sentry for OTEL even without changing the target application

Subtasks:

  • [x] Tracing #3445 and #3455
    • [x] Continue an incoming trace
    • [x] Attach trace to outgoing requests
    • [x] Make Sentry API for retrieving baggage etc. work
    • [x] Make Sentry API for continuing a trace work
  • [x] Sentry Span op and OTel span name can be updated independent of each other and diverge, fix #3468
  • [x] ~~Configurable POTel vs legacy OTel mode (e.g. by having a separate JAR)~~
  • [x] Sentry status should use OTel status #3439
  • [x] Make tracesSamplingDecision work #3462
  • [x] Figure out what to do with instrumenter - it is no longer needed
  • [x] How do we want to deal with multi init and OTel [#3674], [#3675], [#3676]
  • [x] Make startTimestamp on SpanOptions work for non OTel as well, maybe get rid of startChild overloads with startTimestamp and suggest using SpanOptions instead #3498
  • [x] Record dropped spans in SentrySampler via client reports #3552
  • [x] Set instrumenter (e.g. using an Aspect) to OTEL from the Java Agent so users don't have to manually do it (now obsolete, due to the agent now taking precedence)
  • [ ] ~~Detect sentry-opentelemetry-agent and set instrumenter automatically even when not using auto config (very similar to above)~~
For DACI
  • [ ] Clarify how OTel can be used directly
    • [ ] e.g. to set an attribute: Span.current().setAttribute(...) should work for 99% of cases (only if the span isn't made current / bound to scope customers would have to access it differently)
    • [ ] describe a way to retrieve the OTel span if it's not the current one
      • [ ] maybe expose a getOtelSpan() on Sentry span but that would drag OTel dependency into Android and hybrid SDKs
      • [ ] while implementing OTels Span interface would be possible we'd be dragging OTel into Android and hybrid SDKs, which we likely don't want
      • [ ] best approach is likely to simply add setAttribute methods that forward to the OTel span
        • [ ] where do these go to for non OTel mode (i.e. SentryTracer, Span)?

adinauer avatar May 21 '24 08:05 adinauer