Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for .NET8.0 HttpClient metrics #4931

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,23 @@
`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.
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved

* **Meter** : `System.Net.Http`
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
* `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`
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved

**NOTE**: 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#view).
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
[#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 @@ -31,6 +31,7 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation;
internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler
{
internal const string OnStopEvent = "System.Net.Http.HttpRequestOut.Stop";
internal static readonly bool IsNet8OrGreater;

internal static readonly AssemblyName AssemblyName = typeof(HttpClientMetrics).Assembly.GetName();
internal static readonly string MeterName = AssemblyName.Name;
Expand All @@ -45,6 +46,18 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler
private readonly bool emitOldAttributes;
private readonly bool emitNewAttributes;

static HttpHandlerMetricsDiagnosticListener()
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
{
try
{
IsNet8OrGreater = typeof(HttpClient).Assembly.GetName().Version.Major >= 8;
}
catch (Exception)
{
IsNet8OrGreater = false;
}
}

public HttpHandlerMetricsDiagnosticListener(string name, HttpClientMetricInstrumentationOptions options)
: base(name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,18 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
});
}
#else
builder.AddMeter(HttpHandlerMetricsDiagnosticListener.MeterName);
if (HttpHandlerMetricsDiagnosticListener.IsNet8OrGreater)
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
{
builder.AddMeter("System.Net.Http");
builder.AddMeter("System.Net.NameResolution");
}
else
{
builder.AddMeter(HttpHandlerMetricsDiagnosticListener.MeterName);

builder.AddInstrumentation(sp => new HttpClientMetrics(
sp.GetRequiredService<IOptionsMonitor<HttpClientMetricInstrumentationOptions>>().Get(Options.DefaultName)));
builder.AddInstrumentation(sp => new HttpClientMetrics(
sp.GetRequiredService<IOptionsMonitor<HttpClientMetricInstrumentationOptions>>().Get(Options.DefaultName)));
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
}
#endif
return builder;
}
Expand Down
86 changes: 74 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,66 @@ 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),
Version = new Version(2, 0),
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
};

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
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
.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")
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
.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 +276,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 +412,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 +481,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
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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of interesting. How is it we don't report 2.0 now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#endif
}

public class HttpOutTestCase
Expand Down