OpenTelemetry support

[bruno-garcia]

PR from work done by Matt

TODO

• Update the GH discussion: Sentry + OpenTelemetry sentry#40712

Not planned

• GenerateSentryErrorsFromOtelSpan is Blocked by Record Exception.Data when calling RecordException open-telemetry/opentelemetry-dotnet#2439 (comment).

This is something only our Javascript SDK has. None of the other Sentry SDKs support it.

It's not critical as we already capture errors through the normal exception handlers. It would only be an issue if a particular Otel instrumentation decided to not let exceptions bubble up, and only to report them as Otel events.

[github-actions]

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 84d5fb7

[jamescrosswell]

@bruno-garcia what did you mean by "Missing a sample project (that is already in the solution filters).."? I assume you saw Samples.AspNetCore.OpenTelemetry/Sentry.Samples.AspNetCore.OpenTelemetry.csproj.

[jamescrosswell]

I noticed some TryGetBaggageHeader and TryGetSentryTraceHeader methods in Sentry.AspNet.HttpContextExtensions.

I also noticed almost the same in SentryTracingMiddleware in the AspNetCore project.

They're slightly different because they're targeting different HttpContext abstractions.

        var value = context.Request.Headers.Get(BaggageHeader.HttpHeaderName); // AspNet
        var value = context.Request.Headers.GetValueOrDefault(BaggageHeader.HttpHeaderName); // AspNetCore

It turns out, we also need something very similar in the SentryPropagator for Otel.

A couple of questions:

• Are we trying to target AspNet and AspNetCore with our Open Telemetry efforts?
• If we're not, is it OK for the Sentry.OpenTelemetry project to take a reference to Sentry.AspNetCore to use those extensions?
• Those extensions do use SentryOptions.LogDebug. I'm not sure if we can inject SentryOptions into the SentryPropagator. Maybe all moot in that case. It just makes me itchy to use copy/paste as a form of code reuse 🙈

[bruno-garcia]

If we're not, is it OK for the Sentry.OpenTelemetry project to take a reference to Sentry.AspNetCore to use those extensions?

I'd say rather not take a dependency on AspNetCore. Any use of OTel outside ASP.NET would have that dep and also would add another dependency to deal with versioning and breaking changes.

Those two methods seem quite small. I'd say we could be pragmatic and copy them over but if I understand correctly the goal is to get the AspNetCore HTTP context object so we can read from it? If that's the case we need to consider introducing a new package like: Sentry.OpenTelemetry.AspNetCore instead. Open has:
https://www.nuget.org/packages/OpenTelemetry.Instrumentation.AspNetCore/

That does take dep on OpenTelemetry and AspNetCore:

Are we trying to target AspNet and AspNetCore with our Open Telemetry efforts?

In any regard better to break the work up in parts, if we can do core bits first, we can follow up with AspNetCore after (we can leave AspNet out for the time being, possibly forever)

Those extensions do use SentryOptions.LogDebug. I'm not sure if we can inject SentryOptions into the SentryPropagator. Maybe all moot in that case. It just makes me itchy to use copy/paste as a form of code reuse

We've been passing SentryOptions all over the place. Often all we need is the IDiagnosticLogger but giving the option allows us to change the logger instance mid-flight for example, and start logging (e.g: going from null to a ConsoleLogger). If passing this instance is done automagically by us under the hood i don't see why not? If we expect the user to pass it, that becomes a problem because the API sounds poor/leaky

[bruno-garcia]

@bruno-garcia what did you mean by "Missing a sample project (that is already in the solution filters).."? I assume you saw Samples.AspNetCore.OpenTelemetry/Sentry.Samples.AspNetCore.OpenTelemetry.csproj.

Oh, right! I'll check on why it's erroring out on the solution filters then

[jamescrosswell]

In any regard better to break the work up in parts, if we can do core bits first, we can follow up with AspNetCore after (we can leave AspNet out for the time being, possibly forever)

I think we could support Otel span processors without any dependency on HttpContext.

However, propagation has an inherent dependency on the concept of "Http Headers", since that's how the trace id gets passed around (at least in Sentry's code - I'm not sure how it gets passed into things like DB servers). So I don't think we can include a solution for propagation without a dependency on Microsoft.AspNetCore.Http.Abstractions and/or System.Web (for AspNet).

Options for that:

1. We put it all in a Sentry.OpenTelemetry package with directives to select the right dependencies, based on the target framework. This would be a bit of a bummer for people wanting to use Otel in non web based projects though
2. We pull it out to separate Sentry.OpenTelemetry.Propagation.AspNet and Sentry.OpenTelemetry.Propagation.AspNetCore assemblies
3. Something else?

[jamescrosswell]

The Dynamic Sampling Context Problem

This is the bit that Matt was wrestling with when I started working on the PR, so probably worth documenting, as it's really not that obvious. The following is an explanation of what's going on in the Java implementation. We may have to get creative to solve in .NET as we don't have the same building blocks to work with that they did in Java.

Java implementation

Issue: Java SDK basic OTEL Support sentry-java#2327
Pull requests:

• Add SpanProcessor for OTEL sentry-java#2357
• Add contexts to SentryTracer and add otel context to it sentry-java#2361

Extracting

SentryPropagator.extract

    Baggage baggage = Baggage.fromHeader(baggageString);
    modifiedContext = modifiedContext.with(SentryOtelKeys.SENTRY_BAGGAGE_KEY, baggage);

.. which indirectly calls this overload,
the only important detail of which is that includeThirdPartyValues==false so basically we just extract any values from
the baggage header that are prefixed with sentry-,

public static Baggage fromHeader(
      final @Nullable String headerValue,
      final boolean includeThirdPartyValues,
      final @NotNull ILogger logger)

