diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 1cf655830..f1e6a6bc9 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -44,6 +44,12 @@ * Fixed `network.protocol.version` attribute values to match the specification. ([#5006](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5006)) +* Set `network.protocol.version` value using the protocol version on the + received response. If the request fails without response, then + `network.protocol.version` attribute will not be set on Activity and + `http.client.request.duration` metric. + ([#5043](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5043)) + ## 1.6.0-beta.2 Released 2023-Oct-26 diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index 924f260a9..f5fb18221 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -205,7 +205,6 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler } activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); - activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(request.Version)); if (request.Headers.TryGetValues("User-Agent", out var userAgentValues)) { @@ -283,6 +282,7 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler if (this.emitNewAttributes) { + activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.Version)); activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); if (activity.Status == ActivityStatusCode.Error) { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index 9b20cbb53..82bdc004e 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -130,7 +130,6 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); tags.Add(new KeyValuePair(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme)); - tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(request.Version))); if (!request.RequestUri.IsDefaultPort) { @@ -139,6 +138,7 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler if (TryFetchResponse(payload, out HttpResponseMessage response)) { + tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.Version))); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode))); // Set error.type to status code for failed requests diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 3852201fd..9a0fd30e7 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -173,7 +173,6 @@ internal static class HttpWebRequestActivitySource } activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); - activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(request.ProtocolVersion)); } try @@ -201,6 +200,7 @@ internal static class HttpWebRequestActivitySource if (tracingEmitNewAttributes) { + activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.ProtocolVersion)); activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); } @@ -243,11 +243,12 @@ internal static class HttpWebRequestActivitySource } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static void AddExceptionTags(Exception exception, Activity activity, out HttpStatusCode? statusCode) + private static void AddExceptionTags(Exception exception, Activity activity, out HttpStatusCode? statusCode, out Version protocolVersion) { Debug.Assert(activity != null, "Activity must not be null"); statusCode = null; + protocolVersion = null; if (!activity.IsAllDataRequested) { @@ -259,6 +260,7 @@ internal static class HttpWebRequestActivitySource if (exception is WebException wexc && wexc.Response is HttpWebResponse response) { statusCode = response.StatusCode; + protocolVersion = response.ProtocolVersion; if (tracingEmitOldAttributes) { @@ -267,6 +269,7 @@ internal static class HttpWebRequestActivitySource if (tracingEmitNewAttributes) { + activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(protocolVersion)); activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode); } @@ -397,6 +400,7 @@ internal static class HttpWebRequestActivitySource { HttpStatusCode? httpStatusCode = null; string errorType = null; + Version protocolVersion = null; // Activity may be null if we are not tracing in these cases: // 1. No listeners @@ -415,11 +419,12 @@ internal static class HttpWebRequestActivitySource errorType = GetErrorType(ex); if (activity != null) { - AddExceptionTags(ex, activity, out httpStatusCode); + AddExceptionTags(ex, activity, out httpStatusCode, out protocolVersion); } else if (ex is WebException wexc && wexc.Response is HttpWebResponse response) { httpStatusCode = response.StatusCode; + protocolVersion = response.ProtocolVersion; } } else @@ -447,6 +452,7 @@ internal static class HttpWebRequestActivitySource } httpStatusCode = responseCopy.StatusCode; + protocolVersion = responseCopy.ProtocolVersion; } else { @@ -456,6 +462,7 @@ internal static class HttpWebRequestActivitySource } httpStatusCode = response.StatusCode; + protocolVersion = response.ProtocolVersion; } if (SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)httpStatusCode.Value) == ActivityStatusCode.Error) @@ -531,7 +538,11 @@ internal static class HttpWebRequestActivitySource tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme); - tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(request.ProtocolVersion)); + if (protocolVersion != null) + { + tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(protocolVersion)); + } + if (!request.RequestUri.IsDefaultPort) { tags.Add(SemanticConventions.AttributeServerPort, request.RequestUri.Port); diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index ec7b4aac2..dfa897388 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -339,13 +339,13 @@ public partial class HttpClientTests var normalizedAttributes = activity.TagObjects.Where(kv => !kv.Key.StartsWith("otel.")).ToDictionary(x => x.Key, x => x.Value.ToString()); - int numberOfNewTags = activity.Status == ActivityStatusCode.Error ? 6 : 5; - int numberOfDupeTags = activity.Status == ActivityStatusCode.Error ? 12 : 11; + int numberOfNewTags = activity.Status == ActivityStatusCode.Error ? 5 : 4; + int numberOfDupeTags = activity.Status == ActivityStatusCode.Error ? 11 : 10; var expectedAttributeCount = semanticConvention == HttpSemanticConvention.Dupe - ? numberOfDupeTags + (tc.ResponseExpected ? 2 : 0) + ? numberOfDupeTags + (tc.ResponseExpected ? 3 : 0) : semanticConvention == HttpSemanticConvention.New - ? numberOfNewTags + (tc.ResponseExpected ? 1 : 0) + ? numberOfNewTags + (tc.ResponseExpected ? 2 : 0) : 6 + (tc.ResponseExpected ? 1 : 0); Assert.Equal(expectedAttributeCount, normalizedAttributes.Count); @@ -374,9 +374,9 @@ public partial class HttpClientTests Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeUrlFull && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpUrl]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); if (tc.ResponseExpected) { + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); if (tc.ResponseCode >= 400) @@ -387,6 +387,7 @@ public partial class HttpClientTests else { Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); + Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion); #if NET8_0_OR_GREATER // we are using fake address so it will be "name_resolution_error" @@ -532,20 +533,18 @@ public partial class HttpClientTests attributes[tag.Key] = tag.Value; } -#if !NET8_0_OR_GREATER - var numberOfTags = 6; -#else - // network.protocol.version is not emitted when response if not received. - // https://github.com/open-telemetry/opentelemetry-dotnet/issues/4928 - var numberOfTags = 5; -#endif + var numberOfTags = 4; if (tc.ResponseExpected) { var expectedStatusCode = int.Parse(normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); - numberOfTags = (expectedStatusCode >= 400) ? 6 : 5; + numberOfTags = (expectedStatusCode >= 400) ? 5 : 4; // error.type extra tag + } + else + { + numberOfTags = 5; // error.type would be extra } - var expectedAttributeCount = numberOfTags + (tc.ResponseExpected ? 1 : 0); + var expectedAttributeCount = numberOfTags + (tc.ResponseExpected ? 2 : 0); // responsecode + protocolversion Assert.Equal(expectedAttributeCount, attributes.Count); @@ -553,12 +552,10 @@ public partial class HttpClientTests Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeUrlScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]); -#if !NET8_0_OR_GREATER - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); -#endif if (tc.ResponseExpected) { + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); if (tc.ResponseCode >= 400) @@ -568,6 +565,7 @@ public partial class HttpClientTests } else { + Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion); Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); #if NET8_0_OR_GREATER