diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 25bdfb76a51..320202b0fea 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -39,6 +39,7 @@ environment variable is set to `http` or `http/dup`. ([#5005](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5005)) + ([#5034](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5034)) ## 1.6.0-beta.2 diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 095ace34d67..67f0204f4e1 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -217,6 +217,31 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity) } } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static string GetErrorType(Exception exception) + { + if (exception is WebException wexc) + { + // TODO: consider other Status values from + // https://learn.microsoft.com/dotnet/api/system.net.webexceptionstatus?view=netframework-4.6.2 + return wexc.Status switch + { + WebExceptionStatus.NameResolutionFailure => "name_resolution_failure", + WebExceptionStatus.ConnectFailure => "connect_failure", + WebExceptionStatus.SendFailure => "send_failure", + WebExceptionStatus.RequestCanceled => "request_cancelled", + WebExceptionStatus.TrustFailure => "trust_failure", + WebExceptionStatus.SecureChannelFailure => "secure_channel_failure", + WebExceptionStatus.ServerProtocolViolation => "server_protocol_violation", + WebExceptionStatus.Timeout => "timeout", + WebExceptionStatus.MessageLengthLimitExceeded => "message_length_limit_exceeded", + _ => wexc.GetType().FullName, + }; + } + + return exception.GetType().FullName; + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void AddExceptionTags(Exception exception, Activity activity, out HttpStatusCode? statusCode) { @@ -371,6 +396,7 @@ private static void HookOrProcessResult(HttpWebRequest request) private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncCallback, Activity activity, object result, bool forceResponseCopy, HttpWebRequest request, long startTimestamp) { HttpStatusCode? httpStatusCode = null; + string errorType = null; // Activity may be null if we are not tracing in these cases: // 1. No listeners @@ -386,6 +412,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC { if (result is Exception ex) { + errorType = GetErrorType(ex); if (activity != null) { AddExceptionTags(ex, activity, out httpStatusCode); @@ -430,6 +457,12 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC httpStatusCode = response.StatusCode; } + + if (SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)httpStatusCode.Value) == ActivityStatusCode.Error) + { + // override the errorType to statusCode for failures. + errorType = TelemetryHelper.GetStatusCodeString(httpStatusCode.Value); + } } } catch (Exception ex) @@ -437,6 +470,11 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC HttpInstrumentationEventSource.Log.FailedProcessResult(ex); } + if (tracingEmitNewAttributes && errorType != null) + { + activity?.SetTag(SemanticConventions.AttributeErrorType, errorType); + } + activity?.Stop(); if (HttpClientDuration.Enabled || HttpClientRequestDuration.Enabled) @@ -504,6 +542,11 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC tags.Add(SemanticConventions.AttributeHttpResponseStatusCode, (int)httpStatusCode.Value); } + if (errorType != null) + { + tags.Add(SemanticConventions.AttributeErrorType, errorType); + } + HttpClientRequestDuration.Record(durationS, tags); } } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs index 9b53a5bc5f6..a17f3f3a70b 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/TelemetryHelper.cs @@ -20,14 +20,14 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation; internal static class TelemetryHelper { - public static readonly object[] BoxedStatusCodes; + public static readonly (object, string)[] BoxedStatusCodes; static TelemetryHelper() { - BoxedStatusCodes = new object[500]; + BoxedStatusCodes = new (object, string)[500]; for (int i = 0, c = 100; i < BoxedStatusCodes.Length; i++, c++) { - BoxedStatusCodes[i] = c; + BoxedStatusCodes[i] = (c, c.ToString()); } } @@ -36,9 +36,20 @@ public static object GetBoxedStatusCode(HttpStatusCode statusCode) int intStatusCode = (int)statusCode; if (intStatusCode >= 100 && intStatusCode < 600) { - return BoxedStatusCodes[intStatusCode - 100]; + return BoxedStatusCodes[intStatusCode - 100].Item1; } - return intStatusCode; + return statusCode; + } + + public static string GetStatusCodeString(HttpStatusCode statusCode) + { + int intStatusCode = (int)statusCode; + if (intStatusCode >= 100 && intStatusCode < 600) + { + return BoxedStatusCodes[intStatusCode - 100].Item2; + } + + return statusCode.ToString(); } } diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index ad3344d8a29..c957c14840d 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -341,7 +341,6 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( var normalizedAttributes = activity.TagObjects.Where(kv => !kv.Key.StartsWith("otel.")).ToDictionary(x => x.Key, x => x.Value.ToString()); -#if !NETFRAMEWORK int numberOfNewTags = activity.Status == ActivityStatusCode.Error ? 6 : 5; int numberOfDupeTags = activity.Status == ActivityStatusCode.Error ? 12 : 11; @@ -350,13 +349,6 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( : semanticConvention == HttpSemanticConvention.New ? numberOfNewTags + (tc.ResponseExpected ? 1 : 0) : 6 + (tc.ResponseExpected ? 1 : 0); -#else - var expectedAttributeCount = semanticConvention == HttpSemanticConvention.Dupe - ? 11 + (tc.ResponseExpected ? 2 : 0) - : semanticConvention == HttpSemanticConvention.New - ? 5 + (tc.ResponseExpected ? 1 : 0) - : 6 + (tc.ResponseExpected ? 1 : 0); -#endif Assert.Equal(expectedAttributeCount, normalizedAttributes.Count); @@ -389,24 +381,23 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( { Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); -#if !NETFRAMEWORK if (tc.ResponseCode >= 400) { Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); } -#endif } else { Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); -#if !NETFRAMEWORK -#if !NET8_0_OR_GREATER - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "System.Net.Http.HttpRequestException"); -#else + +#if NET8_0_OR_GREATER // we are using fake address so it will be "name_resolution_error" // TODO: test other error types. Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_error"); -#endif +#elif NETFRAMEWORK + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_failure"); +#else + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "System.Net.Http.HttpRequestException"); #endif } } @@ -543,7 +534,6 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( attributes[tag.Key] = tag.Value; } -#if !NETFRAMEWORK #if !NET8_0_OR_GREATER var numberOfTags = 6; #else @@ -558,10 +548,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( } var expectedAttributeCount = numberOfTags + (tc.ResponseExpected ? 1 : 0); -#else - var expectedAttributeCount = 5 + (tc.ResponseExpected ? 1 : 0); -#endif Assert.Equal(expectedAttributeCount, attributes.Count); Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); @@ -576,29 +563,24 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( { Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); -#if !NETFRAMEWORK if (tc.ResponseCode >= 400) { Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); } -#endif } else { Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); -#if !NETFRAMEWORK -#if !NET8_0_OR_GREATER - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "System.Net.Http.HttpRequestException"); -#else +#if NET8_0_OR_GREATER // we are using fake address so it will be "name_resolution_error" // TODO: test other error types. Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_error"); +#elif NETFRAMEWORK + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_failure"); - // network.protocol.version is not emitted when response if not received. - // https://github.com/open-telemetry/opentelemetry-dotnet/issues/4928 - Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion); -#endif +#else + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "System.Net.Http.HttpRequestException"); #endif }