All the sentry-* values from the baggage header get stuffed back in the context (as can be seen from the very first
code snippet above) using modifiedContext.with which doesn't exist in the .NET OTEL SDK:

return value.storeInContext(this);

And that puts the data in something implementing ContextStorage... The concrete class is probably not relevant. It'll be stored somewhere so it can be used later. The key question then is where does this DSC get used later?

SentryPropagator.inject below gets everything it needs for DSC injection from the Transaction associated with the
Sentry Span... And it puts all of those things directly in the baggage header of the carrier (e.g. an HttpRequest
header). So the stuff in ContextStorage is not used for injection.

Where it does appear to be used is in SpanProcessor.onStart, which calls SentrySpanProcessor.getTraceData

baggage = parentContext.get(SentryOtelKeys.SENTRY_BAGGAGE_KEY);

When creating a new Sentry Span representing a Transaction, if that information is available then it's used to create
the new Transaction

  TransactionContext.fromSentryTrace(
      transactionName,
      transactionNameSource,
      op,
      traceData.getSentryTraceHeader(),
      traceData.getBaggage(),
      spanId);

Injecting DSC

SentryPropagator.inject

    final @Nullable BaggageHeader baggageHeader = sentrySpan.toBaggageHeader(Collections.emptyList());
    if (baggageHeader != null) {
      setter.set(carrier, baggageHeader.getName(), baggageHeader.getValue());
    }

.. which eventually translates into:

SentryTracer.updateBaggageValues

    baggage.setValuesFromTransaction(
        this, userAtomicReference.get(), hub.getOptions(), this.getSamplingDecision());

... and finally into:

baggage.setValuesFromTransaction

    
    public void setValuesFromTransaction(
      final @NotNull ITransaction transaction,
      final @Nullable User user,
      final @NotNull SentryOptions sentryOptions,
      final @Nullable TracesSamplingDecision samplingDecision
      ) 
    {
        setTraceId(transaction.getSpanContext().getTraceId().toString());
        setPublicKey(new Dsn(sentryOptions.getDsn()).getPublicKey());
        setRelease(sentryOptions.getRelease());
        setEnvironment(sentryOptions.getEnvironment());
        setUserSegment(user != null ? getSegment(user) : null);
        setTransaction(isHighQualityTransactionName(transaction.getTransactionNameSource()) ? transaction.getName() : null);
        setSampleRate(sampleRateToString(sampleRate(samplingDecision)));
    }

The Dynamic Sampling Context Solution

In the .NET SDK the equivalent of the Context object that is being passed around in Java is PropagationContext which
only contains two properties:

public ActivityContext ActivityContext { get; }
public Baggage Baggage { get; }

It turns out those properties are sufficient for our purposes. Some of the details required to create a DSC come from
baggage. The Java implementation also uses SentryTraceHeader.TraceId and
SentryTraceHeader.SpanId... which we extract/store in the PropagationContext.ActivityContext.

So we already have everything we need. We simply need to leverage these in SentrySpanProcessor.OnStart when creating Spans for Transactions.

[jamescrosswell]

This is possibly incorrect. The sentryTraceHeader.SpanId would be the Id of the upstream span right? Should we be using this to set the SpanTracer.ParentSpanId orTransactionTracer.ParentSpanId in the SentrySpanProcessor instead? And then we'd set the ActivityContext.SpanId here to ActivityId.GetRandom()?

[jamescrosswell]

It seems to be what the Java code is doing though...

      final SpanId spanId = new SpanId(traceData.getSpanId());

[jamescrosswell]

@bruno-garcia / @bitsandfoxes I think this is "first draft complete". Keen to get a proper review on this.

Also, let me know if you can think of a more comprehensive way of integration testing this.

Currently I'm using the following Python script, which makes a call to the hello endpoint of Sentry.Samples.OpenTelemetry.AspNetCore:

import requests
import sentry_sdk
#
sentry_sdk.init(
    dsn="yourDsnHere",
    traces_sample_rate=1.0,
    debug=True
)

def eat_slice(slice):
    with sentry_sdk.start_span(description="Eat Slice") as span:
        print(f'span.trace_id: {span.trace_id}')
        print(f'span.span_id: {span.span_id}')
        span.set_tag("slice.topping", slice)
        response = requests.get('http://localhost:5092/hello')
        print(response.status_code)

def eat_pizza():
    while True:
        user_input = input("What kind of pizza do you want to eat?")
        if user_input.lower() == "quit":
            break
        else:
            with sentry_sdk.start_transaction(op="task", name="Eat Pizza") as transaction:
                print(f'transaction.trace_id: {transaction.trace_id}')
                eat_slice(user_input)

eat_pizza()

The /hello endpoint then makes an HTTP request of it's own to /echo... so that tests:

• An outbound request from Python, with sentry-trace and baggage headers set
• An inbound call to /hello where trace information gets pulled from the setnry-trace and baggage headers (tests Propagator.Extract)
• Appropriate transaction span gets created by the .NET SDK (tests SpanProcessor for root spans)
• An outbound call to /echo where trace information gets propagated via the setnry-trace and baggage headers (tests Propagator.Inject)
• A couple of non-transaction spans get created by the .NET SDK (tests SpanProcessor for non root spans)

All of that then shows up in Sentry looks something like this:

Maybe instead of calling it's own /echo endpoint, the Python app could be a Flask server with an echo endpoint that the ASPNETCORE app calls back... that would guarantee the trace data was being propagated out of process?

[jamescrosswell]

@bruno-garcia suggested creating a new PR for this so that I can get a review from him (rather than the other way around)... I'll close this PR and create a new one with all the relevant history/context then.