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.

Sign up for GitHub

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 HttpClient metrics for .NET Framework #4768

Closed
wants to merge 12 commits into from
Closed

Add HttpClient metrics for .NET Framework #4768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
85331a6
piggyback off HttpClient tracing implementation to record metrics
matt-hensley Aug 10, 2023
c565eff
modify test for net462 metrics
matt-hensley Aug 10, 2023
de98635
modify AddHttpClientInstrumentation to match tracing impl with name p…
matt-hensley Aug 14, 2023
b4405a0
Merge branch 'main' into net462httpclientmetrics
matt-hensley Aug 14, 2023
3a208b9
removed call to set endtime
matt-hensley Aug 16, 2023
f52b469
updated CHANGELOG + README
matt-hensley Aug 17, 2023
56e2203
resolve public API warnings
matt-hensley Aug 17, 2023
dadfeb4
swap to more performant enumeration https://github.com/dotnet/runtime…
matt-hensley Aug 17, 2023
364b1c2
skip copying activity tags if metric isn't subscribed to (enabled)
matt-hensley Aug 19, 2023
95c433b
move .NET/.NET Core instrumentation to else block
matt-hensley Aug 19, 2023
1b4c3cd
expanded instructions around tracing requirements
matt-hensley Aug 21, 2023
06c704b
Merge branch 'main' into net462httpclientmetrics
matt-hensley Aug 21, 2023
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
1 change: 1 addition & 0 deletions src/OpenTelemetry.Instrumentation.Http/.publicApi/net462/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordExcept
OpenTelemetry.Metrics.MeterProviderBuilderExtensions
OpenTelemetry.Trace.TracerProviderBuilderExtensions
static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
1 change: 1 addition & 0 deletions src/OpenTelemetry.Instrumentation.Http/.publicApi/net6.0/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordExcept
OpenTelemetry.Metrics.MeterProviderBuilderExtensions
OpenTelemetry.Trace.TracerProviderBuilderExtensions
static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
1 change: 1 addition & 0 deletions src/OpenTelemetry.Instrumentation.Http/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordExcept
OpenTelemetry.Metrics.MeterProviderBuilderExtensions
OpenTelemetry.Trace.TracerProviderBuilderExtensions
static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

* Added support for exposing `http.client.duration` on .NET Framework for `HttpWebRequest`, including `HttpClient`. Metrics support requires tracing to be enabled via `AddHttpClientInstrumentation` (#4768)[https://github.com/open-telemetry/opentelemetry-dotnet/pull/4768]

## 1.5.1-beta.1

Released 2023-Jul-20
Expand Down
51 changes: 51 additions & 0 deletions src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#if NETFRAMEWORK
using System.Collections;
using System.Diagnostics;
using System.Diagnostics.Metrics;
using System.Net;
using System.Reflection;
using System.Reflection.Emit;
Expand All @@ -39,13 +40,17 @@ internal static class HttpWebRequestActivitySource
internal static readonly AssemblyName AssemblyName = typeof(HttpWebRequestActivitySource).Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name + ".HttpWebRequest";
internal static readonly string ActivityName = ActivitySourceName + ".HttpRequestOut";
internal static readonly string InstrumentationName = AssemblyName.Name;

internal static readonly Func<HttpWebRequest, string, IEnumerable<string>> HttpWebRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name);
internal static readonly Action<HttpWebRequest, string, string> HttpWebRequestHeaderValuesSetter = (request, name, value) => request.Headers.Add(name, value);

private static readonly Version Version = AssemblyName.Version;
private static readonly ActivitySource WebRequestActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());

private static readonly Meter WebRequestMeter = new Meter(InstrumentationName, Version.ToString());
private static readonly Histogram<double> HttpClientDuration;

private static HttpClientInstrumentationOptions options;

private static bool emitOldAttributes;
Expand Down Expand Up @@ -87,6 +92,7 @@ static HttpWebRequestActivitySource()
PerformInjection();

Options = new HttpClientInstrumentationOptions();
HttpClientDuration = WebRequestMeter.CreateHistogram<double>("http.client.duration", "ms", "Measures the duration of outbound HTTP requests.");
}
catch (Exception ex)
{
Expand Down Expand Up @@ -396,6 +402,51 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
}

activity.Stop();

// Only calculate duration if the Meter is subscribed to
if (HttpClientDuration.Enabled)
{
TagList tags = default;

// see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md
if (emitOldAttributes)
{
foreach (ref readonly var tag in activity.EnumerateTagObjects())
{
switch (tag.Key)
{
case SemanticConventions.AttributeHttpMethod:
case SemanticConventions.AttributeHttpScheme:
case SemanticConventions.AttributeHttpFlavor:
case SemanticConventions.AttributeNetPeerName:
case SemanticConventions.AttributeNetPeerPort:
case SemanticConventions.AttributeHttpStatusCode:
tags.Add(new KeyValuePair<string, object>(tag.Key, tag.Value));
break;
}
}
}

// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md
if (emitNewAttributes)
{
foreach (ref readonly var tag in activity.EnumerateTagObjects())
{
switch (tag.Key)
{
case SemanticConventions.AttributeHttpRequestMethod:
case SemanticConventions.AttributeNetworkProtocolVersion:
case SemanticConventions.AttributeServerAddress:
case SemanticConventions.AttributeServerPort:
case SemanticConventions.AttributeHttpResponseStatusCode:
tags.Add(new KeyValuePair<string, object>(tag.Key, tag.Value));
break;
}
}
}

HttpClientDuration.Record(activity.Duration.TotalMilliseconds, tags);
}
}

