Skip to content
This repository has been archived by the owner on Aug 14, 2024. It is now read-only.

Enabling OpenTelemetry vs. Sentry instrumentation #720

Closed
AbhiPrasad opened this issue Oct 18, 2022 · 3 comments · Fixed by #721
Closed

Enabling OpenTelemetry vs. Sentry instrumentation #720

AbhiPrasad opened this issue Oct 18, 2022 · 3 comments · Fixed by #721
Assignees

Comments

@AbhiPrasad
Copy link
Member

AbhiPrasad commented Oct 18, 2022

In order for the SDKs to support OpenTelemetry, we need to disable Sentry instrumentation and enable OpenTelemetry instrumentation. To enable otel, we introduce a new top-level option, instrumenter, which decides what instrumentation to enable. Currently there are two options, otel and sentry.

Sentry.init({
  instrumenter: 'otel',
});

Today we can generate performance data in two ways with a Sentry SDK.

  1. Sentry.startTransaction()
  2. transaction.startChild() or span.startChild()

The easiest solution to gate Sentry instrumentation is to go through all the call sites of Sentry.startTransaction() and span.startChild() done by SDK auto-instrumentation and gate them behind flags. Essentially, if the current instrumenter is Sentry, then call Sentry.startTransaction() or span.startChild(). This can get pretty messy though, since we have to cover each callsite with an if statement, and handle cases where spans/transactions are not defined.

let transaction;
if (instrumenter === 'sentry') {
  transaction = Sentry.startTransaction();
}

The better option is probably to hide this logic inside Sentry.startTransaction() (hub.startTransaction()). The problem here is that this doesn't address transaction.startChild()/span.startChild(), since they aren't top level methods, they are instead methods on the class. We can solve this by introducing a new top level method that creates spans on the active transaction (this is being investigated here: getsentry/rfcs#28).

// starts transaction
Sentry.startTransaction(transactionContext);
// starts span around callback
// name tbd (hardest problem in CS 😢)
Sentry.trace(spanContext, callback);

Sentry.trace will need to always put the span on the scope. If this doesn't happen we'll have to update a lot of logic to conditionally call scope.setSpan based on if Sentry.trace returned an undefined span.

@dcramer
Copy link
Member

dcramer commented Oct 18, 2022

We still need transaction markers, no? So we would still need start/end transaction calls somehow

@AbhiPrasad
Copy link
Member Author

Yes - under the hood we would still generate these transactions using Sentry.startTransaction calls, but we still need to differentiate Sentry.startTransaction calls that come from us transforming otel data -> sentry in an OpenTelemetry span processor vs. those that happen in our SDK integrations for auto-instrumentation.

@sl0thentr0py
Copy link
Member

sl0thentr0py commented Oct 19, 2022

Sentry.trace will need to always put the span on the scope. If this doesn't happen we'll have to update a lot of logic to conditionally call scope.setSpan based on if Sentry.trace returned an undefined span.

updating scope should happen within Sentry.trace

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging a pull request may close this issue.

3 participants