[Instrumentation.Http] follow up from previous pr (#4919)

open-telemetry · Oct 6, 2023 · be45aab · be45aab
1 parent c2293e0
commit be45aab
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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<MetricPoint>();
@@ -381,6 +382,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
                     Assert.True(sum > 0);
                 }
 
+                // Inspect Metric Attributes
                 var attributes = new Dictionary<string, object>();
                 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<double>();
+                foreach (var t in histogramBuckets)
+                {
+                    histogramBounds.Add(t.ExplicitBound);
                 }
+
+                Assert.Equal(
+                    expected: new List<double> { 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<MetricPoint>();
@@ -439,6 +451,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
                     Assert.True(sum > 0);
                 }
 
+                // Inspect Metric Attributes
                 var attributes = new Dictionary<string, object>();
                 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<double>();
+                foreach (var t in histogramBuckets)
+                {
+                    histogramBounds.Add(t.ExplicitBound);
+                }
+
+                Assert.Equal(
+                    expected: new List<double> { 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);
             }
         }
     }