feat(otel): mark span processor based OpenTelemetry setup methods as obsolete - #5586
jamescrosswell wants to merge 2 commits into
Conversation
…lete AddSentry and UseOpenTelemetry will be removed in 7.0.0 in favour of AddSentryOtlpExporter/UseOtlp from Sentry.OpenTelemetry.Exporter. Samples still using them are migrated to OTLP. Closes #4932 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5586 +/- ##
=======================================
Coverage 74.76% 74.76%
=======================================
Files 515 515
Lines 18963 18963
Branches 3694 3694
=======================================
Hits 14177 14177
Misses 3908 3908
Partials 878 878 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| [Obsolete(ObsoleteMessage)] | ||
| public static void UseOpenTelemetry(this SentryOptions options, TracerProviderBuilder traceProviderBuilder, | ||
| TextMapPropagator? defaultTextMapPropagator = null, bool disableSentryTracing = false) | ||
| { |
There was a problem hiding this comment.
Bug: The obsolete method UseOpenTelemetry calls other obsolete methods without suppressing warning CS0618. With TreatWarningsAsErrors enabled, this could cause a build failure.
Severity: MEDIUM
Suggested Fix
Wrap the calls to the obsolete methods within the UseOpenTelemetry method with #pragma warning disable CS0618 and a corresponding #pragma warning restore CS0618. This will explicitly suppress the compiler warning and prevent a potential build failure, aligning with existing patterns in the codebase.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/Sentry.OpenTelemetry/SentryOptionsExtensions.cs#L38-L43
Potential issue: The project is configured with
`<TreatWarningsAsErrors>true</TreatWarningsAsErrors>`. The `[Obsolete]` method
`UseOpenTelemetry` at lines 38-43 calls other `[Obsolete]` methods. While the C#
compiler sometimes suppresses `CS0618` warnings for calls from one obsolete member to
another, this behavior is not guaranteed and is context-dependent. Given that the
codebase contains numerous explicit `#pragma warning disable CS0618` suppressions for
similar cases, the absence of one here creates a tangible risk of a build failure, which
would prevent the code from being deployed.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
C# skips the obsolete warning when the code making the call is itself marked [Obsolete].
| { | ||
| internal const string ObsoleteMessage = | ||
| "Use AddSentryOtlpExporter from the Sentry.OpenTelemetry.Exporter package instead. " + | ||
| "This method will be removed in version 7.0.0."; |
There was a problem hiding this comment.
| "This method will be removed in version 7.0.0."; | |
| "This method will be removed in future versions."; |
| .AddSentryOtlpExporter(dsn) // <-- Configure OpenTelemetry to send traces to Sentry | ||
| .Build(); | ||
|
|
||
| SentrySdk.Init(o => |
There was a problem hiding this comment.
To match the rest of the readme:
| SentrySdk.Init(o => | |
| SentrySdk.Init(options => |
| builder.WebHost.UseSentry(options => | ||
| { | ||
| options.Dsn = "...Your DSN..."; | ||
| options.Dsn = dsn; |
There was a problem hiding this comment.
Here in the README we don't have the var dsn from the samples, but not a big deal.
We could add the var dsn = "...Your DSN..." declaration above in the examples, or not. Your call!
| SentrySdk.Init(o => | ||
| { | ||
| options.Dsn = "...Your DSN..."; | ||
| options.Dsn = dsn; |
There was a problem hiding this comment.
Same here about the "var dsn" reference.
| /// </summary> | ||
| public static class SentryOptionsExtensions | ||
| { | ||
| internal const string ObsoleteMessage = |
There was a problem hiding this comment.
In other places we define this variable with more explicit names... Here we could use e.g. ObsoleteUseOpenTelemetry.
| /// </summary> | ||
| public static class TracerProviderBuilderExtensions | ||
| { | ||
| internal const string ObsoleteMessage = |
There was a problem hiding this comment.
Here we could use e.g. ObsoleteAddSentry.
2 participants
Marks the span-processor based ("POTEL") OpenTelemetry setup methods in
Sentry.OpenTelemetryas[Obsolete], ahead of removing them in 7.0.0:TracerProviderBuilder.AddSentry(...)→ useAddSentryOtlpExporter(dsn)SentryOptions.UseOpenTelemetry(...)(both overloads) → useUseOtlp()The replacements live in the
Sentry.OpenTelemetry.Exporterpackage, added in #4899. Obsolete warnings name that package so users know where to find the new methods.The GraphQL.Server and AspNetCore.Blazor.Server samples still used the old methods, so they now use OTLP. The OpenTelemetry sample READMEs had outdated snippets too, and those are fixed.
Closes #4932
🤖 Generated with Claude Code