Using Activity.RecordException now correctly updates the error status of OpenTelemetry Spans

CHANGELOG.md

@@ -14,6 +14,7 @@
 - Fixed SDK not sending exceptions via Blazor WebAssembly due to a `PlatformNotSupportedException` ([#2506](https://github.com/getsentry/sentry-dotnet/pull/2506))
 - Align SDK with docs regarding session update for dropped events ([#2496](https://github.com/getsentry/sentry-dotnet/pull/2496))
 - Introduced `HttpMessageHandler` in favor of the now deprecated `HttpClientHandler` on the options. This allows the SDK to support NSUrlSessionHandler on iOS ([#2503](https://github.com/getsentry/sentry-dotnet/pull/2503))
+- Fixed Error status of OpenTelemetry Spans when using Activity.RecordException ([#2515](https://github.com/getsentry/sentry-dotnet/pull/2515))
 
 ### Dependencies
 

src/Sentry.OpenTelemetry/OpenTelemetry/ATTRIBUTION.txt

@@ -0,0 +1,18 @@
+Parts of the code in this subdirectory have been adapted from
+https://github.com/open-telemetry/opentelemetry-dotnet/
+
+The original license is as follows:
+
+Copyright The OpenTelemetry Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.

src/Sentry.OpenTelemetry/OpenTelemetry/SemanticConventions.cs

@@ -0,0 +1,115 @@
+// Modified from:
+// https://github.com/open-telemetry/opentelemetry-dotnet/blob/dacc532d51ca0f3775160b84fa6d7d9403a8ccde/src/Shared/SemanticConventions.cs
+
+// <copyright file="SemanticConventions.cs" company="OpenTelemetry Authors">
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+
+// ReSharper disable once CheckNamespace
+namespace Sentry.OpenTelemetry;
+
+/// <summary>
+/// Constants for semantic attribute names outlined by the OpenTelemetry specifications.
+/// <see href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/README.md"/> and
+/// <see href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/README.md"/>.
+/// </summary>
+internal static class SemanticConventions
+{
+    // The set of constants matches the specification as of this commit.
+    // https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/trace/semantic_conventions
+    // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md
+    public const string AttributeNetTransport = "net.transport";
+    public const string AttributeNetPeerIp = "net.peer.ip";
+    public const string AttributeNetPeerPort = "net.peer.port";
+    public const string AttributeNetPeerName = "net.peer.name";
+    public const string AttributeNetHostIp = "net.host.ip";
+    public const string AttributeNetHostPort = "net.host.port";
+    public const string AttributeNetHostName = "net.host.name";
+
+    public const string AttributeEnduserId = "enduser.id";
+    public const string AttributeEnduserRole = "enduser.role";
+    public const string AttributeEnduserScope = "enduser.scope";
+
+    public const string AttributePeerService = "peer.service";
+
+    public const string AttributeHttpMethod = "http.method";
+    public const string AttributeHttpUrl = "http.url";
+    public const string AttributeHttpTarget = "http.target";
+    public const string AttributeHttpHost = "http.host";
+    public const string AttributeHttpScheme = "http.scheme";
+    public const string AttributeHttpStatusCode = "http.status_code";
+    public const string AttributeHttpStatusText = "http.status_text";
+    public const string AttributeHttpFlavor = "http.flavor";
+    public const string AttributeHttpServerName = "http.server_name";
+    public const string AttributeHttpRoute = "http.route";
+    public const string AttributeHttpClientIP = "http.client_ip";
+    public const string AttributeHttpUserAgent = "http.user_agent";
+    public const string AttributeHttpRequestContentLength = "http.request_content_length";
+    public const string AttributeHttpRequestContentLengthUncompressed = "http.request_content_length_uncompressed";
+    public const string AttributeHttpResponseContentLength = "http.response_content_length";
+    public const string AttributeHttpResponseContentLengthUncompressed = "http.response_content_length_uncompressed";
+
+    public const string AttributeDbSystem = "db.system";
+    public const string AttributeDbConnectionString = "db.connection_string";
+    public const string AttributeDbUser = "db.user";
+    public const string AttributeDbMsSqlInstanceName = "db.mssql.instance_name";
+    public const string AttributeDbJdbcDriverClassName = "db.jdbc.driver_classname";
+    public const string AttributeDbName = "db.name";
+    public const string AttributeDbStatement = "db.statement";
+    public const string AttributeDbOperation = "db.operation";
+    public const string AttributeDbInstance = "db.instance";
+    public const string AttributeDbUrl = "db.url";
+    public const string AttributeDbCassandraKeyspace = "db.cassandra.keyspace";
+    public const string AttributeDbHBaseNamespace = "db.hbase.namespace";
+    public const string AttributeDbRedisDatabaseIndex = "db.redis.database_index";
+    public const string AttributeDbMongoDbCollection = "db.mongodb.collection";
+
+    public const string AttributeRpcSystem = "rpc.system";
+    public const string AttributeRpcService = "rpc.service";
+    public const string AttributeRpcMethod = "rpc.method";
+    public const string AttributeRpcGrpcStatusCode = "rpc.grpc.status_code";
+
+    public const string AttributeMessageType = "message.type";
+    public const string AttributeMessageId = "message.id";
+    public const string AttributeMessageCompressedSize = "message.compressed_size";
+    public const string AttributeMessageUncompressedSize = "message.uncompressed_size";
+
+    public const string AttributeFaasTrigger = "faas.trigger";
+    public const string AttributeFaasExecution = "faas.execution";
+    public const string AttributeFaasDocumentCollection = "faas.document.collection";
+    public const string AttributeFaasDocumentOperation = "faas.document.operation";
+    public const string AttributeFaasDocumentTime = "faas.document.time";
+    public const string AttributeFaasDocumentName = "faas.document.name";
+    public const string AttributeFaasTime = "faas.time";
+    public const string AttributeFaasCron = "faas.cron";
+
+    public const string AttributeMessagingSystem = "messaging.system";
+    public const string AttributeMessagingDestination = "messaging.destination";
+    public const string AttributeMessagingDestinationKind = "messaging.destination_kind";
+    public const string AttributeMessagingTempDestination = "messaging.temp_destination";
+    public const string AttributeMessagingProtocol = "messaging.protocol";
+    public const string AttributeMessagingProtocolVersion = "messaging.protocol_version";
+    public const string AttributeMessagingUrl = "messaging.url";
+    public const string AttributeMessagingMessageId = "messaging.message_id";
+    public const string AttributeMessagingConversationId = "messaging.conversation_id";
+    public const string AttributeMessagingPayloadSize = "messaging.message_payload_size_bytes";
+    public const string AttributeMessagingPayloadCompressedSize = "messaging.message_payload_compressed_size_bytes";
+    public const string AttributeMessagingOperation = "messaging.operation";
+
+    public const string AttributeExceptionEventName = "exception";
+    public const string AttributeExceptionType = "exception.type";
+    public const string AttributeExceptionMessage = "exception.message";
+    public const string AttributeExceptionStacktrace = "exception.stacktrace";
+}

src/Sentry.OpenTelemetry/OpenTelemetry/SpanAttributeConstants.cs

@@ -0,0 +1,31 @@
+// Modified from:
+// https://github.com/open-telemetry/opentelemetry-dotnet/blob/dacc532d51ca0f3775160b84fa6d7d9403a8ccde/src/Shared/SpanAttributeConstants.cs
+
+// <copyright file="SpanAttributeConstants.cs" company="OpenTelemetry Authors">
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+
+// ReSharper disable once CheckNamespace
+namespace Sentry.OpenTelemetry;
+
+/// <summary>
+/// Defines well-known span attribute keys.
+/// </summary>
+internal static class SpanAttributeConstants
+{
+    public const string StatusCodeKey = "otel.status_code";
+    public const string StatusDescriptionKey = "otel.status_description";
+    public const string DatabaseStatementTypeKey = "db.statement_type";
+}

src/Sentry.OpenTelemetry/OpenTelemetry/StatusTags.cs

@@ -0,0 +1,10 @@
+// ReSharper disable once CheckNamespace
+namespace Sentry.OpenTelemetry;
+
+internal static class StatusTags
+{
+    // See https://github.com/open-telemetry/opentelemetry-dotnet/blob/dacc532d51ca0f3775160b84fa6d7d9403a8ccde/src/Shared/StatusHelper.cs#L26
+    public const string UnsetStatusCodeTagValue = "UNSET";
+    public const string OkStatusCodeTagValue = "OK";
+    public const string ErrorStatusCodeTagValue = "ERROR";
+}

src/Sentry.OpenTelemetry/SentrySpanProcessor.cs

@@ -1,4 +1,5 @@
 using OpenTelemetry;
+using OpenTelemetry.Trace;
 using Sentry.Extensibility;
 using Sentry.Internal.Extensions;
 
@@ -169,14 +170,22 @@ public override void OnEnd(Activity data)
         _map.TryRemove(data.SpanId, out _);
     }
 
-    internal static SpanStatus GetSpanStatus(ActivityStatusCode status, IDictionary<string, object?> attributes) =>
-        status switch
+    internal static SpanStatus GetSpanStatus(ActivityStatusCode status, IDictionary<string, object?> attributes)
+    {
+        // See https://github.com/open-telemetry/opentelemetry-dotnet/discussions/4703
+        if (attributes.TryGetValue(SpanAttributeConstants.StatusCodeKey, out var statusCode)
+            && statusCode is StatusTags.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<string, object?> attributes)
     {
@@ -203,21 +212,21 @@ private static (string operation, string description, TransactionNameSource sour
 
         // HTTP span
         // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/http/
-        if (attributes.TryGetTypedValue("http.method", out string httpMethod))
+        if (attributes.TryGetTypedValue(SemanticConventions.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("http.route", out string httpRoute))
+            if (attributes.TryGetTypedValue(SemanticConventions.AttributeHttpRoute, out string httpRoute))
             {
                 // A route exists.  Use the method and route.
                 return ("http.server", $"{httpMethod} {httpRoute}", TransactionNameSource.Route);
             }
 
-            if (attributes.TryGetTypedValue("http.target", out string httpTarget))
+            if (attributes.TryGetTypedValue(SemanticConventions.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;
@@ -230,9 +239,9 @@ private static (string operation, string description, TransactionNameSource sour
 
         // DB span
         // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/database/
-        if (attributes.ContainsKey("db.system"))
+        if (attributes.ContainsKey(SemanticConventions.AttributeDbSystem))
         {
-            if (attributes.TryGetTypedValue("db.statement", out string dbStatement))
+            if (attributes.TryGetTypedValue(SemanticConventions.AttributeDbStatement, out string dbStatement))
             {
                 // We have a database statement.  Use it.
                 return ("db", dbStatement, TransactionNameSource.Task);
@@ -244,21 +253,21 @@ private static (string operation, string description, TransactionNameSource sour
 
         // RPC span
         // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/rpc/
-        if (attributes.ContainsKey("rpc.service"))
+        if (attributes.ContainsKey(SemanticConventions.AttributeRpcService))
         {
             return ("rpc", activity.DisplayName, TransactionNameSource.Route);
         }
 
         // Messaging span
         // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/messaging/
-        if (attributes.ContainsKey("messaging.system"))
+        if (attributes.ContainsKey(SemanticConventions.AttributeMessagingSystem))
         {
             return ("message", activity.DisplayName, TransactionNameSource.Route);
         }
 
         // FaaS (Functions/Lambda) span
         // https://opentelemetry.io/docs/specs/otel/trace/semantic_conventions/faas/
-        if (attributes.TryGetTypedValue("faas.trigger", out string faasTrigger))
+        if (attributes.TryGetTypedValue(SemanticConventions.AttributeFaasTrigger, out string faasTrigger))
         {
             return (faasTrigger, activity.DisplayName, TransactionNameSource.Route);
         }
@@ -286,36 +295,57 @@ private static (string operation, string description, TransactionNameSource sour
 
     private void GenerateSentryErrorsFromOtelSpan(Activity activity, IDictionary<string, object?> 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 == "exception"))
-    //     {
-    //         // Note, this doesn't do anything yet because `exception` is not a valid attribute.
-    //         // We cannot just use `exception.type`, `exception.message`, and `exception.stacktrace`.
-    //         // See https://github.com/open-telemetry/opentelemetry-dotnet/issues/2439#issuecomment-1577314568
-    //
-    //         var eventAttributes = @event.Tags.ToDictionary();
-    //         if (!eventAttributes.TryGetTypedValue("exception", out Exception exception))
-    //         {
-    //             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);
-    //         _hub.CaptureEvent(sentryEvent, scope =>
-    //         {
-    //             scope.Contexts["otel"] = GetOtelContext(spanAttributes);
-    //
-    //             var trace = scope.Contexts.Trace;
-    //             trace.TraceId = activity.TraceId.AsSentryId();
-    //             trace.SpanId = activity.SpanId.AsSentrySpanId();
-    //             trace.ParentSpanId = activity.ParentSpanId.AsSentrySpanId();
-    //         });
-    //     }
+        // 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 == SemanticConventions.AttributeExceptionEventName))
+        {
+            var eventAttributes = @event.Tags.ToDictionary();
+            // 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(SemanticConventions.AttributeExceptionType, out string exceptionType))
+            {
+                continue;
+            }
+            eventAttributes.TryGetTypedValue(SemanticConventions.AttributeExceptionMessage, out string message);
+            eventAttributes.TryGetTypedValue(SemanticConventions.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();
+            });
+        }
     }
 }