Skip to content

Commit

Permalink
Add support for .NET8.0 HttpClient metrics (#4931)
Browse files Browse the repository at this point in the history
  • Loading branch information
vishweshbankwar authored Oct 20, 2023
1 parent 38e21a9 commit d07d030
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 22 deletions.
29 changes: 29 additions & 0 deletions src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,35 @@
`http.client.request.duration` metrics on .NET Framework for `HttpWebRequest`.
([#4870](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4870))

* Following `HttpClient` metrics will now be enabled by default when targeting
`.NET8.0` framework or newer.

* **Meter** : `System.Net.Http`
* `http.client.request.duration`
* `http.client.active_requests`
* `http.client.open_connections`
* `http.client.connection.duration`
* `http.client.request.time_in_queue`

* **Meter** : `System.Net.NameResolution`
* `dns.lookups.duration`

For details about each individual metric check [System.Net metrics
docs
page](https://learn.microsoft.com/dotnet/core/diagnostics/built-in-metrics-system-net).

**NOTES**:
* When targeting `.NET8.0` framework or newer, `http.client.request.duration` metric
will only follow
[v1.22.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-metrics.md#metric-httpclientrequestduration)
semantic conventions specification. Ability to switch behavior to older
conventions using `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable is
not available.
* Users can opt-out of metrics that are not required using
[views](https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/metrics/customizing-the-sdk#drop-an-instrument).

([#4931](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4931))

## 1.5.1-beta.1

Released 2023-Jul-20
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
// limitations under the License.
// </copyright>

#if !NET8_0_OR_GREATER
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using OpenTelemetry.Instrumentation.Http;
using OpenTelemetry.Instrumentation.Http.Implementation;
#endif

using OpenTelemetry.Internal;

namespace OpenTelemetry.Metrics;
Expand All @@ -37,6 +40,11 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
{
Guard.ThrowIfNull(builder);

#if NET8_0_OR_GREATER
return builder
.AddMeter("System.Net.Http")
.AddMeter("System.Net.NameResolution");
#else
// Note: Warm-up the status code mapping.
_ = TelemetryHelper.BoxedStatusCodes;

Expand All @@ -45,12 +53,6 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
services.RegisterOptionsFactory(configuration => new HttpClientMetricInstrumentationOptions(configuration));
});

// TODO: Handle HttpClientMetricInstrumentationOptions
// SetHttpFlavor - seems like this would be handled by views
// Filter - makes sense for metric instrumentation
// Enrich - do we want a similar kind of functionality for metrics?
// RecordException - probably doesn't make sense for metric instrumentation

#if NETFRAMEWORK
builder.AddMeter(HttpWebRequestActivitySource.MeterName);

Expand All @@ -70,5 +72,6 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
sp.GetRequiredService<IOptionsMonitor<HttpClientMetricInstrumentationOptions>>().Get(Options.DefaultName)));
#endif
return builder;
#endif
}
}
85 changes: 73 additions & 12 deletions test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
#if NETFRAMEWORK
using System.Net.Http;
#endif
#if !NET8_0_OR_GREATER
using System.Reflection;
using System.Text.Json;
#endif
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using OpenTelemetry.Metrics;
Expand All @@ -33,6 +35,7 @@ public partial class HttpClientTests
{
public static readonly IEnumerable<object[]> TestData = HttpTestData.ReadTestCases();

#if !NET8_0_OR_GREATER
[Theory]
[MemberData(nameof(TestData))]
public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsOldSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc)
Expand Down Expand Up @@ -71,29 +74,30 @@ await HttpOutCallsAreCollectedSuccessfullyBodyAsync(
enableMetrics: true,
semanticConvention: HttpSemanticConvention.Dupe).ConfigureAwait(false);
}
#endif

[Theory]
[MemberData(nameof(TestData))]
public async Task HttpOutCallsAreCollectedSuccessfullyTracesOnlyAsync(HttpTestData.HttpOutTestCase tc)
public async Task HttpOutCallsAreCollectedSuccessfullyMetricsOnlyAsync(HttpTestData.HttpOutTestCase tc)
{
await HttpOutCallsAreCollectedSuccessfullyBodyAsync(
this.host,
this.port,
tc,
enableTracing: true,
enableMetrics: false).ConfigureAwait(false);
enableTracing: false,
enableMetrics: true).ConfigureAwait(false);
}

[Theory]
[MemberData(nameof(TestData))]
public async Task HttpOutCallsAreCollectedSuccessfullyMetricsOnlyAsync(HttpTestData.HttpOutTestCase tc)
public async Task HttpOutCallsAreCollectedSuccessfullyTracesOnlyAsync(HttpTestData.HttpOutTestCase tc)
{
await HttpOutCallsAreCollectedSuccessfullyBodyAsync(
this.host,
this.port,
tc,
enableTracing: false,
enableMetrics: true).ConfigureAwait(false);
enableTracing: true,
enableMetrics: false).ConfigureAwait(false);
}

[Theory]
Expand All @@ -108,6 +112,7 @@ await HttpOutCallsAreCollectedSuccessfullyBodyAsync(
enableMetrics: false).ConfigureAwait(false);
}

#if !NET8_0_OR_GREATER
[Fact]
public async Task DebugIndividualTestAsync()
{
Expand Down Expand Up @@ -140,6 +145,7 @@ public async Task DebugIndividualTestAsync()
var t = (Task)this.GetType().InvokeMember(nameof(this.HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsOldSemanticConventionsAsync), BindingFlags.InvokeMethod, null, this, HttpTestData.GetArgumentsFromTestCaseObject(input).First());
await t.ConfigureAwait(false);
}
#endif

[Fact]
public async Task CheckEnrichmentWhenSampling()
Expand All @@ -148,6 +154,65 @@ public async Task CheckEnrichmentWhenSampling()
await CheckEnrichment(new AlwaysOnSampler(), true, this.url).ConfigureAwait(false);
}

#if NET8_0_OR_GREATER
[Theory]
[MemberData(nameof(TestData))]
public async Task ValidateNet8MetricsAsync(HttpTestData.HttpOutTestCase tc)
{
var metrics = new List<Metric>();
var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddHttpClientInstrumentation()
.AddInMemoryExporter(metrics)
.Build();

var testUrl = HttpTestData.NormalizeValues(tc.Url, this.host, this.port);

try
{
using var c = new HttpClient();
using var request = new HttpRequestMessage
{
RequestUri = new Uri(testUrl),
Method = new HttpMethod(tc.Method),
};

request.Headers.Add("contextRequired", "false");
request.Headers.Add("responseCode", (tc.ResponseCode == 0 ? 200 : tc.ResponseCode).ToString());
await c.SendAsync(request).ConfigureAwait(false);
}
catch (Exception)
{
// test case can intentionally send request that will result in exception
}
finally
{
meterProvider.Dispose();
}

// dns.lookups.duration is a typo
// https://github.com/dotnet/runtime/issues/92917
var requestMetrics = metrics
.Where(metric =>
metric.Name == "http.client.request.duration" ||
metric.Name == "http.client.active_requests" ||
metric.Name == "http.client.request.time_in_queue" ||
metric.Name == "http.client.connection.duration" ||
metric.Name == "http.client.open_connections" ||
metric.Name == "dns.lookups.duration")
.ToArray();

if (tc.ResponseExpected)
{
Assert.Equal(6, requestMetrics.Count());
}
else
{
// http.client.connection.duration and http.client.open_connections will not be emitted.
Assert.Equal(4, requestMetrics.Count());
}
}
#endif

private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
string host,
int port,
Expand Down Expand Up @@ -210,11 +275,6 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
{
RequestUri = new Uri(testUrl),
Method = new HttpMethod(tc.Method),
#if NETFRAMEWORK
Version = new Version(1, 1),
#else
Version = new Version(2, 0),
#endif
};

if (tc.Headers != null)
Expand Down Expand Up @@ -351,6 +411,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
Assert.Single(requestMetrics);
}

#if !NET8_0_OR_GREATER
if (semanticConvention == null || semanticConvention.Value.HasFlag(HttpSemanticConvention.Old))
{
var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.duration");
Expand Down Expand Up @@ -419,7 +480,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
expected: new List<double> { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000, double.PositiveInfinity },
actual: histogramBounds);
}

#endif
if (semanticConvention != null && semanticConvention.Value.HasFlag(HttpSemanticConvention.New))
{
var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.request.duration");
Expand Down
4 changes: 0 additions & 4 deletions test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ public static string NormalizeValues(string value, string host, int port)
return value
.Replace("{host}", host)
.Replace("{port}", port.ToString())
#if NETFRAMEWORK
.Replace("{flavor}", "1.1");
#else
.Replace("{flavor}", "2.0");
#endif
}

public class HttpOutTestCase
Expand Down

0 comments on commit d07d030

Please sign in to comment.