From be45aab2bad1d6ca27a9ab6d78d49398011656d2 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Date: Thu, 5 Oct 2023 17:17:53 -0700 Subject: [PATCH] [Instrumentation.Http] follow up from previous pr (#4919) --- .../CHANGELOG.md | 6 +-- .../README.md | 30 ++++++++--- .../HttpClientTests.cs | 53 ++++++++++++++----- 3 files changed, 66 insertions(+), 23 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 26a3971a0f9..1443489e51d 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -2,8 +2,8 @@ ## Unreleased -* Introduced a new metric, `http.client.request.duration` measured in seconds. - The OTel SDK +* Introduced a new metric for `HttpClient`, `http.client.request.duration` + measured in seconds. The OTel SDK [applies custom histogram buckets](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4820) for this metric to comply with the [Semantic Convention for Http Metrics](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md). @@ -31,7 +31,7 @@ * [http-metrics](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md) * Added support for publishing `http.client.duration` & - `http.client.request.duration` metrics on .NET Framework + `http.client.request.duration` metrics on .NET Framework for `HttpWebRequest`. ([#4870](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4870)) ## 1.5.1-beta.1 diff --git a/src/OpenTelemetry.Instrumentation.Http/README.md b/src/OpenTelemetry.Instrumentation.Http/README.md index 18b6192a105..0e36fc1d3e5 100644 --- a/src/OpenTelemetry.Instrumentation.Http/README.md +++ b/src/OpenTelemetry.Instrumentation.Http/README.md @@ -99,13 +99,31 @@ to see how to enable this instrumentation in an ASP.NET application. #### List of metrics produced -The instrumentation is implemented based on [metrics semantic -conventions](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#metric-httpclientduration). -Currently, the instrumentation supports the following metric. +A different metric is emitted depending on whether a user opts-in to the new +Http Semantic Conventions using `OTEL_SEMCONV_STABILITY_OPT_IN`. -| Name | Instrument Type | Unit | Description | -|-------|-----------------|------|-------------| -| `http.client.duration` | Histogram | `ms` | Measures the duration of outbound HTTP requests. | +* By default, the instrumentation emits the following metric. + + | Name | Instrument Type | Unit | Description | + |-------|-----------------|------|-------------| + | `http.client.duration` | Histogram | `ms` | Measures the duration of outbound HTTP requests. | + +* If user sets the environment variable to `http`, the instrumentation emits + the following metric. + + | Name | Instrument Type | Unit | Description | + |-------|-----------------|------|-------------| + | `http.client.request.duration` | Histogram | `s` | Measures the duration of outbound HTTP requests. | + + This metric is emitted in `seconds` as per the semantic convention. While + the convention [recommends using custom histogram buckets](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md) + , this feature is not yet available via .NET Metrics API. + A [workaround](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4820) + has been included in OTel SDK starting version `1.6.0` which applies + recommended buckets by default for `http.client.request.duration`. + +* If user sets the environment variable to `http/dup`, the instrumentation + emits both `http.client.duration` and `http.client.request.duration`. ## Advanced configuration diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index 93bdfa54ecb..07cf5df58ae 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -355,6 +355,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( { var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.duration"); Assert.NotNull(metric); + Assert.Equal("ms", metric.Unit); Assert.True(metric.MetricType == MetricType.Histogram); var metricPoints = new List(); @@ -381,6 +382,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( Assert.True(sum > 0); } + // Inspect Metric Attributes var attributes = new Dictionary(); foreach (var tag in metricPoint.Tags) { @@ -391,28 +393,38 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( Assert.Equal(expectedAttributeCount, attributes.Count); - if (semanticConvention == null || semanticConvention.Value.HasFlag(HttpSemanticConvention.Old)) + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerName && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpFlavor && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); + if (tc.ResponseExpected) { - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerName && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpFlavor && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); - if (tc.ResponseExpected) - { - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); - } - else - { - Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode); - } + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); + } + else + { + Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode); + } + + // Inspect Histogram Bounds + var histogramBuckets = metricPoint.GetHistogramBuckets(); + var histogramBounds = new List(); + foreach (var t in histogramBuckets) + { + histogramBounds.Add(t.ExplicitBound); } + + Assert.Equal( + expected: new List { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000, double.PositiveInfinity }, + actual: histogramBounds); } if (semanticConvention != null && semanticConvention.Value.HasFlag(HttpSemanticConvention.New)) { var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.request.duration"); Assert.NotNull(metric); + Assert.Equal("s", metric.Unit); Assert.True(metric.MetricType == MetricType.Histogram); var metricPoints = new List(); @@ -439,6 +451,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( Assert.True(sum > 0); } + // Inspect Metric Attributes var attributes = new Dictionary(); foreach (var tag in metricPoint.Tags) { @@ -461,6 +474,18 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( { Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); } + + // Inspect Histogram Bounds + var histogramBuckets = metricPoint.GetHistogramBuckets(); + var histogramBounds = new List(); + foreach (var t in histogramBuckets) + { + histogramBounds.Add(t.ExplicitBound); + } + + Assert.Equal( + expected: new List { 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10, double.PositiveInfinity }, + actual: histogramBounds); } } }