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

Performance powered by OpenTelemetry (POTel) #3436

Open
16 of 54 tasks
adinauer opened this issue May 21, 2024 · 1 comment
Open
16 of 54 tasks

Performance powered by OpenTelemetry (POTel) #3436

adinauer opened this issue May 21, 2024 · 1 comment
Assignees

Comments

@adinauer
Copy link
Member

adinauer commented May 21, 2024

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:

M:

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:

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 adinauer self-assigned this May 21, 2024
@adinauer adinauer changed the title POTel Performance powered by OpenTelemetry (POTel) May 21, 2024
@lbloder
Copy link
Collaborator

lbloder commented Sep 30, 2024

Test updating op / description (op and description might have been overriden in SentrySpanExporter:204)

op = OtelSpan.name / SentrySpan.operation (these are kept in sync betwen Sentry and Otel)
description = SentrySpan.description (if not set defaults to OtelSpan.name)
Description is always taken from SentrySpan (defaults to OtelSpan.name)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Discussion
Development

No branches or pull requests

2 participants