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

.NET SDK powered by OTel

Open smeubank opened this issue 1 year ago • 4 comments

Description:

See parent ticket for more project background.

We would like to investigate using OpenTelemetry's .NET SDK to replace Sentry performance package in our own SDK. This would allow our users to benefit from all of the instrumentation of the open standard's SDK, while allowing us to tailor the experience.

Requirement:

The OTel SDK should run the show in terms of instrumentations and collecting telemetry, our SDK should only do some fine tuning to ensure data is flagged for ingestion properly (OTel attributed as tags for example)

Open Point:

  • Sentry will have a product ready OTLP endpoint, with this, we have an opportunity to exclude the transaction model all together from our implementation. It should be investigated if we need to adapt our existing trxn model.
  • Does OTLP give us a simpler route?

Information:

smeubank avatar Jan 23 '24 21:01 smeubank

A couple of thoughts:

  • It seems like a prerequisite to this is aligning the way Sentry and OTEL represent traces... specifically, OTEL doesn't distinguish between Transactions and Spans (in OTEL you can have a span that has no parent, but otherwise it has all the same properties etc. of other parented spans).
  • Are we planning on adding a hard dependency on OpenTelemetry to the Sentry package? Or would we move Tracing capabilities to the Sentry.OpenTelemetry package?
  • For integrations where OpenTelemetry instrumentation already exists, would we be removing our proprietry instrumentation? For example, Sentry.AspNetCore has two bits of middleware... SentryMiddleware and SentryTracingMiddleware. Would the SentryTracingMiddleware be obsolete (replaced by OpenTelemetry tracing for ASP.NET Core)? All that we would really retain then would be the crash reporting?
  • Are there some integrations that we maintain that don't have OpenTelemetry equivalents and what do we do with those? For example, if we want to move to using OpenTelemetry Activity for tracing, presumably integrations like DiagnosticSource that take events from DiagnosticListener and create SentrySpans from these would need to generate OpenTelemetry Activities from those DiagnosticListener events instead. These are not really Sentry specific integrations/packages anymore - they become general OpenTelemetry adapters that could potentially be donated to and maintained in another repo (assuming equivalents don't already exist - ideally someone has already built these for OpenTelemetry and we could just obsolete/retire our own integrations... either way, I think we're moving to a model where we're leveraging integrations that are maintained by the broader .NET community).
  • Does it make sense for Sentry to have it's own OTLP_ENDPOINT_URL? From what I can tell, you'd do that if Sentry was not running in the same process as the application that was generating the telemetry... so the telemetry would be serialized via json/protobuf or whatever and sent to the endpoint, where it got deserialized and processed/stored. In our case, if Sentry is running in the same process, perhaps we're better off retaining the SentrySpanProcessor to collect traces? Or are we thinking of removing this from the SDKs (i.e. the endpoints would sit on Sentry.io). Probably needs a bit of research/consideration...

Overall, this seems like a really really big piece of work. We might need to break it down into bite sized chunks. Possibly merging Spans and Transactions would be a good first step?

jamescrosswell avatar Feb 23 '24 00:02 jamescrosswell

Are we planning on adding a hard dependency on OpenTelemetry to the Sentry package? Or would we move Tracing capabilities to the Sentry.OpenTelemetry package?

This remains to be discussed. Ideally, users would not need to know or care where spans come from.

For integrations where OpenTelemetry instrumentation already exists, would we be removing our proprietary instrumentation?

There is quite a big gap in what versions are supported by Sentry vs OTel. I'd go on a case-by-case basis to go with the one offering more/better/more detailed support.

bitsandfoxes avatar Feb 29 '24 14:02 bitsandfoxes

Transaction vs Span

+ AddBreadcrumb(Breadcrumb breadcrumb) : void
+ Breadcrumbs : IReadOnlyCollection<Breadcrumb>
+ Contexts : SentryContexts
 Description : string?
+ Distribution : string?
 EndTimestamp : DateTimeOffset?
+ Environment : string?
+ EventId : SentryId
 Extra : IReadOnlyDictionary<string,object?>
+ Fingerprint : IReadOnlyList<string>
 GetTraceHeader() : SentryTraceHeader
 IsFinished : bool
+ IsParentSampled : bool?
 IsSampled : bool?
+ Level : SentryLevel?
 Measurements : IReadOnlyDictionary<string,Measurement>
+ Name : string
+ NameSource : TransactionNameSource
 Operation : string
 ParentSpanId : SpanId?
+ Platform : string?
+ Release : string?
+ Request : SentryRequest
+ SampleRate : double?
+ Sdk : SdkVersion
 SetExtra(string key, object? value) : void
 SetMeasurement(string name, Measurement measurement) : void
 SetTag(string key, string value) : void
 SpanId : SpanId
+ Spans : IReadOnlyCollection<SentrySpan>
 StartTimestamp : DateTimeOffset
 Status : SpanStatus?
 Tags : IReadOnlyDictionary<string,string>
 TraceId : SentryId
 UnsetTag(string key) : void
+ User : SentryUser
 WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) : void

- SentrySpan(ISpan tracer)
- SentrySpan(SpanId? parentSpanId, string operation)
+ SentryTransaction(ITransactionTracer tracer)
+ SentryTransaction(string name, string operation, TransactionNameSource nameSource)
+ SentryTransaction(string name, string operation)

-FromJson(JsonElement json) : SentrySpan
+FromJson(JsonElement json) : SentryTransaction

Additionally ITransactionTracer contains a GetLastActiveSpan() : ISpan? method, which ISpanTracer and SpanTracer lack.

jamescrosswell avatar Mar 12 '24 02:03 jamescrosswell