private static void PrepareReflectionObjects()
Expand Down
31 changes: 30 additions & 1 deletion src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,26 @@ public static class MeterProviderBuilderExtensions
/// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
public static MeterProviderBuilder AddHttpClientInstrumentation(
this MeterProviderBuilder builder)
{
return AddHttpClientInstrumentation(builder, name: null);
}

/// <summary>
/// Enables HttpClient instrumentation.
/// </summary>
/// <param name="builder"><see cref="MeterProviderBuilder"/> being configured.</param>
/// <param name="name">Name which is used when retrieving options.</param>
/// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
public static MeterProviderBuilder AddHttpClientInstrumentation(
matt-hensley marked this conversation as resolved.
Show resolved Hide resolved
this MeterProviderBuilder builder, string name)
{
Guard.ThrowIfNull(builder);

// Note: Warm-up the status code mapping.
_ = TelemetryHelper.BoxedStatusCodes;

name ??= Options.DefaultName;

builder.ConfigureServices(services =>
{
services.RegisterOptionsFactory(configuration => new HttpClientMetricInstrumentationOptions(configuration));
Expand All @@ -53,8 +67,23 @@ public static MeterProviderBuilder AddHttpClientInstrumentation(
// 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.InstrumentationName);
builder.ConfigureServices(s =>
{
s.ConfigureOpenTelemetryMeterProvider((sp, _) =>
{
var options = sp.GetRequiredService<IOptionsMonitor<HttpClientInstrumentationOptions>>().Get(name);

HttpWebRequestActivitySource.Options = options;
});
});
#else
builder.AddMeter(HttpClientMetrics.InstrumentationName);
return builder.AddInstrumentation(sp => new HttpClientMetrics(
builder.AddInstrumentation(sp => new HttpClientMetrics(
sp.GetRequiredService<IOptionsMonitor<HttpClientMetricInstrumentationOptions>>().CurrentValue));
#endif

return builder;
}
}
4 changes: 3 additions & 1 deletion src/OpenTelemetry.Instrumentation.Http/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ public class Program
#### Metrics

> **Note**
> Metrics are not available for .NET Framework.
> Metrics are only available for .NET Framework when traces are recorded. This requires:
> 1. Tracing to be enabled by calling `.AddHttpClientInstrumentation()` on `TracerProviderBuilder`
> 2. [If a sampler is in place, return `SamplingDecision.RecordAndSampled` for `ShouldSample`](https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/docs/trace/extending-the-sdk/README.md#filtering-processor)
Copy link
Member

Choose a reason for hiding this comment

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

nits:
"If a sampler" is not right, as there is always a sampler! Suggestion:
The sampler must return RecordAndSample for ever Activity in order for the Metrics to be accurate. If some spans (Activity) are sampled, but others not, it'll affect Metrics accuracy, as Metric Tags (Attributes) are generated based on the Activity.

Copy link
Member

Choose a reason for hiding this comment

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

also, the user provided Filters can also affect this,

opentelemetry-dotnet/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs

Line 274 in 06c704b

if (!WebRequestActivitySource.HasListeners() || !Options.EventFilterHttpWebRequest(request))
.

  1. Enable tracing.
  2. Sampler returns RecordAndSample
  3. No user defined filters.

Copy link
Member

Choose a reason for hiding this comment

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

I suggest revisit the design of relying on Activity for generating metrics, and instead produce Metrics independently.
The approach done in this PR is more like a "Span2Metrics" processor, and can done as a regular ActivityProcessor instead, so it'll be easy to convey that Metrics are only as good as the underlying Activity.

Could you check how big of a change it'd to produce metrics independently? (It may not be as bad to rely on Activity for duration alone...but not for the whole tags.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did some research and have an idea. Could follow a similar pattern to HttpClient. The idea would be to:

  • Call ActivitySource.CreateActivity if tracing is enabled. This Activity would be used by metrics and traces
  • Instantiate a standalone Activity if only metrics are enabled. This wouldn't cause a trace to be emitted
  • Null Activity if neither metrics nor traces are enabled

If metrics or traces are enabled, the existing tags would still be set, with the only difference being whether the Activity ends up processed by an exporter.

Does this approach sound OK? There are some details to work through, but this looks feasible to me. Also welcome to other ideas if anyone has thoughts.


The following example demonstrates adding `HttpClient` instrumentation with the
extension method `.AddHttpClientInstrumentation()` on `MeterProviderBuilder` to
Expand Down
8 changes: 4 additions & 4 deletions test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,6 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut
Assert.True(enrichWithExceptionCalled);
}

#if NETFRAMEWORK
Assert.Empty(requestMetrics);
#else
Assert.Single(requestMetrics);

var metric = requestMetrics[0];
Expand Down Expand Up @@ -193,7 +190,11 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut
var method = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpMethod, tc.Method);
var scheme = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpScheme, "http");
var statusCode = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpStatusCode, tc.ResponseCode == 0 ? 200 : tc.ResponseCode);
#if NETFRAMEWORK
var flavor = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpFlavor, "1.1");
#else
var flavor = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpFlavor, "2.0");
#endif
var hostName = new KeyValuePair<string, object>(SemanticConventions.AttributeNetPeerName, tc.ResponseExpected ? host : "sdlfaldfjalkdfjlkajdflkajlsdjf");
var portNumber = new KeyValuePair<string, object>(SemanticConventions.AttributeNetPeerPort, port);
Assert.Contains(hostName, attributes);
Expand All @@ -211,7 +212,6 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut
Assert.DoesNotContain(statusCode, attributes);
Assert.Equal(5, attributes.Length);
}
#endif
}

[Fact]
Expand Down