From 058b19c0f6fa44d5cc9abab51bbb94a89dd14008 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 25 Mar 2024 23:21:23 +1300 Subject: [PATCH 01/11] Experimenting with System.Diagnostic wrappers --- .../Internal/Tracing/ActivitySourceWrapper.cs | 18 ++++++ .../Internal/Tracing/ActivityWrapper.cs | 32 +++++++++++ .../Internal/Tracing/SentryTraceProvider.cs | 15 +++++ .../Tracing/SentryTracingIntegration.cs | 19 +++++++ .../Sentry.DiagnosticSource.csproj | 2 +- src/Sentry/Internal/DisabledSpan.cs | 32 +++++++++++ src/Sentry/Internal/DisabledTraceProvider.cs | 8 +++ src/Sentry/Internal/DisabledTracer.cs | 9 +++ src/Sentry/Internal/ISentrySpan.cs | 9 +++ src/Sentry/Internal/ISentryTraceProvider.cs | 6 ++ src/Sentry/Internal/ISentryTracer.cs | 7 +++ src/Sentry/Sentry.csproj | 1 + src/Sentry/SentryOptions.cs | 55 ++++++++++++++++--- 13 files changed, 204 insertions(+), 9 deletions(-) create mode 100644 src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySourceWrapper.cs create mode 100644 src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs create mode 100644 src/Sentry.DiagnosticSource/Internal/Tracing/SentryTraceProvider.cs create mode 100644 src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs create mode 100644 src/Sentry/Internal/DisabledSpan.cs create mode 100644 src/Sentry/Internal/DisabledTraceProvider.cs create mode 100644 src/Sentry/Internal/DisabledTracer.cs create mode 100644 src/Sentry/Internal/ISentrySpan.cs create mode 100644 src/Sentry/Internal/ISentryTraceProvider.cs create mode 100644 src/Sentry/Internal/ISentryTracer.cs diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySourceWrapper.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySourceWrapper.cs new file mode 100644 index 0000000000..ef08b92f4e --- /dev/null +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySourceWrapper.cs @@ -0,0 +1,18 @@ +namespace Sentry.Internal.Tracing; + +/// +/// Wraps the functionality that we use from ActivitySource in an interface so that we +/// can access this from our integrations without taking a hard dependency on +/// System.Diagnostics.ActivitySource (which is only available in .NET 5.0 and later) +/// +internal class ActivitySourceWrapper(string name, string? version = "") : ISentryTracer +{ + private readonly ActivitySource _activitySource = new(name, version); + + public ISentrySpan? StartSpan(string operationName) => + _activitySource.StartActivity(operationName) is { } activity ? new ActivityWrapper(activity) : null; + + public ISentrySpan? CurrentSpan => System.Diagnostics.Activity.Current == null + ? null + : new ActivityWrapper(System.Diagnostics.Activity.Current); +} diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs new file mode 100644 index 0000000000..5fea9c8dca --- /dev/null +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs @@ -0,0 +1,32 @@ +namespace Sentry.Internal.Tracing; + +#if !NET6_0_OR_GREATER +using System.Diagnostics; +#endif + +/// +/// Wraps the functionality that we use from Activity in an interface so that we can +/// access this from our integrations without taking a hard dependency on +/// System.Diagnostics.Activity (which is only available in .NET 5.0 and later) +/// +internal class ActivityWrapper(System.Diagnostics.Activity activity) : ISentrySpan +{ + public void SetAttribute(string key, object value) => activity.SetTag(key, value); + + public void AddEvent(string message) => activity.AddEvent(new ActivityEvent(message)); + + public void SetStatus(SpanStatus status, string? description = default) + { + if (status == SpanStatus.Ok) + { + activity.SetStatus(ActivityStatusCode.Ok); + return; + } + var errorMessage = description ?? status.ToString(); + activity.SetStatus(ActivityStatusCode.Error, errorMessage); + } + + public void Stop () => activity.Stop(); + + public void Dispose() => activity.Dispose(); +} diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTraceProvider.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTraceProvider.cs new file mode 100644 index 0000000000..497354eabd --- /dev/null +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTraceProvider.cs @@ -0,0 +1,15 @@ +namespace Sentry.Internal.Tracing; + +/// +/// The default concrete implementation of that uses +/// and from the +/// namespace to implement tracing. +/// +internal class SentryTraceProvider : ISentryTraceProvider +{ + private Lazy> _lazyActivitySources = new(); + private ConcurrentDictionary _activitySources => _lazyActivitySources.Value; + + public ISentryTracer GetTracer(string name, string? version = "") + => _activitySources.GetOrAdd(name, new ActivitySourceWrapper(name, version)); +} diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs new file mode 100644 index 0000000000..2e2d408838 --- /dev/null +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs @@ -0,0 +1,19 @@ +using Sentry.Extensibility; +using Sentry.Integrations; + +namespace Sentry.Internal.Tracing; + +internal class SentryTracingIntegration : ISdkIntegration +{ + public void Register(IHub hub, SentryOptions options) + { + if (!options.IsPerformanceMonitoringEnabled) + { + options.Log(SentryLevel.Info, "SentryTracing Integration is disabled because tracing is disabled."); + return; + } + + options.SentryTraceProvider = new SentryTraceProvider(); + // TODO: Also configure a listener : https://learn.microsoft.com/en-us/dotnet/core/diagnostics/distributed-tracing-collection-walkthroughs#collect-traces-using-custom-logic + } +} diff --git a/src/Sentry.DiagnosticSource/Sentry.DiagnosticSource.csproj b/src/Sentry.DiagnosticSource/Sentry.DiagnosticSource.csproj index 2bb043f837..80c0eadc68 100644 --- a/src/Sentry.DiagnosticSource/Sentry.DiagnosticSource.csproj +++ b/src/Sentry.DiagnosticSource/Sentry.DiagnosticSource.csproj @@ -14,7 +14,7 @@ - + diff --git a/src/Sentry/Internal/DisabledSpan.cs b/src/Sentry/Internal/DisabledSpan.cs new file mode 100644 index 0000000000..645314cd64 --- /dev/null +++ b/src/Sentry/Internal/DisabledSpan.cs @@ -0,0 +1,32 @@ +namespace Sentry.Internal; + +internal class DisabledSpan : ISentrySpan +{ + private static readonly Lazy LazyInstance = new(); + public static DisabledSpan Instance => LazyInstance.Value; + + public void Dispose() + { + // No-Op + } + + public void SetAttribute(string key, object value) + { + // No-Op + } + + public void AddEvent(string message) + { + // No-Op + } + + public void SetStatus(SpanStatus status, string? description = default) + { + // No-Op + } + + public void Stop() + { + // No-Op + } +} diff --git a/src/Sentry/Internal/DisabledTraceProvider.cs b/src/Sentry/Internal/DisabledTraceProvider.cs new file mode 100644 index 0000000000..16b564dce2 --- /dev/null +++ b/src/Sentry/Internal/DisabledTraceProvider.cs @@ -0,0 +1,8 @@ +namespace Sentry.Internal; + +internal class DisabledTraceProvider : ISentryTraceProvider +{ + private static readonly Lazy LazyInstance = new(); + public static DisabledTraceProvider Instance => LazyInstance.Value; + public ISentryTracer GetTracer(string name, string? version = "") => DisabledTracer.Instance; +} diff --git a/src/Sentry/Internal/DisabledTracer.cs b/src/Sentry/Internal/DisabledTracer.cs new file mode 100644 index 0000000000..2f5fd097cc --- /dev/null +++ b/src/Sentry/Internal/DisabledTracer.cs @@ -0,0 +1,9 @@ +namespace Sentry.Internal; + +internal class DisabledTracer : ISentryTracer +{ + private static readonly Lazy LazyInstance = new(); + public static DisabledTracer Instance => LazyInstance.Value; + public ISentrySpan StartSpan(string operationName) => DisabledSpan.Instance; + public ISentrySpan? CurrentSpan => DisabledSpan.Instance; +} diff --git a/src/Sentry/Internal/ISentrySpan.cs b/src/Sentry/Internal/ISentrySpan.cs new file mode 100644 index 0000000000..c993568a0f --- /dev/null +++ b/src/Sentry/Internal/ISentrySpan.cs @@ -0,0 +1,9 @@ +namespace Sentry.Internal; + +internal interface ISentrySpan: IDisposable +{ + void SetAttribute(string key, object value); + void AddEvent(string message); + void SetStatus(SpanStatus status, string? description = default); + void Stop (); +} diff --git a/src/Sentry/Internal/ISentryTraceProvider.cs b/src/Sentry/Internal/ISentryTraceProvider.cs new file mode 100644 index 0000000000..9a48c57f02 --- /dev/null +++ b/src/Sentry/Internal/ISentryTraceProvider.cs @@ -0,0 +1,6 @@ +namespace Sentry.Internal; + +internal interface ISentryTraceProvider +{ + public ISentryTracer GetTracer(string name, string? version = ""); +} diff --git a/src/Sentry/Internal/ISentryTracer.cs b/src/Sentry/Internal/ISentryTracer.cs new file mode 100644 index 0000000000..8421bfd7ae --- /dev/null +++ b/src/Sentry/Internal/ISentryTracer.cs @@ -0,0 +1,7 @@ +namespace Sentry.Internal; + +internal interface ISentryTracer +{ + ISentrySpan? StartSpan(string operationName); + ISentrySpan? CurrentSpan { get; } +} diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index 77d1d4b965..af3affa75a 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -52,6 +52,7 @@ $(DefineConstants);HAS_DIAGNOSTIC_INTEGRATION + $(DefineConstants);HAS_TRACING_INTEGRATION diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index f6e6c29c10..cfc6071c3f 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -13,6 +13,10 @@ using Sentry.Internal.DiagnosticSource; #endif +#if HAS_TRACING_INTEGRATION +using Sentry.Internal.Tracing; +#endif + #if ANDROID using Sentry.Android; using Sentry.Android.AssemblyReader; @@ -153,42 +157,42 @@ internal IEnumerable Integrations get { // Auto-session tracking to be the first to run - if ((_defaultIntegrations & DefaultIntegrations.AutoSessionTrackingIntegration) != 0) + if (_defaultIntegrations.Includes(DefaultIntegrations.AutoSessionTrackingIntegration)) { yield return new AutoSessionTrackingIntegration(); } - if ((_defaultIntegrations & DefaultIntegrations.AppDomainUnhandledExceptionIntegration) != 0) + if (_defaultIntegrations.Includes(DefaultIntegrations.AppDomainUnhandledExceptionIntegration)) { yield return new AppDomainUnhandledExceptionIntegration(); } - if ((_defaultIntegrations & DefaultIntegrations.AppDomainProcessExitIntegration) != 0) + if (_defaultIntegrations.Includes(DefaultIntegrations.AppDomainProcessExitIntegration)) { yield return new AppDomainProcessExitIntegration(); } - if ((_defaultIntegrations & DefaultIntegrations.UnobservedTaskExceptionIntegration) != 0) + if (_defaultIntegrations.Includes(DefaultIntegrations.UnobservedTaskExceptionIntegration)) { yield return new UnobservedTaskExceptionIntegration(); } #if NETFRAMEWORK - if ((_defaultIntegrations & DefaultIntegrations.NetFxInstallationsIntegration) != 0) + if (_defaultIntegrations.Includes(DefaultIntegrations.NetFxInstallationsIntegration)) { yield return new NetFxInstallationsIntegration(); } #endif #if HAS_DIAGNOSTIC_INTEGRATION - if ((_defaultIntegrations & DefaultIntegrations.SentryDiagnosticListenerIntegration) != 0) + if (_defaultIntegrations.Includes(DefaultIntegrations.SentryDiagnosticListenerIntegration)) { yield return new SentryDiagnosticListenerIntegration(); } #endif #if NET5_0_OR_GREATER && !__MOBILE__ - if ((_defaultIntegrations & DefaultIntegrations.WinUiUnhandledExceptionIntegration) != 0 + if (_defaultIntegrations.Includes(DefaultIntegrations.WinUiUnhandledExceptionIntegration) && RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { yield return new WinUIUnhandledExceptionIntegration(); @@ -196,12 +200,19 @@ internal IEnumerable Integrations #endif #if NET8_0_OR_GREATER - if ((_defaultIntegrations & DefaultIntegrations.SystemDiagnosticsMetricsIntegration) != 0) + if (_defaultIntegrations.Includes(DefaultIntegrations.SystemDiagnosticsMetricsIntegration)) { yield return new SystemDiagnosticsMetricsIntegration(); } #endif +#if HAS_TRACING_INTEGRATION + if (_defaultIntegrations.Includes(DefaultIntegrations.SentryTracingIntegration)) + { + yield return new SentryTracingIntegration(); + } +#endif + foreach (var integration in _integrations) { yield return integration; @@ -223,6 +234,14 @@ internal IEnumerable Integrations internal ISentryHttpClientFactory? SentryHttpClientFactory { get; set; } + private ISentryTraceProvider? _sentryTraceProvider; + + internal ISentryTraceProvider SentryTraceProvider + { + get => _sentryTraceProvider ?? DisabledTraceProvider.Instance; + set => _sentryTraceProvider = value; + } + internal HttpClient GetHttpClient() { var factory = SentryHttpClientFactory ?? new DefaultSentryHttpClientFactory(); @@ -1223,6 +1242,9 @@ public SentryOptions() #endif #if NET8_0_OR_GREATER | DefaultIntegrations.SystemDiagnosticsMetricsIntegration +#endif +#if HAS_TRACING_INTEGRATION + | DefaultIntegrations.SentryTracingIntegration #endif ; @@ -1608,6 +1630,14 @@ public void DisableSystemDiagnosticsMetricsIntegration() => RemoveDefaultIntegration(DefaultIntegrations.SystemDiagnosticsMetricsIntegration); #endif +#if HAS_TRACING_INTEGRATION + /// + /// Disables the Sentry Tracing integration. + /// + public void DisableSentryTracingIntegration() + => RemoveDefaultIntegration(DefaultIntegrations.SentryTracingIntegration); +#endif + internal bool HasIntegration() => _integrations.Any(integration => integration is TIntegration); internal void RemoveDefaultIntegration(DefaultIntegrations defaultIntegrations) => _defaultIntegrations &= ~defaultIntegrations; @@ -1630,6 +1660,9 @@ internal enum DefaultIntegrations #endif #if NET8_0_OR_GREATER SystemDiagnosticsMetricsIntegration = 1 << 7, +#endif +#if HAS_TRACING_INTEGRATION + SentryTracingIntegration = 1 << 8, #endif } @@ -1680,3 +1713,9 @@ internal void SetupLogging() return TryGetDsnSpecificCacheDirectoryPath(); } } + +internal static class SentryOptionsExtensions +{ + public static bool Includes(this SentryOptions.DefaultIntegrations integrations, SentryOptions.DefaultIntegrations value) + => (integrations & value) != 0; +} From e2bbc81b8ff70573338a7696310decf106b255a1 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 25 Mar 2024 22:27:57 +0000 Subject: [PATCH 02/11] Format code --- modules/sentry-native | 2 +- .../Internal/Tracing/ActivityWrapper.cs | 2 +- src/Sentry/Internal/ISentrySpan.cs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/sentry-native b/modules/sentry-native index 4ec95c0725..9bc0fc75e3 160000 --- a/modules/sentry-native +++ b/modules/sentry-native @@ -1 +1 @@ -Subproject commit 4ec95c0725df5f34440db8fa8d37b4c519fce74e +Subproject commit 9bc0fc75e34cb43e7019d76b0decb6c0cddbfd34 diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs index 5fea9c8dca..8b57c354d8 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs @@ -26,7 +26,7 @@ public void SetStatus(SpanStatus status, string? description = default) activity.SetStatus(ActivityStatusCode.Error, errorMessage); } - public void Stop () => activity.Stop(); + public void Stop() => activity.Stop(); public void Dispose() => activity.Dispose(); } diff --git a/src/Sentry/Internal/ISentrySpan.cs b/src/Sentry/Internal/ISentrySpan.cs index c993568a0f..fa2109c58b 100644 --- a/src/Sentry/Internal/ISentrySpan.cs +++ b/src/Sentry/Internal/ISentrySpan.cs @@ -1,9 +1,9 @@ namespace Sentry.Internal; -internal interface ISentrySpan: IDisposable +internal interface ISentrySpan : IDisposable { void SetAttribute(string key, object value); void AddEvent(string message); void SetStatus(SpanStatus status, string? description = default); - void Stop (); + void Stop(); } From cec23fc5cdac30aed4a5e55b12a018a9f287f689 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 26 Mar 2024 21:44:13 +1300 Subject: [PATCH 03/11] WIP: Added framework for Listener (pushing to switch IDE to macOS) --- .../Internal/Tracing/ActivitySpanProcessor.cs | 425 ++++++++++++++++ .../Tracing/SentryActivityListener.cs | 37 ++ .../Tracing/SentryTracingIntegration.cs | 11 +- .../Internal/Tracing/TraceExtensions.cs} | 4 +- .../Sentry.DiagnosticSource.csproj | 2 +- .../OpenTelemetryTransactionProcessor.cs | 1 + .../Sentry.OpenTelemetry.csproj | 18 + src/Sentry.OpenTelemetry/SentryPropagator.cs | 1 + .../SentrySpanProcessor.cs | 391 +-------------- src/Sentry/DefaultIntegrationsExtensions.cs | 7 + src/Sentry/Sentry.csproj | 10 +- src/Sentry/SentryOptions.cs | 6 - .../Tracing/ActivitySpanProcessorTests.cs | 452 ++++++++++++++++++ .../OpenTelemetryExtensionsTests.cs | 2 + .../OpenTelemetryTransactionProcessorTests.cs | 2 + .../Sentry.OpenTelemetry.Tests.csproj | 11 +- .../SentryPropagatorTests.cs | 1 + .../SentrySpanProcessorTests.cs | 412 +--------------- 18 files changed, 984 insertions(+), 809 deletions(-) create mode 100644 src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs create mode 100644 src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs rename src/{Sentry.OpenTelemetry/OpenTelemetryExtensions.cs => Sentry.DiagnosticSource/Internal/Tracing/TraceExtensions.cs} (91%) create mode 100644 src/Sentry/DefaultIntegrationsExtensions.cs create mode 100644 test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs new file mode 100644 index 0000000000..554159b25b --- /dev/null +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs @@ -0,0 +1,425 @@ +using Sentry.Extensibility; +using Sentry.Internal.Extensions; +using Sentry.Internal.OpenTelemetry; + +namespace Sentry.Internal.Tracing; + +/// +/// Helper class to convert events to Sentry Spans. +/// +internal class ActivitySpanProcessor +{ + private readonly IHub _hub; + + private Action? _beforeFinish; + + // ReSharper disable once MemberCanBePrivate.Global - Used by tests + internal readonly ConcurrentDictionary _map = new(); + private readonly SentryOptions? _options; + private readonly Lazy> _resourceAttributes; + + private static readonly long PruningInterval = TimeSpan.FromSeconds(5).Ticks; + internal long _lastPruned = 0; + private readonly Lazy _realHub; + + internal ActivitySpanProcessor(IHub hub, Action? beforeFinish = null, + Func>? resourceAttributeResolver = null) + { + _hub = hub; + _beforeFinish = beforeFinish; + _realHub = new Lazy(() => + _hub switch + { + Hub thisHub => thisHub, + HubAdapter when SentrySdk.CurrentHub is Hub sdkHub => sdkHub, + _ => null + }); + + _options = hub.GetSentryOptions(); + + if (_options is null) + { + throw new InvalidOperationException( + "The Sentry SDK has not been initialised. To use tracing you need to initialize the Sentry SDK."); + } + + // Resource attributes are consistent between spans, but not available during construction. + // Thus, get a single instance lazily. + resourceAttributeResolver ??= () => new Dictionary(); + _resourceAttributes = new Lazy>(resourceAttributeResolver); + } + + internal ISpan? GetMappedSpan(ActivitySpanId spanId) => _map.GetValueOrDefault(spanId); + + /// + public void OnStart(System.Diagnostics.Activity data) + { + if (data.ParentSpanId != default && _map.TryGetValue(data.ParentSpanId, out var parentSpan)) + { + // We can find the parent span - start a child span. + var context = new SpanContext( + data.OperationName, + data.SpanId.AsSentrySpanId(), + data.ParentSpanId.AsSentrySpanId(), + data.TraceId.AsSentryId(), + data.DisplayName, + null, + null) + { + Instrumenter = Instrumenter.OpenTelemetry + }; + + var span = (SpanTracer)parentSpan.StartChild(context); + span.StartTimestamp = data.StartTimeUtc; + // Used to filter out spans that are not recorded when finishing a transaction. + span.SetFused(data); + span.IsFiltered = () => span.GetFused() + is { IsAllDataRequested: false, Recorded: false }; + _map[data.SpanId] = span; + } + else + { +#if HAS_DIAGNOSTICS_7_OR_GREATER + // If a parent exists, then copy its sampling decision. + bool? isSampled = data.HasRemoteParent ? data.Recorded : null; +#else + bool? isSampled = null; +#endif + + // No parent span found - start a new transaction + var transactionContext = new TransactionContext(data.DisplayName, + data.OperationName, + data.SpanId.AsSentrySpanId(), + data.ParentSpanId.AsSentrySpanId(), + data.TraceId.AsSentryId(), + data.DisplayName, null, isSampled, isSampled) + { + Instrumenter = Instrumenter.OpenTelemetry + }; + + var baggageHeader = data.Baggage.AsBaggageHeader(); + var dynamicSamplingContext = baggageHeader.CreateDynamicSamplingContext(); + var transaction = (TransactionTracer)_hub.StartTransaction( + transactionContext, new Dictionary(), dynamicSamplingContext + ); + transaction.StartTimestamp = data.StartTimeUtc; + _hub.ConfigureScope(scope => scope.Transaction = transaction); + transaction.SetFused(data); + _map[data.SpanId] = transaction; + } + + // Housekeeping + PruneFilteredSpans(); + } + + public void OnEnd(System.Diagnostics.Activity data) + { + // Make a dictionary of the attributes (aka "tags") for faster lookup when used throughout the processor. + var attributes = data.TagObjects.ToDict(); + + var url = + attributes.TryGetTypedValue(OtelSemanticConventions.AttributeUrlFull, out string? tempUrl) ? tempUrl + : attributes.TryGetTypedValue(OtelSemanticConventions.AttributeHttpUrl, out string? fallbackUrl) ? fallbackUrl // Falling back to pre-1.5.0 + : null; + + if (!string.IsNullOrEmpty(url) && (_options?.IsSentryRequest(url) ?? false)) + { + _options?.DiagnosticLogger?.LogDebug($"Ignoring Activity {data.SpanId} for Sentry request."); + + if (_map.TryRemove(data.SpanId, out var removed)) + { + if (removed is SpanTracer spanTracerToRemove) + { + spanTracerToRemove.IsSentryRequest = true; + } + + if (removed is TransactionTracer transactionTracer) + { + transactionTracer.IsSentryRequest = true; + } + } + + return; + } + + if (!_map.TryGetValue(data.SpanId, out var span)) + { + _options?.DiagnosticLogger?.LogError($"Span not found for SpanId: {data.SpanId}. Did OnStart run? We might have a bug in the SDK."); + return; + } + + var (operation, description, source) = ParseOtelSpanDescription(data, attributes); + span.Operation = operation; + span.Description = description; + + if (span is TransactionTracer transaction) + { + transaction.Name = description; + transaction.NameSource = source; + + // Use the end timestamp from the activity data. + transaction.EndTimestamp = data.StartTimeUtc + data.Duration; + + // Transactions set otel attributes (and resource attributes) as context. + transaction.Contexts["otel"] = GetOtelContext(attributes); + } + else + { + // Use the end timestamp from the activity data. + ((SpanTracer)span).EndTimestamp = data.StartTimeUtc + data.Duration; + + // Spans set otel attributes in extras (passed to Sentry as "data" on the span). + // Resource attributes do not need to be set, as they would be identical as those set on the transaction. + span.SetExtras(attributes); + span.SetExtra("otel.kind", data.Kind); + } + + // In ASP.NET Core the middleware finishes up (and the scope gets popped) before the activity is ended. So we + // need to restore the scope here (it's saved by our middleware when the request starts) + var activityScope = GetSavedScope(data); + if (activityScope is { } savedScope) + { + var hub = _realHub.Value; + hub?.RestoreScope(savedScope); + } + GenerateSentryErrorsFromOtelSpan(data, attributes); + + var status = GetSpanStatus(data.Status, attributes); + _beforeFinish?.Invoke(span, data); + span.Finish(status); + + _map.TryRemove(data.SpanId, out _); + + // Housekeeping + PruneFilteredSpans(); + } + + /// + /// Clean up items that may have been filtered out. + /// See https://github.com/getsentry/sentry-dotnet/pull/3198 + /// + internal void PruneFilteredSpans(bool force = false) + { + if (!force && !NeedsPruning()) + { + return; + } + + foreach (var mappedItem in _map) + { + var (spanId, span) = mappedItem; + var activity = span.GetFused(); + if (activity is { Recorded: false, IsAllDataRequested: false }) + { + _map.TryRemove(spanId, out _); + } + } + } + + private bool NeedsPruning() + { + var lastPruned = Interlocked.Read(ref _lastPruned); + if (lastPruned > DateTime.UtcNow.Ticks - PruningInterval) + { + return false; + } + + var thisPruned = DateTime.UtcNow.Ticks; + Interlocked.CompareExchange(ref _lastPruned, thisPruned, lastPruned); + // May be false if another thread gets there first + return Interlocked.Read(ref _lastPruned) == thisPruned; + } + + private static Scope? GetSavedScope(System.Diagnostics.Activity? activity) + { + while (activity is not null) + { + if (activity.GetFused() is { } savedScope) + { + return savedScope; + } + activity = activity.Parent; + } + return null; + } + + internal static SpanStatus GetSpanStatus(ActivityStatusCode status, IDictionary attributes) + { + // See https://github.com/open-telemetry/opentelemetry-dotnet/discussions/4703 + if (attributes.TryGetValue(OtelSpanAttributeConstants.StatusCodeKey, out var statusCode) + && statusCode is OtelStatusTags.ErrorStatusCodeTagValue + ) + { + return GetErrorSpanStatus(attributes); + } + return status switch + { + ActivityStatusCode.Unset => SpanStatus.Ok, + ActivityStatusCode.Ok => SpanStatus.Ok, + ActivityStatusCode.Error => GetErrorSpanStatus(attributes), + _ => SpanStatus.UnknownError + }; + } + + private static SpanStatus GetErrorSpanStatus(IDictionary attributes) + { + if (attributes.TryGetTypedValue("http.status_code", out int httpCode)) + { + return SpanStatusConverter.FromHttpStatusCode(httpCode); + } + + if (attributes.TryGetTypedValue("rpc.grpc.status_code", out int grpcCode)) + { + return SpanStatusConverter.FromGrpcStatusCode(grpcCode); + } + + return SpanStatus.UnknownError; + } + + private static (string operation, string description, TransactionNameSource source) ParseOtelSpanDescription( + System.Diagnostics.Activity activity, + IDictionary attributes) + { + // This function should loosely match the JavaScript implementation at: + // https://github.com/getsentry/sentry-javascript/blob/3487fa3af7aa72ac7fdb0439047cb7367c591e77/packages/opentelemetry-node/src/utils/parseOtelSpanDescription.ts + // However, it should also follow the OpenTelemetry semantic conventions specification, as indicated. + + // HTTP span + // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/http/ + if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeHttpMethod, out string httpMethod)) + { + if (activity.Kind == ActivityKind.Client) + { + // Per OpenTelemetry spec, client spans use only the method. + return ("http.client", httpMethod, TransactionNameSource.Custom); + } + + if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeHttpRoute, out string httpRoute)) + { + // A route exists. Use the method and route. + return ("http.server", $"{httpMethod} {httpRoute}", TransactionNameSource.Route); + } + + if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeHttpTarget, out string httpTarget)) + { + // A target exists. Use the method and target. If the target is "/" we can treat it like a route. + var source = httpTarget == "/" ? TransactionNameSource.Route : TransactionNameSource.Url; + return ("http.server", $"{httpMethod} {httpTarget}", source); + } + + // Some other type of HTTP server span. Pass it through with the original name. + return ("http.server", activity.DisplayName, TransactionNameSource.Custom); + } + + // DB span + // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/database/ + if (attributes.ContainsKey(OtelSemanticConventions.AttributeDbSystem)) + { + if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeDbStatement, out string dbStatement)) + { + // We have a database statement. Use it. + return ("db", dbStatement, TransactionNameSource.Task); + } + + // Some other type of DB span. Pass it through with the original name. + return ("db", activity.DisplayName, TransactionNameSource.Task); + } + + // RPC span + // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/rpc/ + if (attributes.ContainsKey(OtelSemanticConventions.AttributeRpcService)) + { + return ("rpc", activity.DisplayName, TransactionNameSource.Route); + } + + // Messaging span + // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/messaging/ + if (attributes.ContainsKey(OtelSemanticConventions.AttributeMessagingSystem)) + { + return ("message", activity.DisplayName, TransactionNameSource.Route); + } + + // FaaS (Functions/Lambda) span + // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/faas/ + if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeFaasTrigger, out string faasTrigger)) + { + return (faasTrigger, activity.DisplayName, TransactionNameSource.Route); + } + + // Default - pass through unmodified. + return (activity.OperationName, activity.DisplayName, TransactionNameSource.Custom); + } + + private Dictionary GetOtelContext(IDictionary attributes) + { + var otelContext = new Dictionary(); + if (attributes.Count > 0) + { + otelContext.Add("attributes", attributes); + } + + var resourceAttributes = _resourceAttributes.Value; + if (resourceAttributes.Count > 0) + { + otelContext.Add("resource", resourceAttributes); + } + + return otelContext; + } + + private void GenerateSentryErrorsFromOtelSpan(System.Diagnostics.Activity activity, IDictionary spanAttributes) + { + // https://develop.sentry.dev/sdk/performance/opentelemetry/#step-7-define-generatesentryerrorsfromotelspan + // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/exceptions/ + foreach (var @event in activity.Events.Where(e => e.Name == OtelSemanticConventions.AttributeExceptionEventName)) + { + var eventAttributes = @event.Tags.ToDict(); + // This would be where we would ideally implement full exception capture. That's not possible at the + // moment since the full exception isn't yet available via the OpenTelemetry API. + // See https://github.com/open-telemetry/opentelemetry-dotnet/issues/2439#issuecomment-1577314568 + // if (!eventAttributes.TryGetTypedValue("exception", out Exception exception)) + // { + // continue; + // } + + // At the moment, OTEL only gives us `exception.type`, `exception.message`, and `exception.stacktrace`... + // So the best we can do is a poor man's exception (no accurate symbolication or anything) + if (!eventAttributes.TryGetTypedValue(OtelSemanticConventions.AttributeExceptionType, out string exceptionType)) + { + continue; + } + eventAttributes.TryGetTypedValue(OtelSemanticConventions.AttributeExceptionMessage, out string message); + eventAttributes.TryGetTypedValue(OtelSemanticConventions.AttributeExceptionStacktrace, out string stackTrace); + + Exception exception; + try + { + var type = Type.GetType(exceptionType)!; + exception = (Exception)Activator.CreateInstance(type, message)!; + exception.SetSentryMechanism("SentrySpanProcessor.ErrorSpan"); + } + catch + { + _options?.DiagnosticLogger?.LogError($"Failed to create poor man's exception for type : {exceptionType}"); + continue; + } + + // TODO: Validate that our `DuplicateEventDetectionEventProcessor` prevents this from doubling exceptions + // that are also caught by other means, such as our AspNetCore middleware, etc. + // (When options.RecordException = true is set on AddAspNetCoreInstrumentation...) + // Also, in such cases - how will we get the otel scope and trace context on the other one? + + var sentryEvent = new SentryEvent(exception, @event.Timestamp); + var otelContext = GetOtelContext(spanAttributes); + otelContext.Add("stack_trace", stackTrace); + sentryEvent.Contexts["otel"] = otelContext; + _hub.CaptureEvent(sentryEvent, scope => + { + var trace = scope.Contexts.Trace; + trace.SpanId = activity.SpanId.AsSentrySpanId(); + trace.ParentSpanId = activity.ParentSpanId.AsSentrySpanId(); + trace.TraceId = activity.TraceId.AsSentryId(); + }); + } + } +} diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs new file mode 100644 index 0000000000..399226a1ab --- /dev/null +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs @@ -0,0 +1,37 @@ +namespace Sentry.Internal.Tracing; + +internal class SentryActivityListener : IDisposable +{ + private readonly ActivityListener? _listener; + public SentryActivityListener() + { + _listener = new ActivityListener() + { + // This is only for internal Sentry events + ShouldListenTo = (source) => source.Name.StartsWith("Sentry"), + Sample = ShouldSample, + ActivityStarted = OnActivityStarted, + ActivityStopped = OnActivityStopped + }; + ActivitySource.AddActivityListener(_listener); + } + + // We sample all the activities... these will get filtered out by the Hub when they get converted to spans + public ActivitySamplingResult ShouldSample(ref ActivityCreationOptions _) + => ActivitySamplingResult.AllDataAndRecorded; + + public void OnActivityStarted(System.Diagnostics.Activity activity) + { + Console.WriteLine("Started: {0,-15} {1,-60}", activity.OperationName, activity.Id); + } + + public void OnActivityStopped(System.Diagnostics.Activity activity) + { + Console.WriteLine("Stopped: {0,-15} {1,-60} {2,-15}", activity.OperationName, activity.Id, activity.Duration); + } + + public void Dispose() + { + _listener?.Dispose(); + } +} diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs index 2e2d408838..2c5554e6fb 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs @@ -5,6 +5,15 @@ namespace Sentry.Internal.Tracing; internal class SentryTracingIntegration : ISdkIntegration { + /* + TODO: Think about where to put this... would be good if it sat on an IDisposable but also something internal. + We could possibly put it on the Hub - add an ITracingHub interface that was internal and held both this and + the SentryTraceProvider. Alternatively we could rename IMetricHub to IInternalHub and put it there... metrics + should probably be using this mechanism for tracing anyway (as it's all internal) but that would complicate + things like code locations... which we could maybe store as custom properties on the Activity. + */ + private SentryActivityListener? _listener; + public void Register(IHub hub, SentryOptions options) { if (!options.IsPerformanceMonitoringEnabled) @@ -14,6 +23,6 @@ public void Register(IHub hub, SentryOptions options) } options.SentryTraceProvider = new SentryTraceProvider(); - // TODO: Also configure a listener : https://learn.microsoft.com/en-us/dotnet/core/diagnostics/distributed-tracing-collection-walkthroughs#collect-traces-using-custom-logic + _listener = new SentryActivityListener(); } } diff --git a/src/Sentry.OpenTelemetry/OpenTelemetryExtensions.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/TraceExtensions.cs similarity index 91% rename from src/Sentry.OpenTelemetry/OpenTelemetryExtensions.cs rename to src/Sentry.DiagnosticSource/Internal/Tracing/TraceExtensions.cs index fecc538723..3396981873 100644 --- a/src/Sentry.OpenTelemetry/OpenTelemetryExtensions.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/TraceExtensions.cs @@ -1,6 +1,6 @@ -namespace Sentry.OpenTelemetry; +namespace Sentry.Internal.Tracing; -internal static class OpenTelemetryExtensions +internal static class TraceExtensions { public static SpanId AsSentrySpanId(this ActivitySpanId id) => SpanId.Parse(id.ToHexString()); diff --git a/src/Sentry.DiagnosticSource/Sentry.DiagnosticSource.csproj b/src/Sentry.DiagnosticSource/Sentry.DiagnosticSource.csproj index 80c0eadc68..1c7bcbc65b 100644 --- a/src/Sentry.DiagnosticSource/Sentry.DiagnosticSource.csproj +++ b/src/Sentry.DiagnosticSource/Sentry.DiagnosticSource.csproj @@ -14,7 +14,7 @@ - + diff --git a/src/Sentry.OpenTelemetry/OpenTelemetryTransactionProcessor.cs b/src/Sentry.OpenTelemetry/OpenTelemetryTransactionProcessor.cs index 4ab485dde1..b8580014c4 100644 --- a/src/Sentry.OpenTelemetry/OpenTelemetryTransactionProcessor.cs +++ b/src/Sentry.OpenTelemetry/OpenTelemetryTransactionProcessor.cs @@ -1,4 +1,5 @@ using Sentry.Extensibility; +using Sentry.Internal.Tracing; namespace Sentry.OpenTelemetry; diff --git a/src/Sentry.OpenTelemetry/Sentry.OpenTelemetry.csproj b/src/Sentry.OpenTelemetry/Sentry.OpenTelemetry.csproj index 66a780f05a..46c13b97f3 100644 --- a/src/Sentry.OpenTelemetry/Sentry.OpenTelemetry.csproj +++ b/src/Sentry.OpenTelemetry/Sentry.OpenTelemetry.csproj @@ -25,4 +25,22 @@ + + + $(DefineConstants);HAS_DIAGNOSTICS_7_OR_GREATER + + + + + + Internal\Tracing\%(RecursiveDir)%(Filename)%(Extension) + + + + diff --git a/src/Sentry.OpenTelemetry/SentryPropagator.cs b/src/Sentry.OpenTelemetry/SentryPropagator.cs index 78fd960d6b..f803dcda6c 100644 --- a/src/Sentry.OpenTelemetry/SentryPropagator.cs +++ b/src/Sentry.OpenTelemetry/SentryPropagator.cs @@ -2,6 +2,7 @@ using OpenTelemetry; using OpenTelemetry.Context.Propagation; using Sentry.Extensibility; +using Sentry.Internal.Tracing; namespace Sentry.OpenTelemetry; diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index 520c09fbd8..d9db8daee3 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -1,8 +1,7 @@ using OpenTelemetry; using Sentry.Extensibility; -using Sentry.Internal; using Sentry.Internal.Extensions; -using Sentry.Internal.OpenTelemetry; +using Sentry.Internal.Tracing; namespace Sentry.OpenTelemetry; @@ -11,17 +10,11 @@ namespace Sentry.OpenTelemetry; /// public class SentrySpanProcessor : BaseProcessor { - private readonly IHub _hub; internal readonly IEnumerable _enrichers; - // ReSharper disable once MemberCanBePrivate.Global - Used by tests - internal readonly ConcurrentDictionary _map = new(); + private readonly ActivitySpanProcessor _activitySpanProcessor; + private readonly IHub _hub; private readonly SentryOptions? _options; - private readonly Lazy> _resourceAttributes; - - private static readonly long PruningInterval = TimeSpan.FromSeconds(5).Ticks; - internal long _lastPruned = 0; - private readonly Lazy _realHub; /// /// Constructs a . @@ -40,14 +33,6 @@ public SentrySpanProcessor(IHub hub) : this(hub, null) internal SentrySpanProcessor(IHub hub, IEnumerable? enrichers) { _hub = hub; - _realHub = new Lazy(() => - _hub switch - { - Hub thisHub => thisHub, - HubAdapter when SentrySdk.CurrentHub is Hub sdkHub => sdkHub, - _ => null - }); - if (_hub is DisabledHub) { // This would only happen if someone tried to create a SentrySpanProcessor manually @@ -56,9 +41,7 @@ internal SentrySpanProcessor(IHub hub, IEnumerable? enri "You should use the TracerProviderBuilderExtensions to configure Sentry with OpenTelemetry"); } - _enrichers = enrichers ?? Enumerable.Empty(); _options = hub.GetSentryOptions(); - if (_options is null) { throw new InvalidOperationException( @@ -73,380 +56,34 @@ internal SentrySpanProcessor(IHub hub, IEnumerable? enri "to initialize the Sentry SDK with options.UseOpenTelemetry()"); } - // Resource attributes are consistent between spans, but not available during construction. - // Thus, get a single instance lazily. - _resourceAttributes = new Lazy>(() => - ParentProvider?.GetResource().Attributes.ToDict() ?? new Dictionary(0)); + _enrichers = enrichers ?? Enumerable.Empty(); + _activitySpanProcessor = new ActivitySpanProcessor(hub, ApplyEnrichers, GetResourceAttributes); } + internal ISpan? GetMappedSpan(ActivitySpanId spanId) => _activitySpanProcessor.GetMappedSpan(spanId); + /// public override void OnStart(Activity data) { - if (data.ParentSpanId != default && _map.TryGetValue(data.ParentSpanId, out var parentSpan)) - { - // We can find the parent span - start a child span. - var context = new SpanContext( - data.OperationName, - data.SpanId.AsSentrySpanId(), - data.ParentSpanId.AsSentrySpanId(), - data.TraceId.AsSentryId(), - data.DisplayName, - null, - null) - { - Instrumenter = Instrumenter.OpenTelemetry - }; - - var span = (SpanTracer)parentSpan.StartChild(context); - span.StartTimestamp = data.StartTimeUtc; - // Used to filter out spans that are not recorded when finishing a transaction. - span.SetFused(data); - span.IsFiltered = () => span.GetFused() is { IsAllDataRequested: false, Recorded: false }; - _map[data.SpanId] = span; - } - else - { - // If a parent exists at all, then copy its sampling decision. - bool? isSampled = data.HasRemoteParent ? data.Recorded : null; - - // No parent span found - start a new transaction - var transactionContext = new TransactionContext(data.DisplayName, - data.OperationName, - data.SpanId.AsSentrySpanId(), - data.ParentSpanId.AsSentrySpanId(), - data.TraceId.AsSentryId(), - data.DisplayName, null, isSampled, isSampled) - { - Instrumenter = Instrumenter.OpenTelemetry - }; - - var baggageHeader = data.Baggage.AsBaggageHeader(); - var dynamicSamplingContext = baggageHeader.CreateDynamicSamplingContext(); - var transaction = (TransactionTracer)_hub.StartTransaction( - transactionContext, new Dictionary(), dynamicSamplingContext - ); - transaction.StartTimestamp = data.StartTimeUtc; - _hub.ConfigureScope(scope => scope.Transaction = transaction); - transaction.SetFused(data); - _map[data.SpanId] = transaction; - } - - // Housekeeping - PruneFilteredSpans(); + _activitySpanProcessor.OnStart(data); } /// public override void OnEnd(Activity data) { - // Make a dictionary of the attributes (aka "tags") for faster lookup when used throughout the processor. - var attributes = data.TagObjects.ToDict(); - - var url = - attributes.TryGetTypedValue(OtelSemanticConventions.AttributeUrlFull, out string? tempUrl) ? tempUrl - : attributes.TryGetTypedValue(OtelSemanticConventions.AttributeHttpUrl, out string? fallbackUrl) ? fallbackUrl // Falling back to pre-1.5.0 - : null; - - if (!string.IsNullOrEmpty(url) && (_options?.IsSentryRequest(url) ?? false)) - { - _options?.DiagnosticLogger?.LogDebug($"Ignoring Activity {data.SpanId} for Sentry request."); - - if (_map.TryRemove(data.SpanId, out var removed)) - { - if (removed is SpanTracer spanTracerToRemove) - { - spanTracerToRemove.IsSentryRequest = true; - } - - if (removed is TransactionTracer transactionTracer) - { - transactionTracer.IsSentryRequest = true; - } - } - - return; - } - - if (!_map.TryGetValue(data.SpanId, out var span)) - { - _options?.DiagnosticLogger?.LogError($"Span not found for SpanId: {data.SpanId}. Did OnStart run? We might have a bug in the SDK."); - return; - } - - var (operation, description, source) = ParseOtelSpanDescription(data, attributes); - span.Operation = operation; - span.Description = description; - - if (span is TransactionTracer transaction) - { - transaction.Name = description; - transaction.NameSource = source; - - // Use the end timestamp from the activity data. - transaction.EndTimestamp = data.StartTimeUtc + data.Duration; - - // Transactions set otel attributes (and resource attributes) as context. - transaction.Contexts["otel"] = GetOtelContext(attributes); - } - else - { - // Use the end timestamp from the activity data. - ((SpanTracer)span).EndTimestamp = data.StartTimeUtc + data.Duration; - - // Spans set otel attributes in extras (passed to Sentry as "data" on the span). - // Resource attributes do not need to be set, as they would be identical as those set on the transaction. - span.SetExtras(attributes); - span.SetExtra("otel.kind", data.Kind); - } - - // In ASP.NET Core the middleware finishes up (and the scope gets popped) before the activity is ended. So we - // need to restore the scope here (it's saved by our middleware when the request starts) - var activityScope = GetSavedScope(data); - if (activityScope is { } savedScope) - { - var hub = _realHub.Value; - hub?.RestoreScope(savedScope); - } - GenerateSentryErrorsFromOtelSpan(data, attributes); - - var status = GetSpanStatus(data.Status, attributes); - foreach (var enricher in _enrichers) - { - enricher.Enrich(span, data, _hub, _options); - } - span.Finish(status); - - _map.TryRemove(data.SpanId, out _); - - // Housekeeping - PruneFilteredSpans(); - } - - /// - /// Clean up items that may have been filtered out. - /// See https://github.com/getsentry/sentry-dotnet/pull/3198 - /// - internal void PruneFilteredSpans(bool force = false) - { - if (!force && !NeedsPruning()) - { - return; - } - - foreach (var mappedItem in _map) - { - var (spanId, span) = mappedItem; - var activity = span.GetFused(); - if (activity is { Recorded: false, IsAllDataRequested: false }) - { - _map.TryRemove(spanId, out _); - } - } - } - - private bool NeedsPruning() - { - var lastPruned = Interlocked.Read(ref _lastPruned); - if (lastPruned > DateTime.UtcNow.Ticks - PruningInterval) - { - return false; - } - - var thisPruned = DateTime.UtcNow.Ticks; - Interlocked.CompareExchange(ref _lastPruned, thisPruned, lastPruned); - // May be false if another thread gets there first - return Interlocked.Read(ref _lastPruned) == thisPruned; - } - - private static Scope? GetSavedScope(Activity? activity) - { - while (activity is not null) - { - if (activity.GetFused() is { } savedScope) - { - return savedScope; - } - activity = activity.Parent; - } - return null; - } - - internal static SpanStatus GetSpanStatus(ActivityStatusCode status, IDictionary attributes) - { - // See https://github.com/open-telemetry/opentelemetry-dotnet/discussions/4703 - if (attributes.TryGetValue(OtelSpanAttributeConstants.StatusCodeKey, out var statusCode) - && statusCode is OtelStatusTags.ErrorStatusCodeTagValue - ) - { - return GetErrorSpanStatus(attributes); - } - return status switch - { - ActivityStatusCode.Unset => SpanStatus.Ok, - ActivityStatusCode.Ok => SpanStatus.Ok, - ActivityStatusCode.Error => GetErrorSpanStatus(attributes), - _ => SpanStatus.UnknownError - }; - } - - private static SpanStatus GetErrorSpanStatus(IDictionary attributes) - { - if (attributes.TryGetTypedValue("http.status_code", out int httpCode)) - { - return SpanStatusConverter.FromHttpStatusCode(httpCode); - } - - if (attributes.TryGetTypedValue("rpc.grpc.status_code", out int grpcCode)) - { - return SpanStatusConverter.FromGrpcStatusCode(grpcCode); - } - - return SpanStatus.UnknownError; - } - - private static (string operation, string description, TransactionNameSource source) ParseOtelSpanDescription( - Activity activity, - IDictionary attributes) - { - // This function should loosely match the JavaScript implementation at: - // https://github.com/getsentry/sentry-javascript/blob/3487fa3af7aa72ac7fdb0439047cb7367c591e77/packages/opentelemetry-node/src/utils/parseOtelSpanDescription.ts - // However, it should also follow the OpenTelemetry semantic conventions specification, as indicated. - - // HTTP span - // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/http/ - if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeHttpMethod, out string httpMethod)) - { - if (activity.Kind == ActivityKind.Client) - { - // Per OpenTelemetry spec, client spans use only the method. - return ("http.client", httpMethod, TransactionNameSource.Custom); - } - - if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeHttpRoute, out string httpRoute)) - { - // A route exists. Use the method and route. - return ("http.server", $"{httpMethod} {httpRoute}", TransactionNameSource.Route); - } - - if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeHttpTarget, out string httpTarget)) - { - // A target exists. Use the method and target. If the target is "/" we can treat it like a route. - var source = httpTarget == "/" ? TransactionNameSource.Route : TransactionNameSource.Url; - return ("http.server", $"{httpMethod} {httpTarget}", source); - } - - // Some other type of HTTP server span. Pass it through with the original name. - return ("http.server", activity.DisplayName, TransactionNameSource.Custom); - } - - // DB span - // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/database/ - if (attributes.ContainsKey(OtelSemanticConventions.AttributeDbSystem)) - { - if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeDbStatement, out string dbStatement)) - { - // We have a database statement. Use it. - return ("db", dbStatement, TransactionNameSource.Task); - } - - // Some other type of DB span. Pass it through with the original name. - return ("db", activity.DisplayName, TransactionNameSource.Task); - } - - // RPC span - // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/rpc/ - if (attributes.ContainsKey(OtelSemanticConventions.AttributeRpcService)) - { - return ("rpc", activity.DisplayName, TransactionNameSource.Route); - } - - // Messaging span - // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/messaging/ - if (attributes.ContainsKey(OtelSemanticConventions.AttributeMessagingSystem)) - { - return ("message", activity.DisplayName, TransactionNameSource.Route); - } - - // FaaS (Functions/Lambda) span - // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/faas/ - if (attributes.TryGetTypedValue(OtelSemanticConventions.AttributeFaasTrigger, out string faasTrigger)) - { - return (faasTrigger, activity.DisplayName, TransactionNameSource.Route); - } - - // Default - pass through unmodified. - return (activity.OperationName, activity.DisplayName, TransactionNameSource.Custom); + _activitySpanProcessor.OnEnd(data); } - private Dictionary GetOtelContext(IDictionary attributes) + private void ApplyEnrichers(ISpan span, Activity data) { - var otelContext = new Dictionary(); - if (attributes.Count > 0) - { - otelContext.Add("attributes", attributes); - } - - var resourceAttributes = _resourceAttributes.Value; - if (resourceAttributes.Count > 0) + foreach (var enricher in _enrichers) { - otelContext.Add("resource", resourceAttributes); + enricher.Enrich(span, data, _hub, _options); } - - return otelContext; } - private void GenerateSentryErrorsFromOtelSpan(Activity activity, IDictionary spanAttributes) + private Dictionary GetResourceAttributes() { - // https://develop.sentry.dev/sdk/performance/opentelemetry/#step-7-define-generatesentryerrorsfromotelspan - // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/exceptions/ - foreach (var @event in activity.Events.Where(e => e.Name == OtelSemanticConventions.AttributeExceptionEventName)) - { - var eventAttributes = @event.Tags.ToDict(); - // This would be where we would ideally implement full exception capture. That's not possible at the - // moment since the full exception isn't yet available via the OpenTelemetry API. - // See https://github.com/open-telemetry/opentelemetry-dotnet/issues/2439#issuecomment-1577314568 - // if (!eventAttributes.TryGetTypedValue("exception", out Exception exception)) - // { - // continue; - // } - - // At the moment, OTEL only gives us `exception.type`, `exception.message`, and `exception.stacktrace`... - // So the best we can do is a poor man's exception (no accurate symbolication or anything) - if (!eventAttributes.TryGetTypedValue(OtelSemanticConventions.AttributeExceptionType, out string exceptionType)) - { - continue; - } - eventAttributes.TryGetTypedValue(OtelSemanticConventions.AttributeExceptionMessage, out string message); - eventAttributes.TryGetTypedValue(OtelSemanticConventions.AttributeExceptionStacktrace, out string stackTrace); - - Exception exception; - try - { - var type = Type.GetType(exceptionType)!; - exception = (Exception)Activator.CreateInstance(type, message)!; - exception.SetSentryMechanism("SentrySpanProcessor.ErrorSpan"); - } - catch - { - _options?.DiagnosticLogger?.LogError($"Failed to create poor man's exception for type : {exceptionType}"); - continue; - } - - // TODO: Validate that our `DuplicateEventDetectionEventProcessor` prevents this from doubling exceptions - // that are also caught by other means, such as our AspNetCore middleware, etc. - // (When options.RecordException = true is set on AddAspNetCoreInstrumentation...) - // Also, in such cases - how will we get the otel scope and trace context on the other one? - - var sentryEvent = new SentryEvent(exception, @event.Timestamp); - var otelContext = GetOtelContext(spanAttributes); - otelContext.Add("stack_trace", stackTrace); - sentryEvent.Contexts["otel"] = otelContext; - _hub.CaptureEvent(sentryEvent, scope => - { - var trace = scope.Contexts.Trace; - trace.SpanId = activity.SpanId.AsSentrySpanId(); - trace.ParentSpanId = activity.ParentSpanId.AsSentrySpanId(); - trace.TraceId = activity.TraceId.AsSentryId(); - }); - } + return ParentProvider?.GetResource().Attributes.ToDict() ?? new Dictionary(0); } } diff --git a/src/Sentry/DefaultIntegrationsExtensions.cs b/src/Sentry/DefaultIntegrationsExtensions.cs new file mode 100644 index 0000000000..e9ddca023d --- /dev/null +++ b/src/Sentry/DefaultIntegrationsExtensions.cs @@ -0,0 +1,7 @@ +namespace Sentry; + +internal static class DefaultIntegrationsExtensions +{ + public static bool Includes(this SentryOptions.DefaultIntegrations integrations, SentryOptions.DefaultIntegrations value) + => (integrations & value) != 0; +} diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index af3affa75a..2f71469b59 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -55,10 +55,16 @@ $(DefineConstants);HAS_TRACING_INTEGRATION - - Internal\%(RecursiveDir)%(Filename)%(Extension) + + Internal\DiagnosticSource\%(RecursiveDir)%(Filename)%(Extension) + + + Internal\Tracing\%(RecursiveDir)%(Filename)%(Extension) + + $(DefineConstants);HAS_DIAGNOSTICS_7_OR_GREATER + - - - - - - - - + diff --git a/test/Sentry.OpenTelemetry.Tests/SentryPropagatorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentryPropagatorTests.cs index 693619aece..940b405319 100644 --- a/test/Sentry.OpenTelemetry.Tests/SentryPropagatorTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/SentryPropagatorTests.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.Primitives; using OpenTelemetry; using OpenTelemetry.Context.Propagation; +using Sentry.Internal.Tracing; namespace Sentry.OpenTelemetry.Tests; diff --git a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs index aab7671d1b..52f58ff113 100644 --- a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs @@ -1,4 +1,5 @@ using Sentry.Internal.OpenTelemetry; +using Sentry.Internal.Tracing; namespace Sentry.OpenTelemetry.Tests; @@ -65,247 +66,6 @@ public void Ctor_Instrumenter_Not_OpenTelemetry_Throws() Assert.Throws(() => _fixture.GetSut()); } - [Fact] - public void GetSpanStatus() - { - using (new AssertionScope()) - { - var noAttributes = new Dictionary(); - - // Unset and OK -> OK - SentrySpanProcessor.GetSpanStatus(ActivityStatusCode.Unset, noAttributes).Should().Be(SpanStatus.Ok); - SentrySpanProcessor.GetSpanStatus(ActivityStatusCode.Ok, noAttributes).Should().Be(SpanStatus.Ok); - - // Error (no attributes) -> UnknownError - SentrySpanProcessor.GetSpanStatus(ActivityStatusCode.Error, noAttributes) - .Should().Be(SpanStatus.UnknownError); - - // Unknown status code -> UnknownError - SentrySpanProcessor.GetSpanStatus((ActivityStatusCode)42, noAttributes) - .Should().Be(SpanStatus.UnknownError); - - // We only test one http scenario, just to make sure the SpanStatusConverter is called for these headers. - // Tests for SpanStatusConverter ensure other http status codes would also work though - var notFoundAttributes = new Dictionary { ["http.status_code"] = 404 }; - SentrySpanProcessor.GetSpanStatus(ActivityStatusCode.Error, notFoundAttributes) - .Should().Be(SpanStatus.NotFound); - - // We only test one grpc scenario, just to make sure the SpanStatusConverter is called for these headers. - // Tests for SpanStatusConverter ensure other grpc status codes would also work though - var grpcAttributes = new Dictionary { ["rpc.grpc.status_code"] = 7 }; - SentrySpanProcessor.GetSpanStatus(ActivityStatusCode.Error, grpcAttributes) - .Should().Be(SpanStatus.PermissionDenied); - - var errorAttributes = new Dictionary { [OtelSpanAttributeConstants.StatusCodeKey] = OtelStatusTags.ErrorStatusCodeTagValue }; - SentrySpanProcessor.GetSpanStatus(ActivityStatusCode.Ok, errorAttributes).Should().Be(SpanStatus.UnknownError); - SentrySpanProcessor.GetSpanStatus(ActivityStatusCode.Unset, errorAttributes).Should().Be(SpanStatus.UnknownError); - } - } - - [Fact] - public void OnStart_Transaction_With_DynamicSamplingContext() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - var expected = new Dictionary() - { - { "trace_id", SentryId.Create().ToString() }, - { "public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff" }, - { "sample_rate", "0.5" }, - }; - var data = Tracer.StartActivity("test op")!; - data.AddBaggage($"{BaggageHeader.SentryKeyPrefix}trace_id", expected["trace_id"]); - data.AddBaggage($"{BaggageHeader.SentryKeyPrefix}public_key", expected["public_key"]); - data.AddBaggage($"{BaggageHeader.SentryKeyPrefix}sample_rate", expected["sample_rate"]); - - // Act - sut.OnStart(data!); - - // Assert - Assert.True(sut._map.TryGetValue(data.SpanId, out var span)); - if (span is not TransactionTracer transaction) - { - Assert.Fail("Span is not a transaction tracer"); - return; - } - if (transaction.DynamicSamplingContext is not { } actual) - { - Assert.Fail("Transaction does not have a dynamic sampling context"); - return; - } - using (new AssertionScope()) - { - actual.Items["trace_id"].Should().Be(expected["trace_id"]); - actual.Items["public_key"].Should().Be(expected["public_key"]); - actual.Items["sample_rate"].Should().Be(expected["sample_rate"]); - } - } - - [Fact] - public void OnStart_WithParentSpanId_StartsChildSpan() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - using var parent = Tracer.StartActivity("Parent"); - sut.OnStart(parent); - - using var data = Tracer.StartActivity("TestActivity"); - - // Act - sut.OnStart(data!); - - // Assert - Assert.True(sut._map.TryGetValue(data.SpanId, out var span)); - using (new AssertionScope()) - { - span.Should().BeOfType(); - span.SpanId.Should().Be(data.SpanId.AsSentrySpanId()); - span.ParentSpanId.Should().Be(data.ParentSpanId.AsSentrySpanId()); - if (span is not SpanTracer spanTracer) - { - Assert.Fail("Span is not a span tracer"); - return; - } - using (new AssertionScope()) - { - spanTracer.SpanId.Should().Be(data.SpanId.AsSentrySpanId()); - spanTracer.ParentSpanId.Should().Be(data.ParentSpanId.AsSentrySpanId()); - spanTracer.TraceId.Should().Be(data.TraceId.AsSentryId()); - spanTracer.Operation.Should().Be(data.OperationName); - spanTracer.Description.Should().Be(data.DisplayName); - spanTracer.Status.Should().BeNull(); - spanTracer.StartTimestamp.Should().Be(data.StartTimeUtc); - } - } - } - - [Fact] - public void OnStart_WithoutParentSpanId_StartsNewTransaction() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - _fixture.ScopeManager = Substitute.For(); - var sut = _fixture.GetSut(); - - var data = Tracer.StartActivity("test op"); - - // Act - sut.OnStart(data!); - - // Assert - Assert.True(sut._map.TryGetValue(data.SpanId, out var span)); - if (span is not TransactionTracer transaction) - { - Assert.Fail("Span is not a transaction tracer"); - return; - } - using (new AssertionScope()) - { - transaction.SpanId.Should().Be(data.SpanId.AsSentrySpanId()); - transaction.ParentSpanId.Should().Be(new ActivitySpanId().AsSentrySpanId()); - transaction.TraceId.Should().Be(data.TraceId.AsSentryId()); - transaction.Name.Should().Be(data.DisplayName); - transaction.Operation.Should().Be(data.OperationName); - transaction.Description.Should().Be(data.DisplayName); - transaction.Status.Should().BeNull(); - transaction.StartTimestamp.Should().Be(data.StartTimeUtc); - _fixture.ScopeManager.Received(1).ConfigureScope(Arg.Any>()); - } - } - - [Fact] - public void OnEnd_FinishesSpan() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - var parent = Tracer.StartActivity(name: "transaction")!; - sut.OnStart(parent); - - var tags = new Dictionary { - { "foo", "bar" } - }; - var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; - data.DisplayName = "test display name"; - sut.OnStart(data); - - sut._map.TryGetValue(data.SpanId, out var span); - - // Act - sut.OnEnd(data); - - // Assert - if (span is not SpanTracer spanTracer) - { - Assert.Fail("Span is not a span tracer"); - return; - } - - using (new AssertionScope()) - { - spanTracer.ParentSpanId.Should().Be(parent.SpanId.AsSentrySpanId()); - spanTracer.Operation.Should().Be(data.OperationName); - spanTracer.Description.Should().Be(data.DisplayName); - spanTracer.EndTimestamp.Should().NotBeNull(); - spanTracer.Extra["otel.kind"].Should().Be(data.Kind); - foreach (var keyValuePair in tags) - { - span.Extra[keyValuePair.Key].Should().Be(keyValuePair.Value); - } - - spanTracer.Status.Should().Be(SpanStatus.Ok); - } - } - - [Fact] - public void OnEnd_Transaction_RestoresSavedScope() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - _fixture.ScopeManager = Substitute.For(); - var sut = _fixture.GetSut(); - - var scope = new Scope(); - var data = Tracer.StartActivity("transaction")!; - data.SetFused(scope); - sut.OnStart(data); - - // Act - sut.OnEnd(data); - - // Assert - _fixture.ScopeManager.Received(1).RestoreScope(scope); - } - - [Fact] - public void OnEnd_Span_RestoresSavedScope() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - _fixture.ScopeManager = Substitute.For(); - var sut = _fixture.GetSut(); - - var scope = new Scope(); - var parent = Tracer.StartActivity("transaction")!; - parent.SetFused(scope); - sut.OnStart(parent); - - var data = Tracer.StartActivity("test operation")!; - data.DisplayName = "test display name"; - sut.OnStart(data); - - // Act - sut.OnEnd(data); - - // Assert - _fixture.ScopeManager.Received(1).RestoreScope(scope); - } - [Fact] public void OnEnd_SpansEnriched() { @@ -319,7 +79,7 @@ public void OnEnd_SpansEnriched() var parent = Tracer.StartActivity(name: "transaction")!; sut.OnStart(parent); - sut._map.TryGetValue(parent.SpanId, out var span); + var span = sut.GetMappedSpan(parent.SpanId); // Act sut.OnEnd(parent); @@ -334,172 +94,4 @@ public void OnEnd_SpansEnriched() transactionTracer.Tags.TryGetValue("foo", out var foo).Should().BeTrue(); foo.Should().Be("bar"); } - - [Fact] - public void OnEnd_FinishesTransaction() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - var tags = new Dictionary { - { "foo", "bar" } - }; - var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; - data.DisplayName = "test display name"; - sut.OnStart(data); - - sut._map.TryGetValue(data.SpanId, out var span); - - // Act - sut.OnEnd(data); - - // Assert - if (span is not TransactionTracer transaction) - { - Assert.Fail("Span is not a transaction tracer"); - return; - } - - using (new AssertionScope()) - { - transaction.ParentSpanId.Should().Be(new ActivitySpanId().AsSentrySpanId()); - transaction.Operation.Should().Be(data.OperationName); - transaction.Description.Should().Be(data.DisplayName); - transaction.Name.Should().Be(data.DisplayName); - transaction.NameSource.Should().Be(TransactionNameSource.Custom); - transaction.EndTimestamp.Should().NotBeNull(); - transaction.Contexts["otel"].Should().BeEquivalentTo(new Dictionary - { - { "attributes", tags } - }); - transaction.Status.Should().Be(SpanStatus.Ok); - } - } - - [Theory] - [InlineData(OtelSemanticConventions.AttributeUrlFull)] - [InlineData(OtelSemanticConventions.AttributeHttpUrl)] - public void OnEnd_IsSentryRequest_DoesNotFinishTransaction(string urlKey) - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - var tags = new Dictionary { { "foo", "bar" }, { urlKey, _fixture.Options.Dsn } }; - var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; - data.DisplayName = "test display name"; - sut.OnStart(data); - - sut._map.TryGetValue(data.SpanId, out var span); - - // Act - sut.OnEnd(data); - - // Assert - if (span is not TransactionTracer transaction) - { - Assert.Fail("Span is not a transaction tracer"); - return; - } - - transaction.IsSentryRequest.Should().BeTrue(); - } - - private static void FilterActivity(Activity activity) - { - // Simulates filtering an activity - see https://github.com/getsentry/sentry-dotnet/pull/3198 - activity.IsAllDataRequested = false; - activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; - } - - [Fact] - public void PruneFilteredSpans_FilteredTransactions_Pruned() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - using var parent = Tracer.StartActivity(); - sut.OnStart(parent!); - - FilterActivity(parent); - - // Act - sut.PruneFilteredSpans(true); - - // Assert - Assert.False(sut._map.TryGetValue(parent.SpanId, out var _)); - } - - [Fact] - public void PruneFilteredSpans_UnFilteredTransactions_NotPruned() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - using var parent = Tracer.StartActivity(); - sut.OnStart(parent!); - - // Act - sut.PruneFilteredSpans(true); - - // Assert - Assert.True(sut._map.TryGetValue(parent.SpanId, out var _)); - } - - [Fact] - public void PruneFilteredSpans_FilteredSpans_Pruned() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - using var parent = Tracer.StartActivity(); - sut.OnStart(parent!); - - using var activity1 = Tracer.StartActivity(); - sut.OnStart(activity1!); - - using var activity2 = Tracer.StartActivity(); - sut.OnStart(activity2!); - - FilterActivity(activity2); - - // Act - sut.PruneFilteredSpans(true); - - // Assert - Assert.True(sut._map.TryGetValue(activity1.SpanId, out var _)); - Assert.False(sut._map.TryGetValue(activity2.SpanId, out var _)); - } - - [Fact] - public void PruneFilteredSpans_RecentlyPruned_DoesNothing() - { - // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var sut = _fixture.GetSut(); - - sut._lastPruned = DateTimeOffset.MaxValue.Ticks; // fake a recent prune - - using var parent = Tracer.StartActivity(); - sut.OnStart(parent!); - - using var activity1 = Tracer.StartActivity(); - sut.OnStart(activity1!); - - using var activity2 = Tracer.StartActivity(); - sut.OnStart(activity2!); - - FilterActivity(activity2); - - // Act - sut.PruneFilteredSpans(); - - // Assert - Assert.True(sut._map.TryGetValue(activity1.SpanId, out var _)); - Assert.True(sut._map.TryGetValue(activity2.SpanId, out var _)); - } } From 555a25e6be7ebe468aa5553d7727e26f7660f026 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 27 Mar 2024 20:51:39 +1300 Subject: [PATCH 04/11] Fixed tests (except net48 tracing tests) --- .../Internal/Tracing/ActivitySpanProcessor.cs | 17 +++++- .../Tracing/SentryActivityListener.cs | 13 +++-- .../Tracing/SentryTracingIntegration.cs | 3 +- .../Tracing/ActivitySpanProcessorTests.cs | 53 ++++++++++--------- .../ActivitySourceTests.cs | 2 +- ...piApprovalTests.Run.DotNet6_0.verified.txt | 1 + ...piApprovalTests.Run.DotNet7_0.verified.txt | 1 + ...piApprovalTests.Run.DotNet8_0.verified.txt | 1 + ...y_registered.DotNet6_0.DotNet.verified.txt | 6 +++ ...y_registered.DotNet7_0.DotNet.verified.txt | 6 +++ ...y_registered.DotNet8_0.DotNet.verified.txt | 6 +++ 11 files changed, 77 insertions(+), 32 deletions(-) diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs index 554159b25b..2aa3368698 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs @@ -12,6 +12,7 @@ internal class ActivitySpanProcessor private readonly IHub _hub; private Action? _beforeFinish; + private readonly Instrumenter _instrumenter; // ReSharper disable once MemberCanBePrivate.Global - Used by tests internal readonly ConcurrentDictionary _map = new(); @@ -22,11 +23,23 @@ internal class ActivitySpanProcessor internal long _lastPruned = 0; private readonly Lazy _realHub; + internal ActivitySpanProcessor(IHub hub) + : this(hub, null, null, Instrumenter.Sentry) + { + } + internal ActivitySpanProcessor(IHub hub, Action? beforeFinish = null, Func>? resourceAttributeResolver = null) + : this(hub, beforeFinish, resourceAttributeResolver, Instrumenter.OpenTelemetry) + { + } + + private ActivitySpanProcessor(IHub hub, Action? beforeFinish, + Func>? resourceAttributeResolver, Instrumenter instrumenter) { _hub = hub; _beforeFinish = beforeFinish; + _instrumenter = instrumenter; _realHub = new Lazy(() => _hub switch { @@ -66,7 +79,7 @@ public void OnStart(System.Diagnostics.Activity data) null, null) { - Instrumenter = Instrumenter.OpenTelemetry + Instrumenter = _instrumenter }; var span = (SpanTracer)parentSpan.StartChild(context); @@ -94,7 +107,7 @@ public void OnStart(System.Diagnostics.Activity data) data.TraceId.AsSentryId(), data.DisplayName, null, isSampled, isSampled) { - Instrumenter = Instrumenter.OpenTelemetry + Instrumenter = _instrumenter }; var baggageHeader = data.Baggage.AsBaggageHeader(); diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs index 399226a1ab..bee544e858 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs @@ -2,9 +2,16 @@ namespace Sentry.Internal.Tracing; internal class SentryActivityListener : IDisposable { + private readonly ActivitySpanProcessor _activitySpanProcessor; private readonly ActivityListener? _listener; - public SentryActivityListener() + + public SentryActivityListener(IHub hub) : this(new ActivitySpanProcessor(hub)) + { + } + + public SentryActivityListener(ActivitySpanProcessor activitySpanProcessor) { + _activitySpanProcessor = activitySpanProcessor; _listener = new ActivityListener() { // This is only for internal Sentry events @@ -22,12 +29,12 @@ public ActivitySamplingResult ShouldSample(ref ActivityCreationOptions(); } @@ -36,7 +42,9 @@ public Fixture() public ActivitySpanProcessor GetSut() { - return new ActivitySpanProcessor(GetHub()); + var processor = new ActivitySpanProcessor(GetHub()); + Listener = new SentryActivityListener(processor); + return processor; } } @@ -91,13 +99,13 @@ public void OnStart_Transaction_With_DynamicSamplingContext() { "public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff" }, { "sample_rate", "0.5" }, }; - var data = Tracer.StartActivity("test op")!; + var data = Tracer.CreateActivity("test op", ActivityKind.Internal)!; data.AddBaggage($"{BaggageHeader.SentryKeyPrefix}trace_id", expected["trace_id"]); data.AddBaggage($"{BaggageHeader.SentryKeyPrefix}public_key", expected["public_key"]); data.AddBaggage($"{BaggageHeader.SentryKeyPrefix}sample_rate", expected["sample_rate"]); // Act - sut.OnStart(data!); + data.Start(); // Assert var span = sut.GetMappedSpan(data.SpanId); @@ -127,12 +135,10 @@ public void OnStart_WithParentSpanId_StartsChildSpan() var sut = _fixture.GetSut(); using var parent = Tracer.StartActivity("Parent"); - sut.OnStart(parent); - using var data = Tracer.StartActivity("TestActivity"); // Act - sut.OnStart(data!); + using var data = Tracer.StartActivity("TestActivity"); // Assert var span = sut.GetMappedSpan(data.SpanId); @@ -167,10 +173,9 @@ public void OnStart_WithoutParentSpanId_StartsNewTransaction() _fixture.ScopeManager = Substitute.For(); var sut = _fixture.GetSut(); - var data = Tracer.StartActivity("test op"); // Act - sut.OnStart(data!); + var data = Tracer.StartActivity("test op"); // Assert var span = sut.GetMappedSpan(data.SpanId); @@ -200,20 +205,20 @@ public void OnEnd_FinishesSpan() // Arrange var sut = _fixture.GetSut(); - var parent = Tracer.StartActivity(name: "transaction")!; - sut.OnStart(parent); + var parent = Tracer.CreateActivity(name: "transaction", ActivityKind.Internal)!; + parent.Start(); var tags = new Dictionary { { "foo", "bar" } }; - var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; + var data = Tracer.CreateActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; data.DisplayName = "test display name"; - sut.OnStart(data); + data.Start(); var span = sut.GetMappedSpan(data.SpanId); // Act - sut.OnEnd(data); + data.Stop(); // Assert if (span is not SpanTracer spanTracer) @@ -246,12 +251,12 @@ public void OnEnd_Transaction_RestoresSavedScope() var sut = _fixture.GetSut(); var scope = new Scope(); - var data = Tracer.StartActivity("transaction")!; + var data = Tracer.CreateActivity("transaction", ActivityKind.Internal)!; data.SetFused(scope); - sut.OnStart(data); + data.Start(); // Act - sut.OnEnd(data); + data.Stop(); // Assert _fixture.ScopeManager.Received(1).RestoreScope(scope); @@ -265,16 +270,16 @@ public void OnEnd_Span_RestoresSavedScope() var sut = _fixture.GetSut(); var scope = new Scope(); - var parent = Tracer.StartActivity("transaction")!; + var parent = Tracer.CreateActivity("transaction", ActivityKind.Internal)!; parent.SetFused(scope); - sut.OnStart(parent); + parent.Start(); - var data = Tracer.StartActivity("test operation")!; + var data = Tracer.CreateActivity("test operation", ActivityKind.Internal)!; data.DisplayName = "test display name"; - sut.OnStart(data); + data.Start(); // Act - sut.OnEnd(data); + data.Stop(); // Assert _fixture.ScopeManager.Received(1).RestoreScope(scope); @@ -383,7 +388,6 @@ public void PruneFilteredSpans_RecentlyPruned_DoesNothing() public void OnEnd_FinishesTransaction() { // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; var sut = _fixture.GetSut(); var tags = new Dictionary { @@ -427,11 +431,10 @@ public void OnEnd_FinishesTransaction() public void OnEnd_IsSentryRequest_DoesNotFinishTransaction(string urlKey) { // Arrange - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; var sut = _fixture.GetSut(); var tags = new Dictionary { { "foo", "bar" }, { urlKey, _fixture.Options.Dsn } }; - var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; + var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; data.DisplayName = "test display name"; sut.OnStart(data); diff --git a/test/Sentry.OpenTelemetry.Tests/ActivitySourceTests.cs b/test/Sentry.OpenTelemetry.Tests/ActivitySourceTests.cs index c7bfdc9cc1..c557c37b45 100644 --- a/test/Sentry.OpenTelemetry.Tests/ActivitySourceTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/ActivitySourceTests.cs @@ -11,7 +11,7 @@ public abstract class ActivitySourceTests : IDisposable protected ActivitySourceTests() { - var activitySourceName = "SentrySpanProcessorTests"; + var activitySourceName = "ActivitySourceTests"; var testSampler = new TestSampler(); Tracer = new ActivitySource(activitySourceName); _traceProvider = Sdk.CreateTracerProviderBuilder() diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index cca662c335..cf86681702 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -702,6 +702,7 @@ namespace Sentry public void DisableAppDomainUnhandledExceptionCapture() { } public void DisableDiagnosticSourceIntegration() { } public void DisableDuplicateEventDetection() { } + public void DisableSentryTracingIntegration() { } public void DisableUnobservedTaskExceptionCapture() { } public void DisableWinUiUnhandledExceptionIntegration() { } public System.Collections.Generic.IEnumerable GetAllEventProcessors() { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt index cca662c335..cf86681702 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt @@ -702,6 +702,7 @@ namespace Sentry public void DisableAppDomainUnhandledExceptionCapture() { } public void DisableDiagnosticSourceIntegration() { } public void DisableDuplicateEventDetection() { } + public void DisableSentryTracingIntegration() { } public void DisableUnobservedTaskExceptionCapture() { } public void DisableWinUiUnhandledExceptionIntegration() { } public System.Collections.Generic.IEnumerable GetAllEventProcessors() { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt index d61bb3c82d..2d82ff7d18 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt @@ -703,6 +703,7 @@ namespace Sentry public void DisableAppDomainUnhandledExceptionCapture() { } public void DisableDiagnosticSourceIntegration() { } public void DisableDuplicateEventDetection() { } + public void DisableSentryTracingIntegration() { } public void DisableSystemDiagnosticsMetricsIntegration() { } public void DisableUnobservedTaskExceptionCapture() { } public void DisableWinUiUnhandledExceptionIntegration() { } diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.DotNet.verified.txt index dfbc55fc32..7ad8093f44 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.DotNet.verified.txt @@ -34,5 +34,11 @@ Args: [ SentryDiagnosticListenerIntegration ] + }, + { + Message: Registering integration: '{0}'., + Args: [ + SentryTracingIntegration + ] } ] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.DotNet.verified.txt index dfbc55fc32..7ad8093f44 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.DotNet.verified.txt @@ -34,5 +34,11 @@ Args: [ SentryDiagnosticListenerIntegration ] + }, + { + Message: Registering integration: '{0}'., + Args: [ + SentryTracingIntegration + ] } ] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.DotNet.verified.txt index 9e3f2681b5..aa2621657e 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.DotNet.verified.txt @@ -44,5 +44,11 @@ { Level: info, Message: System.Diagnostics.Metrics Integration is disabled because no listeners are configured. + }, + { + Message: Registering integration: '{0}'., + Args: [ + SentryTracingIntegration + ] } ] \ No newline at end of file From d50c26b247e971bb91f93b4320b3da9f2f645c16 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Wed, 27 Mar 2024 08:03:41 +0000 Subject: [PATCH 05/11] Format code --- .../Internal/Tracing/ActivitySpanProcessorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs b/test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs index 39e8c52932..35c880825f 100644 --- a/test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs +++ b/test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs @@ -434,7 +434,7 @@ public void OnEnd_IsSentryRequest_DoesNotFinishTransaction(string urlKey) var sut = _fixture.GetSut(); var tags = new Dictionary { { "foo", "bar" }, { urlKey, _fixture.Options.Dsn } }; - var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; + var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!; data.DisplayName = "test display name"; sut.OnStart(data); From 92ed0f28903ca55438b066f67b7722f66d216071 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 28 Mar 2024 12:34:55 +1300 Subject: [PATCH 06/11] Fixed Activities starting with empty IDs on .NET Framework --- CHANGELOG.md | 3 ++- .../Internal/Tracing/ActivitySpanProcessor.cs | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbbcb63b73..eb6471f8a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,8 @@ ### API changes - Removed `SentryOptionsExtensions` class - all the public methods moved directly to `SentryOptions` ([#3195](https://github.com/getsentry/sentry-dotnet/pull/3195)) - +- Sentry now uses System.Diagnostics.DiagnosticSource internally for tracing. Any traces created by Sentry can now be captured via OpenTelemetry. If you are using .NET Core 3.1 or earlier, you will need to add the `Sentry.DiagnosticSource` package to your solution to enable tracing. ([#3238](https://github.com/getsentry/sentry-dotnet/pull/3238)) + ### Dependencies - Bump CLI from v2.30.0 to v2.30.2 ([#3214](https://github.com/getsentry/sentry-dotnet/pull/3214), [#3218](https://github.com/getsentry/sentry-dotnet/pull/3218)) diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs index 2aa3368698..69db9761ee 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs @@ -23,6 +23,17 @@ internal class ActivitySpanProcessor internal long _lastPruned = 0; private readonly Lazy _realHub; + static ActivitySpanProcessor() + { +#if !NET5_0_OR_GREATER + // TODO: Could customers potentially be relying on the Hierarchical format? If so, this will get us in trouble. + // Activity.SpanId gets a non-zero value only if the activity ID format is W3C. The default is W3C since .NET 5, + // but even in new versions of the System.Diagnostics.DiagnosticSource package, the new defaults only apply when + // your app is running on modern .NET, and it keeps using the older Hierarchical format on .NET Framework. + Activity.DefaultIdFormat = ActivityIdFormat.W3C; +#endif + } + internal ActivitySpanProcessor(IHub hub) : this(hub, null, null, Instrumenter.Sentry) { From 0531ac7d68c61dece4d29342a8084bfb8f1edcda Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 28 Mar 2024 22:08:07 +1300 Subject: [PATCH 07/11] Fallback to sentry tracing rather than disabled tracing if DiagnosticSource is unavailable --- .../Internal/Tracing/ActivitySpanProcessor.cs | 22 ++++++--- .../Internal/Tracing/ActivityTraceProvider.cs | 15 ++++++ ...ctivityWrapper.cs => ActivityTraceSpan.cs} | 2 +- ...vitySourceWrapper.cs => ActivityTracer.cs} | 10 ++-- ...ation.cs => ActivityTracingIntegration.cs} | 6 +-- .../Tracing/SentryActivityListener.cs | 2 +- .../Internal/Tracing/SentryTraceProvider.cs | 15 ------ src/Sentry/HubExtensions.cs | 12 +++++ src/Sentry/IMetricHub.cs | 4 +- src/Sentry/Instrumenter.cs | 5 ++ src/Sentry/Internal/DisabledSpan.cs | 32 ------------- src/Sentry/Internal/DisabledTraceProvider.cs | 8 ---- src/Sentry/Internal/DisabledTracer.cs | 9 ---- src/Sentry/Internal/Hub.cs | 24 +++------- src/Sentry/Internal/ISentryTraceProvider.cs | 6 --- src/Sentry/Internal/ISentryTracer.cs | 7 --- src/Sentry/Internal/Tracing/ITraceProvider.cs | 6 +++ .../{ISentrySpan.cs => Tracing/ITraceSpan.cs} | 4 +- src/Sentry/Internal/Tracing/ITracer.cs | 7 +++ .../Internal/Tracing/SentryTraceProvider.cs | 11 +++++ .../Internal/Tracing/SentryTraceSpan.cs | 48 +++++++++++++++++++ src/Sentry/Internal/Tracing/SentryTracer.cs | 13 +++++ .../Tracing/SentryTracingIntegration.cs | 19 ++++++++ src/Sentry/Sentry.csproj | 2 +- src/Sentry/SentryOptions.cs | 36 +++++++------- .../Tracing/ActivitySpanProcessorTests.cs | 4 +- ...piApprovalTests.Run.DotNet6_0.verified.txt | 5 +- ...piApprovalTests.Run.DotNet7_0.verified.txt | 5 +- ...piApprovalTests.Run.DotNet8_0.verified.txt | 5 +- .../ApiApprovalTests.Run.Net4_8.verified.txt | 3 +- test/Sentry.Tests/HubTests.cs | 40 ---------------- ...y_registered.DotNet6_0.DotNet.verified.txt | 2 +- ...y_registered.DotNet7_0.DotNet.verified.txt | 2 +- ...y_registered.DotNet8_0.DotNet.verified.txt | 2 +- 34 files changed, 205 insertions(+), 188 deletions(-) create mode 100644 src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceProvider.cs rename src/Sentry.DiagnosticSource/Internal/Tracing/{ActivityWrapper.cs => ActivityTraceSpan.cs} (92%) rename src/Sentry.DiagnosticSource/Internal/Tracing/{ActivitySourceWrapper.cs => ActivityTracer.cs} (58%) rename src/Sentry.DiagnosticSource/Internal/Tracing/{SentryTracingIntegration.cs => ActivityTracingIntegration.cs} (83%) delete mode 100644 src/Sentry.DiagnosticSource/Internal/Tracing/SentryTraceProvider.cs delete mode 100644 src/Sentry/Internal/DisabledSpan.cs delete mode 100644 src/Sentry/Internal/DisabledTraceProvider.cs delete mode 100644 src/Sentry/Internal/DisabledTracer.cs delete mode 100644 src/Sentry/Internal/ISentryTraceProvider.cs delete mode 100644 src/Sentry/Internal/ISentryTracer.cs create mode 100644 src/Sentry/Internal/Tracing/ITraceProvider.cs rename src/Sentry/Internal/{ISentrySpan.cs => Tracing/ITraceSpan.cs} (69%) create mode 100644 src/Sentry/Internal/Tracing/ITracer.cs create mode 100644 src/Sentry/Internal/Tracing/SentryTraceProvider.cs create mode 100644 src/Sentry/Internal/Tracing/SentryTraceSpan.cs create mode 100644 src/Sentry/Internal/Tracing/SentryTracer.cs create mode 100644 src/Sentry/Internal/Tracing/SentryTracingIntegration.cs diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs index 69db9761ee..75450dd027 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs @@ -26,21 +26,29 @@ internal class ActivitySpanProcessor static ActivitySpanProcessor() { #if !NET5_0_OR_GREATER + if (Activity.DefaultIdFormat == ActivityIdFormat.W3C) + { + return; + } + // TODO: Could customers potentially be relying on the Hierarchical format? If so, this will get us in trouble. - // Activity.SpanId gets a non-zero value only if the activity ID format is W3C. The default is W3C since .NET 5, - // but even in new versions of the System.Diagnostics.DiagnosticSource package, the new defaults only apply when - // your app is running on modern .NET, and it keeps using the older Hierarchical format on .NET Framework. + // + // Another option would be to warn customers and have them set this themselves (that would be more deliberate). + // + // Activity.SpanId only gets a non-zero value if the activity ID format is W3C (the default since net5.0). The + // default is Hierarchical on .NET Framework, .NET Core 3.1 and below, which won't work with Sentry tracing. + Debug.WriteLine("Setting Activity.DefaultIdFormat to W3C."); Activity.DefaultIdFormat = ActivityIdFormat.W3C; #endif } - internal ActivitySpanProcessor(IHub hub) - : this(hub, null, null, Instrumenter.Sentry) + internal ActivitySpanProcessor(IHub hub, Instrumenter instrumenter) + : this(hub, null, null, instrumenter) { } - internal ActivitySpanProcessor(IHub hub, Action? beforeFinish = null, - Func>? resourceAttributeResolver = null) + internal ActivitySpanProcessor(IHub hub, Action? beforeFinish, + Func>? resourceAttributeResolver) : this(hub, beforeFinish, resourceAttributeResolver, Instrumenter.OpenTelemetry) { } diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceProvider.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceProvider.cs new file mode 100644 index 0000000000..740754c8ee --- /dev/null +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceProvider.cs @@ -0,0 +1,15 @@ +namespace Sentry.Internal.Tracing; + +/// +/// The concrete implementation of that uses +/// and from the +/// namespace to implement tracing. +/// +internal class ActivityTraceProvider : ITraceProvider +{ + private Lazy> _lazyActivitySources = new(); + private ConcurrentDictionary _activitySources => _lazyActivitySources.Value; + + public ITracer GetTracer(string name, string? version = "") + => _activitySources.GetOrAdd(name, new ActivityTracer(name, version)); +} diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceSpan.cs similarity index 92% rename from src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs rename to src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceSpan.cs index 8b57c354d8..85516c0cfd 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityWrapper.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceSpan.cs @@ -9,7 +9,7 @@ namespace Sentry.Internal.Tracing; /// access this from our integrations without taking a hard dependency on /// System.Diagnostics.Activity (which is only available in .NET 5.0 and later) /// -internal class ActivityWrapper(System.Diagnostics.Activity activity) : ISentrySpan +internal class ActivityTraceSpan(System.Diagnostics.Activity activity) : ITraceSpan { public void SetAttribute(string key, object value) => activity.SetTag(key, value); diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySourceWrapper.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracer.cs similarity index 58% rename from src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySourceWrapper.cs rename to src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracer.cs index ef08b92f4e..4a38130fce 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySourceWrapper.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracer.cs @@ -5,14 +5,14 @@ namespace Sentry.Internal.Tracing; /// can access this from our integrations without taking a hard dependency on /// System.Diagnostics.ActivitySource (which is only available in .NET 5.0 and later) /// -internal class ActivitySourceWrapper(string name, string? version = "") : ISentryTracer +internal class ActivityTracer(string name, string? version = "") : ITracer { private readonly ActivitySource _activitySource = new(name, version); - public ISentrySpan? StartSpan(string operationName) => - _activitySource.StartActivity(operationName) is { } activity ? new ActivityWrapper(activity) : null; + public ITraceSpan? StartSpan(string operationName) => + _activitySource.StartActivity(operationName) is { } activity ? new ActivityTraceSpan(activity) : null; - public ISentrySpan? CurrentSpan => System.Diagnostics.Activity.Current == null + public ITraceSpan? CurrentSpan => System.Diagnostics.Activity.Current == null ? null - : new ActivityWrapper(System.Diagnostics.Activity.Current); + : new ActivityTraceSpan(System.Diagnostics.Activity.Current); } diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracingIntegration.cs similarity index 83% rename from src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs rename to src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracingIntegration.cs index 4f9630b61d..68e33ca105 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTracingIntegration.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracingIntegration.cs @@ -3,7 +3,7 @@ namespace Sentry.Internal.Tracing; -internal class SentryTracingIntegration : ISdkIntegration +internal class ActivityTracingIntegration : ISdkIntegration { /* TODO: Think about where to put this... would be good if it sat on an IDisposable but also something internal. @@ -23,7 +23,7 @@ public void Register(IHub hub, SentryOptions options) } // TODO: Should we be registering this if OpenTelemetry is enabled? - options.SentryTraceProvider = new SentryTraceProvider(); - _listener = new SentryActivityListener(hub); + options.InternalTraceProvider = new ActivityTraceProvider(); + _listener = new SentryActivityListener(hub, Instrumenter.ActivitySource); } } diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs index bee544e858..ebfa8f7a90 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryActivityListener.cs @@ -5,7 +5,7 @@ internal class SentryActivityListener : IDisposable private readonly ActivitySpanProcessor _activitySpanProcessor; private readonly ActivityListener? _listener; - public SentryActivityListener(IHub hub) : this(new ActivitySpanProcessor(hub)) + public SentryActivityListener(IHub hub, Instrumenter instrumenter) : this(new ActivitySpanProcessor(hub, instrumenter)) { } diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTraceProvider.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTraceProvider.cs deleted file mode 100644 index 497354eabd..0000000000 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/SentryTraceProvider.cs +++ /dev/null @@ -1,15 +0,0 @@ -namespace Sentry.Internal.Tracing; - -/// -/// The default concrete implementation of that uses -/// and from the -/// namespace to implement tracing. -/// -internal class SentryTraceProvider : ISentryTraceProvider -{ - private Lazy> _lazyActivitySources = new(); - private ConcurrentDictionary _activitySources => _lazyActivitySources.Value; - - public ISentryTracer GetTracer(string name, string? version = "") - => _activitySources.GetOrAdd(name, new ActivitySourceWrapper(name, version)); -} diff --git a/src/Sentry/HubExtensions.cs b/src/Sentry/HubExtensions.cs index a4bedfddec..7735c9bb6b 100644 --- a/src/Sentry/HubExtensions.cs +++ b/src/Sentry/HubExtensions.cs @@ -11,6 +11,18 @@ namespace Sentry; [EditorBrowsable(EditorBrowsableState.Never)] public static class HubExtensions { + /// + /// Starts a child span for the current transaction or, if there is no active transaction, starts a new transaction. + /// + internal static ISpan StartSpan(this IHub hub, string operation, string description) + { + ITransactionTracer? currentTransaction = null; + hub.ConfigureScope(s => currentTransaction = s.Transaction); + return currentTransaction is { } transaction + ? transaction.StartChild(operation, description) + : hub.StartTransaction(operation, description); + } + /// /// Starts a transaction. /// diff --git a/src/Sentry/IMetricHub.cs b/src/Sentry/IMetricHub.cs index 99f9d7d778..c486e8cb52 100644 --- a/src/Sentry/IMetricHub.cs +++ b/src/Sentry/IMetricHub.cs @@ -14,9 +14,7 @@ internal interface IMetricHub /// void CaptureCodeLocations(CodeLocations codeLocations); - /// - /// Starts a child span for the current transaction or, if there is no active transaction, starts a new transaction. - /// + /// ISpan StartSpan(string operation, string description); /// diff --git a/src/Sentry/Instrumenter.cs b/src/Sentry/Instrumenter.cs index f169216255..cdf3bfe575 100644 --- a/src/Sentry/Instrumenter.cs +++ b/src/Sentry/Instrumenter.cs @@ -10,6 +10,11 @@ public enum Instrumenter /// Sentry, + /// + /// Spans are instrumented with Sentry using ActivitySource. + /// + ActivitySource, + /// /// Spans are instrumented via OpenTelemetry. /// diff --git a/src/Sentry/Internal/DisabledSpan.cs b/src/Sentry/Internal/DisabledSpan.cs deleted file mode 100644 index 645314cd64..0000000000 --- a/src/Sentry/Internal/DisabledSpan.cs +++ /dev/null @@ -1,32 +0,0 @@ -namespace Sentry.Internal; - -internal class DisabledSpan : ISentrySpan -{ - private static readonly Lazy LazyInstance = new(); - public static DisabledSpan Instance => LazyInstance.Value; - - public void Dispose() - { - // No-Op - } - - public void SetAttribute(string key, object value) - { - // No-Op - } - - public void AddEvent(string message) - { - // No-Op - } - - public void SetStatus(SpanStatus status, string? description = default) - { - // No-Op - } - - public void Stop() - { - // No-Op - } -} diff --git a/src/Sentry/Internal/DisabledTraceProvider.cs b/src/Sentry/Internal/DisabledTraceProvider.cs deleted file mode 100644 index 16b564dce2..0000000000 --- a/src/Sentry/Internal/DisabledTraceProvider.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace Sentry.Internal; - -internal class DisabledTraceProvider : ISentryTraceProvider -{ - private static readonly Lazy LazyInstance = new(); - public static DisabledTraceProvider Instance => LazyInstance.Value; - public ISentryTracer GetTracer(string name, string? version = "") => DisabledTracer.Instance; -} diff --git a/src/Sentry/Internal/DisabledTracer.cs b/src/Sentry/Internal/DisabledTracer.cs deleted file mode 100644 index 2f5fd097cc..0000000000 --- a/src/Sentry/Internal/DisabledTracer.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace Sentry.Internal; - -internal class DisabledTracer : ISentryTracer -{ - private static readonly Lazy LazyInstance = new(); - public static DisabledTracer Instance => LazyInstance.Value; - public ISentrySpan StartSpan(string operationName) => DisabledSpan.Instance; - public ISentrySpan? CurrentSpan => DisabledSpan.Instance; -} diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 1a82aa4fa9..7f08c51ba6 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -1,5 +1,6 @@ using Sentry.Extensibility; using Sentry.Infrastructure; +using Sentry.Internal.Tracing; using Sentry.Protocol.Envelopes; using Sentry.Protocol.Metrics; @@ -20,7 +21,7 @@ internal class Hub : IHub, IMetricHub, IDisposable // Internal for testability internal ConditionalWeakTable ExceptionToSpanMap { get; } = new(); - internal IInternalScopeManager ScopeManager { get; } + public IInternalScopeManager ScopeManager { get; } /// public IMetricAggregator Metrics { get; } @@ -75,6 +76,9 @@ internal Hub( options.LogDebug("Registering integration: '{0}'.", integration.GetType().Name); integration.Register(this, options); } + + // If no tracing integration was registered, fall back to Sentry tracing + options.InternalTraceProvider ??= new SentryTraceProvider(this); } public void ConfigureScope(Action configureScope) @@ -119,16 +123,6 @@ internal ITransactionTracer StartTransaction( IReadOnlyDictionary customSamplingContext, DynamicSamplingContext? dynamicSamplingContext) { - var instrumenter = (context as SpanContext)?.Instrumenter; - if (instrumenter != _options.Instrumenter) - { - _options.LogWarning( - $"Attempted to start a transaction via {instrumenter} instrumentation when the SDK is" + - $" configured for {_options.Instrumenter} instrumentation. The transaction will not be created."); - - return NoOpTransaction.Instance; - } - var transaction = new TransactionTracer(this, context); // If the hub is disabled, we will always sample out. In other words, starting a transaction @@ -539,14 +533,10 @@ public void CaptureCodeLocations(CodeLocations codeLocations) } } - /// + /// public ISpan StartSpan(string operation, string description) { - ITransactionTracer? currentTransaction = null; - ConfigureScope(s => currentTransaction = s.Transaction); - return currentTransaction is { } transaction - ? transaction.StartChild(operation, description) - : this.StartTransaction(operation, description); + return HubExtensions.StartSpan(this, operation, description); } public void CaptureSession(SessionUpdate sessionUpdate) diff --git a/src/Sentry/Internal/ISentryTraceProvider.cs b/src/Sentry/Internal/ISentryTraceProvider.cs deleted file mode 100644 index 9a48c57f02..0000000000 --- a/src/Sentry/Internal/ISentryTraceProvider.cs +++ /dev/null @@ -1,6 +0,0 @@ -namespace Sentry.Internal; - -internal interface ISentryTraceProvider -{ - public ISentryTracer GetTracer(string name, string? version = ""); -} diff --git a/src/Sentry/Internal/ISentryTracer.cs b/src/Sentry/Internal/ISentryTracer.cs deleted file mode 100644 index 8421bfd7ae..0000000000 --- a/src/Sentry/Internal/ISentryTracer.cs +++ /dev/null @@ -1,7 +0,0 @@ -namespace Sentry.Internal; - -internal interface ISentryTracer -{ - ISentrySpan? StartSpan(string operationName); - ISentrySpan? CurrentSpan { get; } -} diff --git a/src/Sentry/Internal/Tracing/ITraceProvider.cs b/src/Sentry/Internal/Tracing/ITraceProvider.cs new file mode 100644 index 0000000000..f1bd67a537 --- /dev/null +++ b/src/Sentry/Internal/Tracing/ITraceProvider.cs @@ -0,0 +1,6 @@ +namespace Sentry.Internal.Tracing; + +internal interface ITraceProvider +{ + public ITracer GetTracer(string name, string? version = ""); +} diff --git a/src/Sentry/Internal/ISentrySpan.cs b/src/Sentry/Internal/Tracing/ITraceSpan.cs similarity index 69% rename from src/Sentry/Internal/ISentrySpan.cs rename to src/Sentry/Internal/Tracing/ITraceSpan.cs index fa2109c58b..6e86c9ebc6 100644 --- a/src/Sentry/Internal/ISentrySpan.cs +++ b/src/Sentry/Internal/Tracing/ITraceSpan.cs @@ -1,6 +1,6 @@ -namespace Sentry.Internal; +namespace Sentry.Internal.Tracing; -internal interface ISentrySpan : IDisposable +internal interface ITraceSpan : IDisposable { void SetAttribute(string key, object value); void AddEvent(string message); diff --git a/src/Sentry/Internal/Tracing/ITracer.cs b/src/Sentry/Internal/Tracing/ITracer.cs new file mode 100644 index 0000000000..6ad91b1087 --- /dev/null +++ b/src/Sentry/Internal/Tracing/ITracer.cs @@ -0,0 +1,7 @@ +namespace Sentry.Internal.Tracing; + +internal interface ITracer +{ + ITraceSpan? StartSpan(string operationName); + ITraceSpan? CurrentSpan { get; } +} diff --git a/src/Sentry/Internal/Tracing/SentryTraceProvider.cs b/src/Sentry/Internal/Tracing/SentryTraceProvider.cs new file mode 100644 index 0000000000..ee02f1807a --- /dev/null +++ b/src/Sentry/Internal/Tracing/SentryTraceProvider.cs @@ -0,0 +1,11 @@ +namespace Sentry.Internal.Tracing; + +internal class SentryTraceProvider(IHub hub) : ITraceProvider +{ + private readonly SentryTracer _tracer = new(hub); + + // Sentry doesn't have the same concept of "Tracers" as the DiagnosticSource classes do, so we always return the + // same tracer for Sentry... it's just a wrapper around the Hub which is actually what Starts and Stops spans when + // using Sentry tracing. + public ITracer GetTracer(string name, string? version = "") => _tracer; +} diff --git a/src/Sentry/Internal/Tracing/SentryTraceSpan.cs b/src/Sentry/Internal/Tracing/SentryTraceSpan.cs new file mode 100644 index 0000000000..bef5015d1a --- /dev/null +++ b/src/Sentry/Internal/Tracing/SentryTraceSpan.cs @@ -0,0 +1,48 @@ +namespace Sentry.Internal.Tracing; + +internal class SentryTraceSpan : ITraceSpan +{ + private readonly ISpan _span; + private Scope? _scope; + + public SentryTraceSpan(IHub hub, ISpan span) + { + _span = span; + hub.ConfigureScope(scope => _scope = scope); + } + + public void Dispose() + { + // ISpan doesn't implement IDisposable + } + + public void SetAttribute(string key, object value) + { + var stringValue = $"{value}"; + if (string.IsNullOrWhiteSpace(stringValue)) + { + _span.UnsetTag(key); + return; + } + _span.SetTag(key, stringValue); + } + + public void AddEvent(string message) + { + _scope?.AddBreadcrumb(message); + } + + public void SetStatus(SpanStatus status, string? description = default) + { + _span.Status = status; + if (_span.Status != SpanStatus.Ok) + { + _span.Description = description; + } + } + + public void Stop() + { + _span.Finish(); + } +} diff --git a/src/Sentry/Internal/Tracing/SentryTracer.cs b/src/Sentry/Internal/Tracing/SentryTracer.cs new file mode 100644 index 0000000000..3a5e72feb1 --- /dev/null +++ b/src/Sentry/Internal/Tracing/SentryTracer.cs @@ -0,0 +1,13 @@ +namespace Sentry.Internal.Tracing; + +internal class SentryTracer(IHub hub) : ITracer +{ + public ITraceSpan StartSpan(string operationName) => new SentryTraceSpan( + hub, + hub.StartSpan(operationName, operationName) + ); + + public ITraceSpan? CurrentSpan => hub.GetSpan() is {} span + ? new SentryTraceSpan(hub, span) + : null; +} diff --git a/src/Sentry/Internal/Tracing/SentryTracingIntegration.cs b/src/Sentry/Internal/Tracing/SentryTracingIntegration.cs new file mode 100644 index 0000000000..28b51beb16 --- /dev/null +++ b/src/Sentry/Internal/Tracing/SentryTracingIntegration.cs @@ -0,0 +1,19 @@ +using Sentry.Extensibility; +using Sentry.Integrations; + +namespace Sentry.Internal.Tracing; + +internal class SentryTracingIntegration : ISdkIntegration +{ + public void Register(IHub hub, SentryOptions options) + { + if (!options.IsPerformanceMonitoringEnabled) + { + options.Log(SentryLevel.Info, "SentryTracing Integration is disabled because tracing is disabled."); + return; + } + + // TODO: Should we be registering this if OpenTelemetry is enabled? + options.InternalTraceProvider = new SentryTraceProvider(hub); + } +} diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index 2f71469b59..f08e8c4dc8 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -52,7 +52,7 @@ $(DefineConstants);HAS_DIAGNOSTIC_INTEGRATION - $(DefineConstants);HAS_TRACING_INTEGRATION + $(DefineConstants);HAS_ACTIVITY_TRACING_INTEGRATION diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index 68f67e2717..d4b42436f6 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -13,9 +13,7 @@ using Sentry.Internal.DiagnosticSource; #endif -#if HAS_TRACING_INTEGRATION using Sentry.Internal.Tracing; -#endif #if ANDROID using Sentry.Android; @@ -206,10 +204,10 @@ internal IEnumerable Integrations } #endif -#if HAS_TRACING_INTEGRATION - if (_defaultIntegrations.Includes(DefaultIntegrations.SentryTracingIntegration)) +#if HAS_ACTIVITY_TRACING_INTEGRATION + if (_defaultIntegrations.Includes(DefaultIntegrations.ActivityTracingIntegration)) { - yield return new SentryTracingIntegration(); + yield return new ActivityTracingIntegration(); } #endif @@ -234,13 +232,13 @@ internal IEnumerable Integrations internal ISentryHttpClientFactory? SentryHttpClientFactory { get; set; } - private ISentryTraceProvider? _sentryTraceProvider; - - internal ISentryTraceProvider SentryTraceProvider - { - get => _sentryTraceProvider ?? DisabledTraceProvider.Instance; - set => _sentryTraceProvider = value; - } + /// + /// The trace provider to be used by the Sentry SDK and it's integrations when creating spans. For net5.0 and later + /// the ActivityTraceProvider is used by default. Earlier versions of .NET (including .NET Framework) default to the + /// SentryTraceProvider. Users of those earlier versions of .NET can manually override that behaviour by registering + /// the ActivityTraceProvider via our Sentry.DiagnosticSource integration. + /// + internal ITraceProvider? InternalTraceProvider { get; set; } internal HttpClient GetHttpClient() { @@ -1243,8 +1241,8 @@ public SentryOptions() #if NET8_0_OR_GREATER | DefaultIntegrations.SystemDiagnosticsMetricsIntegration #endif -#if HAS_TRACING_INTEGRATION - | DefaultIntegrations.SentryTracingIntegration +#if HAS_ACTIVITY_TRACING_INTEGRATION + | DefaultIntegrations.ActivityTracingIntegration #endif ; @@ -1630,12 +1628,12 @@ public void DisableSystemDiagnosticsMetricsIntegration() => RemoveDefaultIntegration(DefaultIntegrations.SystemDiagnosticsMetricsIntegration); #endif -#if HAS_TRACING_INTEGRATION +#if HAS_ACTIVITY_TRACING_INTEGRATION /// /// Disables the Sentry Tracing integration. /// - public void DisableSentryTracingIntegration() - => RemoveDefaultIntegration(DefaultIntegrations.SentryTracingIntegration); + public void DisableActivityTracingIntegration() + => RemoveDefaultIntegration(DefaultIntegrations.ActivityTracingIntegration); #endif internal bool HasIntegration() => _integrations.Any(integration => integration is TIntegration); @@ -1661,8 +1659,8 @@ internal enum DefaultIntegrations #if NET8_0_OR_GREATER SystemDiagnosticsMetricsIntegration = 1 << 7, #endif -#if HAS_TRACING_INTEGRATION - SentryTracingIntegration = 1 << 8, +#if HAS_ACTIVITY_TRACING_INTEGRATION + ActivityTracingIntegration = 1 << 8, #endif } diff --git a/test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs b/test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs index 35c880825f..22172554e1 100644 --- a/test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs +++ b/test/Sentry.DiagnosticSource.Tests/Internal/Tracing/ActivitySpanProcessorTests.cs @@ -30,7 +30,7 @@ public Fixture() AutoSessionTracking = false }; #if NET5_0_OR_GREATER - Options.DisableSentryTracingIntegration(); // We'll create our own for these tests... + Options.DisableActivityTracingIntegration(); // We'll create our own for these tests... #endif Client = Substitute.For(); @@ -42,7 +42,7 @@ public Fixture() public ActivitySpanProcessor GetSut() { - var processor = new ActivitySpanProcessor(GetHub()); + var processor = new ActivitySpanProcessor(GetHub(), Instrumenter.ActivitySource); Listener = new SentryActivityListener(processor); return processor; } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index cf86681702..71d9514f0e 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -317,7 +317,8 @@ namespace Sentry public enum Instrumenter { Sentry = 0, - OpenTelemetry = 1, + ActivitySource = 1, + OpenTelemetry = 2, } public readonly struct MeasurementUnit : System.IEquatable { @@ -698,11 +699,11 @@ namespace Sentry public void AddTransactionProcessorProvider(System.Func> processorProvider) { } public void AddTransactionProcessors(System.Collections.Generic.IEnumerable processors) { } public void ApplyDefaultTags(Sentry.IHasTags hasTags) { } + public void DisableActivityTracingIntegration() { } public void DisableAppDomainProcessExitFlush() { } public void DisableAppDomainUnhandledExceptionCapture() { } public void DisableDiagnosticSourceIntegration() { } public void DisableDuplicateEventDetection() { } - public void DisableSentryTracingIntegration() { } public void DisableUnobservedTaskExceptionCapture() { } public void DisableWinUiUnhandledExceptionIntegration() { } public System.Collections.Generic.IEnumerable GetAllEventProcessors() { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt index cf86681702..71d9514f0e 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt @@ -317,7 +317,8 @@ namespace Sentry public enum Instrumenter { Sentry = 0, - OpenTelemetry = 1, + ActivitySource = 1, + OpenTelemetry = 2, } public readonly struct MeasurementUnit : System.IEquatable { @@ -698,11 +699,11 @@ namespace Sentry public void AddTransactionProcessorProvider(System.Func> processorProvider) { } public void AddTransactionProcessors(System.Collections.Generic.IEnumerable processors) { } public void ApplyDefaultTags(Sentry.IHasTags hasTags) { } + public void DisableActivityTracingIntegration() { } public void DisableAppDomainProcessExitFlush() { } public void DisableAppDomainUnhandledExceptionCapture() { } public void DisableDiagnosticSourceIntegration() { } public void DisableDuplicateEventDetection() { } - public void DisableSentryTracingIntegration() { } public void DisableUnobservedTaskExceptionCapture() { } public void DisableWinUiUnhandledExceptionIntegration() { } public System.Collections.Generic.IEnumerable GetAllEventProcessors() { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt index 2d82ff7d18..124f188850 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt @@ -318,7 +318,8 @@ namespace Sentry public enum Instrumenter { Sentry = 0, - OpenTelemetry = 1, + ActivitySource = 1, + OpenTelemetry = 2, } public readonly struct MeasurementUnit : System.IEquatable { @@ -699,11 +700,11 @@ namespace Sentry public void AddTransactionProcessorProvider(System.Func> processorProvider) { } public void AddTransactionProcessors(System.Collections.Generic.IEnumerable processors) { } public void ApplyDefaultTags(Sentry.IHasTags hasTags) { } + public void DisableActivityTracingIntegration() { } public void DisableAppDomainProcessExitFlush() { } public void DisableAppDomainUnhandledExceptionCapture() { } public void DisableDiagnosticSourceIntegration() { } public void DisableDuplicateEventDetection() { } - public void DisableSentryTracingIntegration() { } public void DisableSystemDiagnosticsMetricsIntegration() { } public void DisableUnobservedTaskExceptionCapture() { } public void DisableWinUiUnhandledExceptionIntegration() { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt index 0ea27a9ec7..b09d55171d 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt @@ -316,7 +316,8 @@ namespace Sentry public enum Instrumenter { Sentry = 0, - OpenTelemetry = 1, + ActivitySource = 1, + OpenTelemetry = 2, } public readonly struct MeasurementUnit : System.IEquatable { diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index e99a9e304e..64a5988f3d 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -616,46 +616,6 @@ public void StartTransaction_DisableTracing_SampledOut() transaction.IsSampled.Should().BeFalse(); } - [Fact] - public void StartTransaction_SameInstrumenter_SampledIn() - { - // Arrange - _fixture.Options.EnableTracing = true; - _fixture.Options.Instrumenter = Instrumenter.Sentry; // The default... making it explicit for this test though - var hub = _fixture.GetSut(); - - var transactionContext = new TransactionContext("name", "operation") - { - Instrumenter = _fixture.Options.Instrumenter - }; - - // Act - var transaction = hub.StartTransaction(transactionContext); - - // Assert - transaction.IsSampled.Should().BeTrue(); - } - - [Fact] - public void StartTransaction_DifferentInstrumenter_NoOp() - { - // Arrange - _fixture.Options.EnableTracing = true; - _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - var hub = _fixture.GetSut(); - - var transactionContext = new TransactionContext("name", "operation") - { - Instrumenter = Instrumenter.Sentry // The default... making it explicit for this test though - }; - - // Act - var transaction = hub.StartTransaction(transactionContext); - - // Assert - transaction.Should().Be(NoOpTransaction.Instance); - } - [Fact] public void StartTransaction_EnableTracing_Sampler_SampledIn() { diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.DotNet.verified.txt index 7ad8093f44..3d12fd4b6b 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.DotNet.verified.txt @@ -38,7 +38,7 @@ { Message: Registering integration: '{0}'., Args: [ - SentryTracingIntegration + ActivityTracingIntegration ] } ] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.DotNet.verified.txt index 7ad8093f44..3d12fd4b6b 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.DotNet.verified.txt @@ -38,7 +38,7 @@ { Message: Registering integration: '{0}'., Args: [ - SentryTracingIntegration + ActivityTracingIntegration ] } ] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.DotNet.verified.txt index aa2621657e..3bb7b3eecf 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.DotNet.verified.txt @@ -48,7 +48,7 @@ { Message: Registering integration: '{0}'., Args: [ - SentryTracingIntegration + ActivityTracingIntegration ] } ] \ No newline at end of file From dcc6ed136f0ef575771bc6e2b8031714b1a20a45 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 28 Mar 2024 22:14:24 +1300 Subject: [PATCH 08/11] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb6471f8a5..5fe72d5117 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ ### API changes - Removed `SentryOptionsExtensions` class - all the public methods moved directly to `SentryOptions` ([#3195](https://github.com/getsentry/sentry-dotnet/pull/3195)) -- Sentry now uses System.Diagnostics.DiagnosticSource internally for tracing. Any traces created by Sentry can now be captured via OpenTelemetry. If you are using .NET Core 3.1 or earlier, you will need to add the `Sentry.DiagnosticSource` package to your solution to enable tracing. ([#3238](https://github.com/getsentry/sentry-dotnet/pull/3238)) +- Sentry now uses System.Diagnostics.DiagnosticSource internally for tracing by default when targeting .NET 6 or later, meaning any traces created by Sentry can be captured via OpenTelemetry. If you are using .NET Core 3.1 or earlier, you can opt into this via the `Sentry.DiagnosticSource` package. ([#3238](https://github.com/getsentry/sentry-dotnet/pull/3238)) ### Dependencies From 489de7f2e3955993242af4a998e2e73b3750b41c Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 28 Mar 2024 09:18:34 +0000 Subject: [PATCH 09/11] Format code --- src/Sentry/Internal/Tracing/SentryTracer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry/Internal/Tracing/SentryTracer.cs b/src/Sentry/Internal/Tracing/SentryTracer.cs index 3a5e72feb1..cd5b76b3a7 100644 --- a/src/Sentry/Internal/Tracing/SentryTracer.cs +++ b/src/Sentry/Internal/Tracing/SentryTracer.cs @@ -7,7 +7,7 @@ internal class SentryTracer(IHub hub) : ITracer hub.StartSpan(operationName, operationName) ); - public ITraceSpan? CurrentSpan => hub.GetSpan() is {} span + public ITraceSpan? CurrentSpan => hub.GetSpan() is { } span ? new SentryTraceSpan(hub, span) : null; } From f64d4cbebfe7fbf044e49d73d387ac02562c3ac9 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 3 Apr 2024 15:35:11 +1300 Subject: [PATCH 10/11] Sentry package now uses new trace interfaces --- .../Internal/Tracing/ActivitySpanProcessor.cs | 29 +++++-- .../Internal/Tracing/ActivityTraceSpan.cs | 52 ++++++++++-- .../Internal/Tracing/ActivityTracer.cs | 13 ++- .../Internal/Tracing/TraceExtensions.cs | 22 +++++ .../SentryHttpMessageHandlerBuilderFilter.cs | 2 +- src/Sentry/Internal/Tracing/ITraceSpan.cs | 27 +++++- src/Sentry/Internal/Tracing/ITracer.cs | 2 +- .../Internal/Tracing/SentryTraceSpan.cs | 40 +++++++-- src/Sentry/Internal/Tracing/SentryTracer.cs | 4 +- src/Sentry/SentryGraphQLHttpMessageHandler.cs | 85 ++++++++++++++++--- src/Sentry/SentryHttpMessageHandler.cs | 55 ++++++++++-- src/Sentry/SentryMessageHandler.cs | 80 +++++++++++++++-- ...piApprovalTests.Run.DotNet6_0.verified.txt | 6 ++ ...piApprovalTests.Run.DotNet7_0.verified.txt | 6 ++ ...piApprovalTests.Run.DotNet8_0.verified.txt | 6 ++ .../ApiApprovalTests.Run.Net4_8.verified.txt | 6 ++ .../SentryGraphQlHttpMessageHandlerTests.cs | 4 + .../SentryHttpMessageHandlerTests.cs | 2 + 18 files changed, 383 insertions(+), 58 deletions(-) diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs index 75450dd027..9258e080eb 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivitySpanProcessor.cs @@ -35,6 +35,10 @@ static ActivitySpanProcessor() // // Another option would be to warn customers and have them set this themselves (that would be more deliberate). // + // Finally, we can override the ActivityIdFormat for each trace by using ActivitySource.CreateActivity instead + // of ActivitySource.StartActivity. That's a bit more fragile and will only work for spans created by the SDK + // (not for anything users instrument themselves). + // // Activity.SpanId only gets a non-zero value if the activity ID format is W3C (the default since net5.0). The // default is Hierarchical on .NET Framework, .NET Core 3.1 and below, which won't work with Sentry tracing. Debug.WriteLine("Setting Activity.DefaultIdFormat to W3C."); @@ -104,8 +108,8 @@ public void OnStart(System.Diagnostics.Activity data) var span = (SpanTracer)parentSpan.StartChild(context); span.StartTimestamp = data.StartTimeUtc; // Used to filter out spans that are not recorded when finishing a transaction. - span.SetFused(data); - span.IsFiltered = () => span.GetFused() + data.BindSentrySpan(span); + span.IsFiltered = () => span.GetActivity() is { IsAllDataRequested: false, Recorded: false }; _map[data.SpanId] = span; } @@ -136,7 +140,7 @@ public void OnStart(System.Diagnostics.Activity data) ); transaction.StartTimestamp = data.StartTimeUtc; _hub.ConfigureScope(scope => scope.Transaction = transaction); - transaction.SetFused(data); + data.BindSentrySpan(transaction); _map[data.SpanId] = transaction; } @@ -215,10 +219,20 @@ public void OnEnd(System.Diagnostics.Activity data) hub?.RestoreScope(savedScope); } GenerateSentryErrorsFromOtelSpan(data, attributes); - - var status = GetSpanStatus(data.Status, attributes); _beforeFinish?.Invoke(span, data); - span.Finish(status); + if (data.GetException() is { } exception) + { + span.Finish(exception); + } + else + { + // TODO: Does this override a status that we might be setting manually? This logic worked for OTel spans but + // might need to be more sophisticated for ActivityTraceSpans... alternatively we need to be more + // sophisticated about how we set status in the first place (leveraging attributes that will be applied + // appropriately by this GetSpanStatus method). + var status = GetSpanStatus(data.Status, attributes); + span.Finish(status); + } _map.TryRemove(data.SpanId, out _); @@ -240,8 +254,7 @@ internal void PruneFilteredSpans(bool force = false) foreach (var mappedItem in _map) { var (spanId, span) = mappedItem; - var activity = span.GetFused(); - if (activity is { Recorded: false, IsAllDataRequested: false }) + if (span.GetActivity() is { Recorded: false, IsAllDataRequested: false }) { _map.TryRemove(spanId, out _); } diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceSpan.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceSpan.cs index 85516c0cfd..a7c4e2ec06 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceSpan.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTraceSpan.cs @@ -11,22 +11,64 @@ namespace Sentry.Internal.Tracing; /// internal class ActivityTraceSpan(System.Diagnostics.Activity activity) : ITraceSpan { - public void SetAttribute(string key, object value) => activity.SetTag(key, value); + public string? Description => activity.DisplayName; - public void AddEvent(string message) => activity.AddEvent(new ActivityEvent(message)); + public ITraceSpan AddEvent(string message) + { + activity.AddEvent(new ActivityEvent(message)); + return this; + } + + public ITraceSpan SetAttribute(string key, object value) + { + activity.SetTag(key, value); + return this; + } + + public ITraceSpan SetDescription(string? description) + { + if (description != null) + { + activity.DisplayName = description; + } + return this; + } - public void SetStatus(SpanStatus status, string? description = default) + public ITraceSpan SetStatus(SpanStatus status, string? description = default) { if (status == SpanStatus.Ok) { activity.SetStatus(ActivityStatusCode.Ok); - return; + return this; } var errorMessage = description ?? status.ToString(); activity.SetStatus(ActivityStatusCode.Error, errorMessage); + return this; } - public void Stop() => activity.Stop(); + public ITraceSpan Stop() + { + activity.Stop(); + return this; + } + + public ITraceSpan SetExtra(string key, object? value) + { + // TODO: Not sure what we want to do about Extra. This would change that data from being stored as "Extra" data + // (which is documented as being obsolete) to being stored as tags... which is a change in behaviour. The docs + // say structured contexts should be used instead, so maybe we need to create some new structured contexts for + // this "arbitrary" extra data that both we and users are currently storing. Alternatively, we can continue to + // store it as extra (in which case we need to implement this in some kind of custom property both here and when + // converting the activity into a SentrySpan for transmission) + activity.SetTag(key, value); + return this; + } + + public ITraceSpan Finish(Exception exception) + { + activity.BindException(exception); + return Stop(); + } public void Dispose() => activity.Dispose(); } diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracer.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracer.cs index 4a38130fce..bb12e5d087 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracer.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/ActivityTracer.cs @@ -9,8 +9,17 @@ internal class ActivityTracer(string name, string? version = "") : ITracer { private readonly ActivitySource _activitySource = new(name, version); - public ITraceSpan? StartSpan(string operationName) => - _activitySource.StartActivity(operationName) is { } activity ? new ActivityTraceSpan(activity) : null; + public ITraceSpan? StartSpan(string operationName, string? description = null) + { + var activity = _activitySource.CreateActivity(operationName, ActivityKind.Internal) + ?.SetIdFormat(ActivityIdFormat.W3C) + ?.Start(); + if (activity is not null) + { + activity.DisplayName = description ?? operationName; + } + return activity is not null ? new ActivityTraceSpan(activity) : null; + } public ITraceSpan? CurrentSpan => System.Diagnostics.Activity.Current == null ? null diff --git a/src/Sentry.DiagnosticSource/Internal/Tracing/TraceExtensions.cs b/src/Sentry.DiagnosticSource/Internal/Tracing/TraceExtensions.cs index 3396981873..2b4c32b94a 100644 --- a/src/Sentry.DiagnosticSource/Internal/Tracing/TraceExtensions.cs +++ b/src/Sentry.DiagnosticSource/Internal/Tracing/TraceExtensions.cs @@ -2,6 +2,9 @@ namespace Sentry.Internal.Tracing; internal static class TraceExtensions { + private const string SentrySpanKey = "_Sentry.SentrySpan"; + private const string SentryExceptionKey = "_Sentry.Exception"; + public static SpanId AsSentrySpanId(this ActivitySpanId id) => SpanId.Parse(id.ToHexString()); public static ActivitySpanId AsActivitySpanId(this SpanId id) => ActivitySpanId.CreateFromString(id.ToString().AsSpan()); @@ -16,4 +19,23 @@ public static BaggageHeader AsBaggageHeader(this IEnumerable (KeyValuePair)kvp!), useSentryPrefix ); + + public static void BindException(this System.Diagnostics.Activity activity, Exception exception) + { + activity.SetCustomProperty(SentryExceptionKey, exception); + } + + public static Exception? GetException(this System.Diagnostics.Activity activity) => + activity.GetCustomProperty(SentryExceptionKey) as Exception; + + public static void BindSentrySpan(this System.Diagnostics.Activity activity, ISpan span) + { + activity.SetCustomProperty(SentrySpanKey, span); + span.SetFused(activity); // We use a weak reference to allow the Activity to be disposed + } + + public static ISpan? GetSentrySpan(this System.Diagnostics.Activity activity) + => activity.GetCustomProperty(SentrySpanKey) is ISpan span ? span : null; + + public static System.Diagnostics.Activity? GetActivity(this ISpan span) => span.GetFused(); } diff --git a/src/Sentry.Extensions.Logging/SentryHttpMessageHandlerBuilderFilter.cs b/src/Sentry.Extensions.Logging/SentryHttpMessageHandlerBuilderFilter.cs index f35cad05d9..22a2af3c0c 100644 --- a/src/Sentry.Extensions.Logging/SentryHttpMessageHandlerBuilderFilter.cs +++ b/src/Sentry.Extensions.Logging/SentryHttpMessageHandlerBuilderFilter.cs @@ -17,7 +17,7 @@ public Action Configure(Action h is SentryHttpMessageHandler)) { handlerBuilder.AdditionalHandlers.Add( - new SentryHttpMessageHandler(hub) + new SentryHttpMessageHandler(hub, default, default, null, true) ); } diff --git a/src/Sentry/Internal/Tracing/ITraceSpan.cs b/src/Sentry/Internal/Tracing/ITraceSpan.cs index 6e86c9ebc6..de72b6b636 100644 --- a/src/Sentry/Internal/Tracing/ITraceSpan.cs +++ b/src/Sentry/Internal/Tracing/ITraceSpan.cs @@ -2,8 +2,27 @@ namespace Sentry.Internal.Tracing; internal interface ITraceSpan : IDisposable { - void SetAttribute(string key, object value); - void AddEvent(string message); - void SetStatus(SpanStatus status, string? description = default); - void Stop(); + string? Description { get; } + ITraceSpan AddEvent(string message); + ITraceSpan SetAttribute(string key, object value); + ITraceSpan SetDescription(string? description); + ITraceSpan SetStatus(SpanStatus status, string? description = default); + ITraceSpan Stop(); + + ITraceSpan SetExtra(string key, object? value); + + ITraceSpan Finish(Exception exception); +} + +internal static class TraceSpanExtensions +{ + internal static ITraceSpan SetExtras(this ITraceSpan traceSpan, IEnumerable> values) + { + foreach (var (key, value) in values) + { + traceSpan.SetExtra(key, value); + } + + return traceSpan; + } } diff --git a/src/Sentry/Internal/Tracing/ITracer.cs b/src/Sentry/Internal/Tracing/ITracer.cs index 6ad91b1087..006023b02b 100644 --- a/src/Sentry/Internal/Tracing/ITracer.cs +++ b/src/Sentry/Internal/Tracing/ITracer.cs @@ -2,6 +2,6 @@ namespace Sentry.Internal.Tracing; internal interface ITracer { - ITraceSpan? StartSpan(string operationName); + ITraceSpan? StartSpan(string operationName, string? description = null); ITraceSpan? CurrentSpan { get; } } diff --git a/src/Sentry/Internal/Tracing/SentryTraceSpan.cs b/src/Sentry/Internal/Tracing/SentryTraceSpan.cs index bef5015d1a..d00c48c6d4 100644 --- a/src/Sentry/Internal/Tracing/SentryTraceSpan.cs +++ b/src/Sentry/Internal/Tracing/SentryTraceSpan.cs @@ -5,6 +5,8 @@ internal class SentryTraceSpan : ITraceSpan private readonly ISpan _span; private Scope? _scope; + public string? Description => _span.Description; + public SentryTraceSpan(IHub hub, ISpan span) { _span = span; @@ -16,33 +18,57 @@ public void Dispose() // ISpan doesn't implement IDisposable } - public void SetAttribute(string key, object value) + public ITraceSpan AddEvent(string message) + { + _scope?.AddBreadcrumb(message); + return this; + } + + public ITraceSpan SetAttribute(string key, object value) { var stringValue = $"{value}"; if (string.IsNullOrWhiteSpace(stringValue)) { _span.UnsetTag(key); - return; } - _span.SetTag(key, stringValue); + else + { + _span.SetTag(key, stringValue); + } + return this; } - public void AddEvent(string message) + public ITraceSpan SetDescription(string? description) { - _scope?.AddBreadcrumb(message); + _span.Description = description; + return this; } - public void SetStatus(SpanStatus status, string? description = default) + public ITraceSpan SetStatus(SpanStatus status, string? description = default) { _span.Status = status; if (_span.Status != SpanStatus.Ok) { _span.Description = description; } + return this; } - public void Stop() + public ITraceSpan Stop() { _span.Finish(); + return this; + } + + public ITraceSpan SetExtra(string key, object? value) + { + _span.SetExtra(key, value); + return this; + } + + public ITraceSpan Finish(Exception exception) + { + _span.Finish(exception); + return this; } } diff --git a/src/Sentry/Internal/Tracing/SentryTracer.cs b/src/Sentry/Internal/Tracing/SentryTracer.cs index 3a5e72feb1..8621972867 100644 --- a/src/Sentry/Internal/Tracing/SentryTracer.cs +++ b/src/Sentry/Internal/Tracing/SentryTracer.cs @@ -2,9 +2,9 @@ namespace Sentry.Internal.Tracing; internal class SentryTracer(IHub hub) : ITracer { - public ITraceSpan StartSpan(string operationName) => new SentryTraceSpan( + public ITraceSpan? StartSpan(string operationName, string? description = null) => new SentryTraceSpan( hub, - hub.StartSpan(operationName, operationName) + hub.StartSpan(operationName, description ?? operationName) ); public ITraceSpan? CurrentSpan => hub.GetSpan() is {} span diff --git a/src/Sentry/SentryGraphQLHttpMessageHandler.cs b/src/Sentry/SentryGraphQLHttpMessageHandler.cs index 9da269d8b5..73352bc4bc 100644 --- a/src/Sentry/SentryGraphQLHttpMessageHandler.cs +++ b/src/Sentry/SentryGraphQLHttpMessageHandler.cs @@ -1,6 +1,7 @@ using Sentry.Extensibility; using Sentry.Internal; using Sentry.Internal.OpenTelemetry; +using Sentry.Internal.Tracing; namespace Sentry; @@ -12,6 +13,8 @@ public class SentryGraphQLHttpMessageHandler : SentryMessageHandler private readonly IHub _hub; private readonly SentryOptions? _options; private readonly ISentryFailedRequestHandler? _failedRequestHandler; + private readonly Lazy _lazyTracer; + private ITracer? Tracer => _lazyTracer.Value; /// /// Constructs an instance of . @@ -24,12 +27,13 @@ public SentryGraphQLHttpMessageHandler(HttpMessageHandler? innerHandler = defaul } internal SentryGraphQLHttpMessageHandler(IHub? hub, SentryOptions? options, - HttpMessageHandler? innerHandler = default, - ISentryFailedRequestHandler? failedRequestHandler = null) - : base(hub, options, innerHandler) + HttpMessageHandler? innerHandler = default, ISentryFailedRequestHandler? failedRequestHandler = null, + bool useNewTracing = false) + : base(hub, options, innerHandler, useNewTracing) { _hub = hub ?? HubAdapter.Instance; _options = options ?? _hub.GetSentryOptions(); + _lazyTracer = new(() => options?.InternalTraceProvider?.GetTracer(nameof(Sentry))); _failedRequestHandler = failedRequestHandler; if (_options != null) { @@ -38,15 +42,13 @@ internal SentryGraphQLHttpMessageHandler(IHub? hub, SentryOptions? options, } /// + [Obsolete("This method is obsolete and will be removed in a future version.")] protected internal override ISpan? ProcessRequest(HttpRequestMessage request, string method, string url) { - var content = GraphQLContentExtractor.ExtractRequestContentAsync(request, _options).Result; - if (content is not { } graphQlRequestContent) + if (!BindContent(request)) { - _options?.LogDebug("Unable to process non GraphQL request content"); return null; } - request.SetFused(graphQlRequestContent); // Start a span that tracks this request // (may be null if transaction is not set on the scope) @@ -58,10 +60,56 @@ internal SentryGraphQLHttpMessageHandler(IHub? hub, SentryOptions? options, return span; } + private protected override ITraceSpan? OnRequest(HttpRequestMessage request, string method, string url) + { + if (!BindContent(request)) + { + return null; + } + + // Start a span that tracks this request + // (may be null if transaction is not set on the scope) + return Tracer?.StartSpan( + "http.client", + $"{method} {url}" // e.g. "GET https://example.com" + ) + ?.SetExtra(OtelSemanticConventions.AttributeHttpRequestMethod, method); + } + + private bool BindContent(HttpRequestMessage request) + { + var content = GraphQLContentExtractor.ExtractRequestContentAsync(request, _options).Result; + if (content is not { } graphQlRequestContent) + { + _options?.LogDebug("Unable to process non GraphQL request content"); + return false; + } + request.SetFused(graphQlRequestContent); + return true; + } + /// + [Obsolete("This method is obsolete and will be removed in a future version.")] protected internal override void HandleResponse(HttpResponseMessage response, ISpan? span, string method, string url) { var graphqlInfo = response.RequestMessage?.GetFused(); + + HandleResponseInternal(response, method, url, graphqlInfo); + + if (span is null) + { + return; + } + + span.SetExtra(OtelSemanticConventions.AttributeHttpResponseStatusCode, (int)response.StatusCode); + span.Description = GetSpanDescriptionOrDefault(graphqlInfo, response.StatusCode) ?? span.Description; + // TODO: See how we can determine the span status for a GraphQL request... + var status = SpanStatusConverter.FromHttpStatusCode(response.StatusCode); // TODO: Don't do this if the span is errored + span.Finish(status); + } + + private void HandleResponseInternal(HttpResponseMessage response, string method, string url, GraphQLRequestContent? graphqlInfo) + { var breadcrumbData = new Dictionary { {"url", url}, @@ -82,20 +130,29 @@ protected internal override void HandleResponse(HttpResponseMessage response, IS graphqlInfo?.OperationType ?? "graphql.operation", "graphql", breadcrumbData - ); + ); // Create events for failed requests _failedRequestHandler?.HandleResponse(response); + } + + private protected override void OnResponse(HttpResponseMessage response, ITraceSpan? span, string method, string url) + { + var graphqlInfo = response.RequestMessage?.GetFused(); + + HandleResponseInternal(response, method, url, graphqlInfo); // This will handle unsuccessful status codes as well - if (span is not null) + if (span is null) { - span.SetExtra(OtelSemanticConventions.AttributeHttpResponseStatusCode, (int)response.StatusCode); - span.Description = GetSpanDescriptionOrDefault(graphqlInfo, response.StatusCode) ?? span.Description; - // TODO: See how we can determine the span status for a GraphQL request... - var status = SpanStatusConverter.FromHttpStatusCode(response.StatusCode); // TODO: Don't do this if the span is errored - span.Finish(status); + return; } + + // TODO: See how we can determine the span status for a GraphQL request... + var status = SpanStatusConverter.FromHttpStatusCode(response.StatusCode); // TODO: Don't do this if the span is errored + span.SetExtra(OtelSemanticConventions.AttributeHttpResponseStatusCode, (int)response.StatusCode) + .SetStatus(status, GetSpanDescriptionOrDefault(graphqlInfo, response.StatusCode) ?? span.Description) + .Stop(); } private string? GetSpanDescriptionOrDefault(GraphQLRequestContent? graphqlInfo, HttpStatusCode statusCode) => diff --git a/src/Sentry/SentryHttpMessageHandler.cs b/src/Sentry/SentryHttpMessageHandler.cs index 925df567d1..9c68d7ca7a 100644 --- a/src/Sentry/SentryHttpMessageHandler.cs +++ b/src/Sentry/SentryHttpMessageHandler.cs @@ -1,5 +1,6 @@ using Sentry.Extensibility; using Sentry.Internal.OpenTelemetry; +using Sentry.Internal.Tracing; namespace Sentry; @@ -11,6 +12,8 @@ public class SentryHttpMessageHandler : SentryMessageHandler private readonly IHub _hub; private readonly SentryOptions? _options; private readonly ISentryFailedRequestHandler? _failedRequestHandler; + private readonly Lazy _lazyTracer; + private ITracer? Tracer => _lazyTracer.Value; /// /// Constructs an instance of . @@ -44,14 +47,17 @@ public SentryHttpMessageHandler(HttpMessageHandler innerHandler, IHub hub) { } - internal SentryHttpMessageHandler(IHub? hub, SentryOptions? options, HttpMessageHandler? innerHandler = default, ISentryFailedRequestHandler? failedRequestHandler = null) - : base(hub, options, innerHandler) + internal SentryHttpMessageHandler(IHub? hub, SentryOptions? options, HttpMessageHandler? innerHandler = default, + ISentryFailedRequestHandler? failedRequestHandler = null, + bool useNewTracing = false) + : base(hub, options, innerHandler, useNewTracing) { _hub = hub ?? HubAdapter.Instance; _options = options ?? _hub.GetSentryOptions(); - _failedRequestHandler = failedRequestHandler; + _lazyTracer = new(() => options?.InternalTraceProvider?.GetTracer(nameof(Sentry))); // Use the default failed request handler if none was supplied - but options is required. + _failedRequestHandler = failedRequestHandler; if (_failedRequestHandler == null && _options != null) { _failedRequestHandler = new SentryHttpFailedRequestHandler(_hub, _options); @@ -59,6 +65,7 @@ internal SentryHttpMessageHandler(IHub? hub, SentryOptions? options, HttpMessage } /// + [Obsolete("This method will be removed in future versions.")] protected internal override ISpan? ProcessRequest(HttpRequestMessage request, string method, string url) { // Start a span that tracks this request @@ -71,8 +78,34 @@ internal SentryHttpMessageHandler(IHub? hub, SentryOptions? options, HttpMessage return span; } + private protected override ITraceSpan? OnRequest(HttpRequestMessage request, string method, string url) => + Tracer?.StartSpan( + "http.client", + $"{method} {url}" // e.g. "GET https://example.com" + ) + ?.SetExtra(OtelSemanticConventions.AttributeHttpRequestMethod, method); + /// + [Obsolete("This method will be removed in future versions.")] protected internal override void HandleResponse(HttpResponseMessage response, ISpan? span, string method, string url) + { + AddBreadcrumb(response, method, url); + + // Create events for failed requests + _failedRequestHandler?.HandleResponse(response); + + // This will handle unsuccessful status codes as well + if (span is null) + { + return; + } + + span.SetExtra(OtelSemanticConventions.AttributeHttpResponseStatusCode, (int)response.StatusCode); + var status = SpanStatusConverter.FromHttpStatusCode(response.StatusCode); + span.Finish(status); + } + + private void AddBreadcrumb(HttpResponseMessage response, string method, string url) { var breadcrumbData = new Dictionary { @@ -81,16 +114,24 @@ protected internal override void HandleResponse(HttpResponseMessage response, IS {"status_code", ((int) response.StatusCode).ToString()} }; _hub.AddBreadcrumb(string.Empty, "http", "http", breadcrumbData); + } + + private protected override void OnResponse(HttpResponseMessage response, ITraceSpan? span, string method, string url) + { + AddBreadcrumb(response, method, url); // Create events for failed requests _failedRequestHandler?.HandleResponse(response); // This will handle unsuccessful status codes as well - if (span is not null) + if (span is null) { - span.SetExtra(OtelSemanticConventions.AttributeHttpResponseStatusCode, (int)response.StatusCode); - var status = SpanStatusConverter.FromHttpStatusCode(response.StatusCode); - span.Finish(status); + return; } + + var status = SpanStatusConverter.FromHttpStatusCode(response.StatusCode); + span.SetExtra(OtelSemanticConventions.AttributeHttpResponseStatusCode, (int)response.StatusCode) + .SetStatus(status) + .Stop(); } } diff --git a/src/Sentry/SentryMessageHandler.cs b/src/Sentry/SentryMessageHandler.cs index aea1ff1f86..be5fc5e5a7 100644 --- a/src/Sentry/SentryMessageHandler.cs +++ b/src/Sentry/SentryMessageHandler.cs @@ -1,5 +1,6 @@ using Sentry.Extensibility; using Sentry.Internal.Extensions; +using Sentry.Internal.Tracing; namespace Sentry; @@ -10,6 +11,7 @@ public abstract class SentryMessageHandler : DelegatingHandler { private readonly IHub _hub; private readonly SentryOptions? _options; + private Func CreateRequestTracer { get; } /// /// Constructs an instance of . @@ -43,8 +45,23 @@ protected SentryMessageHandler(HttpMessageHandler innerHandler, IHub hub) { } - internal SentryMessageHandler(IHub? hub, SentryOptions? options, HttpMessageHandler? innerHandler = default) + /// + /// Internal constructor for testing. + /// + /// + /// Potentially SDK users have inherited from this class and overriden either the or + /// methods. Those really should have been private internal but we can't change that + /// without breaking changes. They've been marked as obsolete but, for the time being, this class defaults to using + /// those obsolete overrides for tracing operations. This can be changed by setting + /// to true, which we do whenever creating an instance of this class internally. Eventually, in a future major + /// release, this should be changed to be the default and the obsolete methods should be removed. + /// + internal SentryMessageHandler(IHub? hub, SentryOptions? options, HttpMessageHandler? innerHandler = default, + bool useNewTracing = false) { + CreateRequestTracer = (useNewTracing) + ? (request, method, url) => new AutoRequestTracer(this, request, method, url) + : (request, method, url) => new SentryRequestTracer(this, request, method, url); _hub = hub ?? HubAdapter.Instance; _options = options ?? _hub.GetSentryOptions(); @@ -63,8 +80,11 @@ internal SentryMessageHandler(IHub? hub, SentryOptions? options, HttpMessageHand /// The request method (e.g. "GET") /// The request URL /// An + [Obsolete("This method will be removed in future versions.")] protected internal abstract ISpan? ProcessRequest(HttpRequestMessage request, string method, string url); + private protected abstract ITraceSpan? OnRequest(HttpRequestMessage request, string method, string url); + /// /// Provides an opportunity for further processing of the span once a response is received. /// @@ -72,8 +92,54 @@ internal SentryMessageHandler(IHub? hub, SentryOptions? options, HttpMessageHand /// The created in /// The request method (e.g. "GET") /// The request URL + [Obsolete("This method will be removed in future versions.")] protected internal abstract void HandleResponse(HttpResponseMessage response, ISpan? span, string method, string url); + private protected abstract void OnResponse(HttpResponseMessage response, ITraceSpan? span, string method, string url); + + private abstract class RequestTracer + { + protected internal abstract void Finish(HttpResponseMessage response, string method, string url); + protected internal abstract void Finish(Exception exception); + } + + +#pragma warning disable CS0618 // Type or member is obsolete + private class SentryRequestTracer : RequestTracer + { + private readonly SentryMessageHandler _handler; + private readonly ISpan? _span; + + public SentryRequestTracer(SentryMessageHandler handler, HttpRequestMessage request, string method, string url) + { + _handler = handler; + _span = _handler.ProcessRequest(request, method, url); + } + + protected internal override void Finish(HttpResponseMessage response, string method, string url) + => _handler.HandleResponse(response, _span, method, url); + + protected internal override void Finish(Exception exception) => _span?.Finish(exception); + } +#pragma warning restore CS0618 // Type or member is obsolete + + private class AutoRequestTracer : RequestTracer + { + private readonly SentryMessageHandler _handler; + private readonly ITraceSpan? _span; + + public AutoRequestTracer(SentryMessageHandler handler, HttpRequestMessage request, string method, string url) + { + _handler = handler; + _span = _handler.OnRequest(request, method, url); + } + + protected internal override void Finish(HttpResponseMessage response, string method, string url) + => _handler.OnResponse(response, _span, method, url); + + protected internal override void Finish(Exception exception) => _span?.Finish(exception); + } + /// protected override async Task SendAsync( HttpRequestMessage request, @@ -82,17 +148,17 @@ protected override async Task SendAsync( var method = request.Method.Method.ToUpperInvariant(); var url = request.RequestUri?.ToString() ?? string.Empty; - var span = ProcessRequest(request, method, url); + var requestTracer = CreateRequestTracer(request, method, url); try { PropagateTraceHeaders(request, url); var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - HandleResponse(response, span, method, url); + requestTracer.Finish(response, method, url); return response; } catch (Exception ex) { - span?.Finish(ex); + requestTracer.Finish(ex); throw; } } @@ -104,17 +170,17 @@ protected override HttpResponseMessage Send(HttpRequestMessage request, Cancella var method = request.Method.Method.ToUpperInvariant(); var url = request.RequestUri?.ToString() ?? string.Empty; - var span = ProcessRequest(request, method, url); + var requestTracer = CreateRequestTracer(request, method, url); try { PropagateTraceHeaders(request, url); var response = base.Send(request, cancellationToken); - HandleResponse(response, span, method, url); + requestTracer.Finish(response, method, url); return response; } catch (Exception ex) { - span?.Finish(ex); + requestTracer?.Finish(ex); throw; } } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index 71d9514f0e..1af3c4e205 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -546,7 +546,9 @@ namespace Sentry public class SentryGraphQLHttpMessageHandler : Sentry.SentryMessageHandler { public SentryGraphQLHttpMessageHandler(System.Net.Http.HttpMessageHandler? innerHandler = null, Sentry.IHub? hub = null) { } + [System.Obsolete("This method is obsolete and will be removed in a future version.")] protected override void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url) { } + [System.Obsolete("This method is obsolete and will be removed in a future version.")] protected override Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url) { } } public class SentryHint @@ -565,7 +567,9 @@ namespace Sentry public SentryHttpMessageHandler(Sentry.IHub hub) { } public SentryHttpMessageHandler(System.Net.Http.HttpMessageHandler innerHandler) { } public SentryHttpMessageHandler(System.Net.Http.HttpMessageHandler innerHandler, Sentry.IHub hub) { } + [System.Obsolete("This method will be removed in future versions.")] protected override void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url) { } + [System.Obsolete("This method will be removed in future versions.")] protected override Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url) { } } public readonly struct SentryId : Sentry.ISentryJsonSerializable, System.IEquatable @@ -614,7 +618,9 @@ namespace Sentry protected SentryMessageHandler(Sentry.IHub hub) { } protected SentryMessageHandler(System.Net.Http.HttpMessageHandler innerHandler) { } protected SentryMessageHandler(System.Net.Http.HttpMessageHandler innerHandler, Sentry.IHub hub) { } + [System.Obsolete("This method will be removed in future versions.")] protected abstract void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url); + [System.Obsolete("This method will be removed in future versions.")] protected abstract Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url); protected override System.Net.Http.HttpResponseMessage Send(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { } protected override System.Threading.Tasks.Task SendAsync(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt index 71d9514f0e..1af3c4e205 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt @@ -546,7 +546,9 @@ namespace Sentry public class SentryGraphQLHttpMessageHandler : Sentry.SentryMessageHandler { public SentryGraphQLHttpMessageHandler(System.Net.Http.HttpMessageHandler? innerHandler = null, Sentry.IHub? hub = null) { } + [System.Obsolete("This method is obsolete and will be removed in a future version.")] protected override void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url) { } + [System.Obsolete("This method is obsolete and will be removed in a future version.")] protected override Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url) { } } public class SentryHint @@ -565,7 +567,9 @@ namespace Sentry public SentryHttpMessageHandler(Sentry.IHub hub) { } public SentryHttpMessageHandler(System.Net.Http.HttpMessageHandler innerHandler) { } public SentryHttpMessageHandler(System.Net.Http.HttpMessageHandler innerHandler, Sentry.IHub hub) { } + [System.Obsolete("This method will be removed in future versions.")] protected override void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url) { } + [System.Obsolete("This method will be removed in future versions.")] protected override Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url) { } } public readonly struct SentryId : Sentry.ISentryJsonSerializable, System.IEquatable @@ -614,7 +618,9 @@ namespace Sentry protected SentryMessageHandler(Sentry.IHub hub) { } protected SentryMessageHandler(System.Net.Http.HttpMessageHandler innerHandler) { } protected SentryMessageHandler(System.Net.Http.HttpMessageHandler innerHandler, Sentry.IHub hub) { } + [System.Obsolete("This method will be removed in future versions.")] protected abstract void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url); + [System.Obsolete("This method will be removed in future versions.")] protected abstract Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url); protected override System.Net.Http.HttpResponseMessage Send(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { } protected override System.Threading.Tasks.Task SendAsync(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt index 124f188850..763c645d5e 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt @@ -547,7 +547,9 @@ namespace Sentry public class SentryGraphQLHttpMessageHandler : Sentry.SentryMessageHandler { public SentryGraphQLHttpMessageHandler(System.Net.Http.HttpMessageHandler? innerHandler = null, Sentry.IHub? hub = null) { } + [System.Obsolete("This method is obsolete and will be removed in a future version.")] protected override void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url) { } + [System.Obsolete("This method is obsolete and will be removed in a future version.")] protected override Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url) { } } public class SentryHint @@ -566,7 +568,9 @@ namespace Sentry public SentryHttpMessageHandler(Sentry.IHub hub) { } public SentryHttpMessageHandler(System.Net.Http.HttpMessageHandler innerHandler) { } public SentryHttpMessageHandler(System.Net.Http.HttpMessageHandler innerHandler, Sentry.IHub hub) { } + [System.Obsolete("This method will be removed in future versions.")] protected override void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url) { } + [System.Obsolete("This method will be removed in future versions.")] protected override Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url) { } } public readonly struct SentryId : Sentry.ISentryJsonSerializable, System.IEquatable @@ -615,7 +619,9 @@ namespace Sentry protected SentryMessageHandler(Sentry.IHub hub) { } protected SentryMessageHandler(System.Net.Http.HttpMessageHandler innerHandler) { } protected SentryMessageHandler(System.Net.Http.HttpMessageHandler innerHandler, Sentry.IHub hub) { } + [System.Obsolete("This method will be removed in future versions.")] protected abstract void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url); + [System.Obsolete("This method will be removed in future versions.")] protected abstract Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url); protected override System.Net.Http.HttpResponseMessage Send(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { } protected override System.Threading.Tasks.Task SendAsync(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt index b09d55171d..37b2261a6d 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt @@ -545,7 +545,9 @@ namespace Sentry public class SentryGraphQLHttpMessageHandler : Sentry.SentryMessageHandler { public SentryGraphQLHttpMessageHandler(System.Net.Http.HttpMessageHandler? innerHandler = null, Sentry.IHub? hub = null) { } + [System.Obsolete("This method is obsolete and will be removed in a future version.")] protected override void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url) { } + [System.Obsolete("This method is obsolete and will be removed in a future version.")] protected override Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url) { } } public class SentryHint @@ -564,7 +566,9 @@ namespace Sentry public SentryHttpMessageHandler(Sentry.IHub hub) { } public SentryHttpMessageHandler(System.Net.Http.HttpMessageHandler innerHandler) { } public SentryHttpMessageHandler(System.Net.Http.HttpMessageHandler innerHandler, Sentry.IHub hub) { } + [System.Obsolete("This method will be removed in future versions.")] protected override void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url) { } + [System.Obsolete("This method will be removed in future versions.")] protected override Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url) { } } public readonly struct SentryId : Sentry.ISentryJsonSerializable, System.IEquatable @@ -613,7 +617,9 @@ namespace Sentry protected SentryMessageHandler(Sentry.IHub hub) { } protected SentryMessageHandler(System.Net.Http.HttpMessageHandler innerHandler) { } protected SentryMessageHandler(System.Net.Http.HttpMessageHandler innerHandler, Sentry.IHub hub) { } + [System.Obsolete("This method will be removed in future versions.")] protected abstract void HandleResponse(System.Net.Http.HttpResponseMessage response, Sentry.ISpan? span, string method, string url); + [System.Obsolete("This method will be removed in future versions.")] protected abstract Sentry.ISpan? ProcessRequest(System.Net.Http.HttpRequestMessage request, string method, string url); protected override System.Threading.Tasks.Task SendAsync(System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) { } } diff --git a/test/Sentry.Tests/SentryGraphQlHttpMessageHandlerTests.cs b/test/Sentry.Tests/SentryGraphQlHttpMessageHandlerTests.cs index 4aa1ac21f0..513d04f018 100644 --- a/test/Sentry.Tests/SentryGraphQlHttpMessageHandlerTests.cs +++ b/test/Sentry.Tests/SentryGraphQlHttpMessageHandlerTests.cs @@ -23,6 +23,7 @@ public class SentryGraphQlHttpMessageHandlerTests private StringContent ValidResponseContent => SentryGraphQlTestHelpers.ResponesContent(ValidResponse); [Fact] + [Obsolete("Obsolete")] public void ProcessRequest_ExtractsGraphQlRequestContent() { // Arrange @@ -44,6 +45,7 @@ public void ProcessRequest_ExtractsGraphQlRequestContent() } [Fact] + [Obsolete("Obsolete")] public void ProcessRequest_SetsSpanData() { // Arrange @@ -74,6 +76,7 @@ public void ProcessRequest_SetsSpanData() // [Theory] // [InlineData(ValidQuery)] [Fact] + [Obsolete("Obsolete")] public void HandleResponse_AddsBreadcrumb() { // Arrange @@ -115,6 +118,7 @@ public void HandleResponse_AddsBreadcrumb() } [Fact] + [Obsolete("Obsolete")] public void HandleResponse_SetsSpanData() { // Arrange diff --git a/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs b/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs index fc38202019..744e18cef5 100644 --- a/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs +++ b/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs @@ -240,6 +240,7 @@ public async Task SendAsync_Executed_FailedRequestsCaptured() } [Fact] + [Obsolete("Obsolete")] public void ProcessRequest_SetsSpanData() { // Arrange @@ -267,6 +268,7 @@ public void ProcessRequest_SetsSpanData() } [Fact] + [Obsolete("Obsolete")] public void HandleResponse_SetsSpanData() { // Arrange From ac08a7b43e33762525ef20932e30edcdd5dd48db Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 3 Apr 2024 19:32:10 +1300 Subject: [PATCH 11/11] Windows verify tests --- ...roperly_registered.DotNet6_0.Windows.DotNet.verified.txt | 6 ++++++ ...roperly_registered.DotNet7_0.Windows.DotNet.verified.txt | 6 ++++++ ...roperly_registered.DotNet8_0.Windows.DotNet.verified.txt | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.Windows.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.Windows.DotNet.verified.txt index 10eaeaf749..44ec09a1a1 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.Windows.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet6_0.Windows.DotNet.verified.txt @@ -40,5 +40,11 @@ Args: [ WinUIUnhandledExceptionIntegration ] + }, + { + Message: Registering integration: '{0}'., + Args: [ + ActivityTracingIntegration + ] } ] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.Windows.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.Windows.DotNet.verified.txt index 10eaeaf749..44ec09a1a1 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.Windows.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet7_0.Windows.DotNet.verified.txt @@ -40,5 +40,11 @@ Args: [ WinUIUnhandledExceptionIntegration ] + }, + { + Message: Registering integration: '{0}'., + Args: [ + ActivityTracingIntegration + ] } ] \ No newline at end of file diff --git a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.Windows.DotNet.verified.txt b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.Windows.DotNet.verified.txt index e14cfaf075..25a5ea7a08 100644 --- a/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.Windows.DotNet.verified.txt +++ b/test/Sentry.Tests/SentryOptionsTests.Integrations_default_ones_are_properly_registered.DotNet8_0.Windows.DotNet.verified.txt @@ -50,5 +50,11 @@ { Level: info, Message: System.Diagnostics.Metrics Integration is disabled because no listeners are configured. + }, + { + Message: Registering integration: '{0}'., + Args: [ + ActivityTracingIntegration + ] } ] \ No newline at end of file