diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index d732ac05166..715a06ff6dd 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Fixed an issue of missing `http.client.duration` metric data in case of +network failures (when response is not available). +([#4098](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4098)) + ## 1.0.0-rc9.14 Released 2023-Feb-24 diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index d8306a84363..8a533f342cf 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -28,6 +28,7 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler internal const string OnStopEvent = "System.Net.Http.HttpRequestOut.Stop"; private readonly PropertyFetcher stopResponseFetcher = new("Response"); + private readonly PropertyFetcher stopRequestFetcher = new("Request"); private readonly Histogram httpClientDuration; public HttpHandlerMetricsDiagnosticListener(string name, Meter meter) @@ -46,14 +47,11 @@ public override void OnEventWritten(string name, object payload) } var activity = Activity.Current; - if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null) + if (this.stopRequestFetcher.TryFetch(payload, out HttpRequestMessage request) && request != null) { - var request = response.RequestMessage; - TagList tags = default; tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method))); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme)); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode)); tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version))); tags.Add(new KeyValuePair(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host)); @@ -62,6 +60,11 @@ public override void OnEventWritten(string name, object payload) tags.Add(new KeyValuePair(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port)); } + if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null) + { + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode)); + } + // We are relying here on HttpClient library to set duration before writing the stop event. // https://github.com/dotnet/runtime/blob/90603686d314147017c8bbe1fa8965776ce607d0/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L178 // TODO: Follow up with .NET team if we can continue to rely on this behavior. diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index e8f4e08a4ed..e4e484e062b 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -157,58 +157,59 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut Assert.True(enrichWithExceptionCalled); } - if (tc.ResponseExpected) - { #if NETFRAMEWORK - Assert.Empty(requestMetrics); + Assert.Empty(requestMetrics); #else - Assert.Single(requestMetrics); + Assert.Single(requestMetrics); - var metric = requestMetrics[0]; - Assert.NotNull(metric); - Assert.True(metric.MetricType == MetricType.Histogram); + var metric = requestMetrics[0]; + Assert.NotNull(metric); + Assert.True(metric.MetricType == MetricType.Histogram); - var metricPoints = new List(); - foreach (var p in metric.GetMetricPoints()) - { - metricPoints.Add(p); - } + var metricPoints = new List(); + foreach (var p in metric.GetMetricPoints()) + { + metricPoints.Add(p); + } - Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; - var count = metricPoint.GetHistogramCount(); - var sum = metricPoint.GetHistogramSum(); + var count = metricPoint.GetHistogramCount(); + var sum = metricPoint.GetHistogramSum(); - Assert.Equal(1L, count); - Assert.Equal(activity.Duration.TotalMilliseconds, sum); + Assert.Equal(1L, count); + Assert.Equal(activity.Duration.TotalMilliseconds, sum); - var attributes = new KeyValuePair[metricPoint.Tags.Count]; - int i = 0; - foreach (var tag in metricPoint.Tags) - { - attributes[i++] = tag; - } + var attributes = new KeyValuePair[metricPoint.Tags.Count]; + int i = 0; + foreach (var tag in metricPoint.Tags) + { + attributes[i++] = tag; + } - var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, tc.Method); - var scheme = new KeyValuePair(SemanticConventions.AttributeHttpScheme, "http"); - var statusCode = new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, tc.ResponseCode == 0 ? 200 : tc.ResponseCode); - var flavor = new KeyValuePair(SemanticConventions.AttributeHttpFlavor, "2.0"); - var hostName = new KeyValuePair(SemanticConventions.AttributeNetPeerName, host); - var portNumber = new KeyValuePair(SemanticConventions.AttributeNetPeerPort, port); - Assert.Contains(method, attributes); - Assert.Contains(scheme, attributes); + var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, tc.Method); + var scheme = new KeyValuePair(SemanticConventions.AttributeHttpScheme, "http"); + var statusCode = new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, tc.ResponseCode == 0 ? 200 : tc.ResponseCode); + var flavor = new KeyValuePair(SemanticConventions.AttributeHttpFlavor, "2.0"); + var hostName = new KeyValuePair(SemanticConventions.AttributeNetPeerName, tc.ResponseExpected ? host : "sdlfaldfjalkdfjlkajdflkajlsdjf"); + var portNumber = new KeyValuePair(SemanticConventions.AttributeNetPeerPort, port); + Assert.Contains(hostName, attributes); + Assert.Contains(portNumber, attributes); + Assert.Contains(method, attributes); + Assert.Contains(scheme, attributes); + Assert.Contains(flavor, attributes); + if (tc.ResponseExpected) + { Assert.Contains(statusCode, attributes); - Assert.Contains(flavor, attributes); - Assert.Contains(hostName, attributes); - Assert.Contains(portNumber, attributes); Assert.Equal(6, attributes.Length); -#endif } else { - Assert.Empty(requestMetrics); + Assert.DoesNotContain(statusCode, attributes); + Assert.Equal(5, attributes.Length); } +#endif } [Fact] diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json index c9f68f4698b..833e26098bb 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json @@ -90,35 +90,37 @@ { "name": "Call that cannot resolve DNS will be reported as error span", "method": "GET", - "url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/", + "url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/", "spanName": "HTTP GET", "spanStatus": "Error", "spanStatusHasDescription": true, "responseExpected": false, "recordException": false, "spanAttributes": { - "http.scheme": "https", + "http.scheme": "http", "http.method": "GET", - "net.peer.name": "sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com", + "net.peer.name": "sdlfaldfjalkdfjlkajdflkajlsdjf", + "net.peer.port": "{port}", "http.flavor": "{flavor}", - "http.url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/" + "http.url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/" } }, { "name": "Call that cannot resolve DNS will be reported as error span. And Records exception", "method": "GET", - "url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/", + "url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/", "spanName": "HTTP GET", "spanStatus": "Error", "spanStatusHasDescription": true, "responseExpected": false, "recordException": true, "spanAttributes": { - "http.scheme": "https", + "http.scheme": "http", "http.method": "GET", - "net.peer.name": "sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com", + "net.peer.name": "sdlfaldfjalkdfjlkajdflkajlsdjf", + "net.peer.port": "{port}", "http.flavor": "{flavor}", - "http.url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/" + "http.url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/" } }, {