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

Capture http.server.duration metric for ASP.NET Core instrumentation #1347

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
fbed107
Capture http.server.duration metric for ASP.NET Core instrumentation
alanwest Oct 13, 2020
c029e26
Fix build
alanwest Oct 13, 2020
a5cbfa7
Use constants from SemanticConventions
alanwest Oct 13, 2020
d1181bb
Provide Meter through factory method
alanwest Oct 14, 2020
9334355
Use StringComparer.Ordinal
alanwest Oct 14, 2020
af68d3c
Fix whitespace
alanwest Oct 14, 2020
0cadfc7
Merge branch 'master' into alanwest/http-metrics-aspnetcore
cijothomas Oct 16, 2020
3e7f067
Merge branch 'master' into alanwest/http-metrics-aspnetcore
alanwest Oct 19, 2020
8551d7d
Dispose instrumentations added in MeterProviderSdk
alanwest Oct 19, 2020
31d2f25
Merge branch 'master' into alanwest/http-metrics-aspnetcore
alanwest Nov 3, 2020
9b32a0e
Move http.server.duration metric name to SemanticConventions
alanwest Nov 3, 2020
e213ec9
Delay creation of metric
alanwest Nov 3, 2020
1809084
Fix namespace
alanwest Nov 3, 2020
f6596b6
Add unit tests
alanwest Nov 4, 2020
45cf5f9
Merge branch 'master' into alanwest/http-metrics-aspnetcore
alanwest Nov 4, 2020
81baa2a
Remove instrumentation options
alanwest Nov 4, 2020
24d5baf
Merge branch 'master' into alanwest/http-metrics-aspnetcore
alanwest Nov 4, 2020
7a5d1ac
Remove TODO
alanwest Nov 4, 2020
3cb3749
Add TODO
alanwest Nov 4, 2020
08f3274
Merge branch 'master' into alanwest/http-metrics-aspnetcore
alanwest Dec 16, 2020
ebaa2ae
Merge branch 'metrics' into alanwest/http-metrics-aspnetcore
cijothomas Feb 5, 2021
35dfefc
Merge branch 'metrics' into alanwest/http-metrics-aspnetcore
cijothomas Mar 27, 2021
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
7 changes: 7 additions & 0 deletions src/OpenTelemetry.Api/Internal/SemanticConventions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ internal static class SemanticConventions
public const string AttributeExceptionType = "exception.type";
public const string AttributeExceptionMessage = "exception.message";
public const string AttributeExceptionStacktrace = "exception.stacktrace";

// TODO: This class is currently in the OpenTelemetry.Trace namespace.
// We could create a new SemanticConventions class in the OpenTelemetry.Metrics namespace,
// though this may be a little strange since many of the attribute names within this class
// are used for both traces and metrics.
// Another option would be to move this class to the OpenTelemetry namespace.
public const string MetricHttpServerDuration = "http.server.duration";
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
}
}
46 changes: 46 additions & 0 deletions src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// <copyright file="AspNetCoreMetrics.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using System;
using OpenTelemetry.Instrumentation.AspNetCore.Implementation;
using OpenTelemetry.Metrics;

namespace OpenTelemetry.Instrumentation.AspNetCore
{
/// <summary>
/// Asp.Net Core Requests instrumentation.
/// </summary>
internal class AspNetCoreMetrics : IDisposable
{
private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;

/// <summary>
/// Initializes a new instance of the <see cref="AspNetCoreMetrics"/> class.
/// </summary>
/// <param name="meter">The meter for obtaining metric instruments.</param>
public AspNetCoreMetrics(Meter meter)
{
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpInMetricsListener("Microsoft.AspNetCore", meter), null);
this.diagnosticSourceSubscriber.Subscribe();
}

/// <inheritdoc/>
public void Dispose()
{
this.diagnosticSourceSubscriber?.Dispose();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// <copyright file="HttpInMetricsListener.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using System;
using System.Collections.Generic;
using System.Diagnostics;
using Microsoft.AspNetCore.Http;
using OpenTelemetry.Metrics;
using OpenTelemetry.Trace;

namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
{
internal class HttpInMetricsListener : ListenerHandler
{
private readonly PropertyFetcher<HttpContext> stopContextFetcher = new PropertyFetcher<HttpContext>("HttpContext");
private readonly Meter meter;

private MeasureMetric<double> httpServerDuration;

public HttpInMetricsListener(string name, Meter meter)
: base(name)
{
this.meter = meter;
}

public override void OnStopActivity(Activity activity, object payload)
{
HttpContext context = this.stopContextFetcher.Fetch(payload);
if (context == null)
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(this.OnStopActivity));
return;
}

// TODO: Prometheus pulls metrics by invoking the /metrics endpoint. Decide if it makes sense to suppress this.
// Below is just a temporary way of achieving this suppression for metrics (we should consider suppressing traces too).
Copy link
Member

Choose a reason for hiding this comment

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

yes, eventually we can leverage the SupressInstrumentation API for this.

// If we want to suppress activity from Prometheus then we should use SuppressInstrumentationScope.
if (context.Request.Path.HasValue && context.Request.Path.Value.Contains("metrics"))
{
return;
}

var labels = new Dictionary<string, string>(StringComparer.Ordinal);

labels[SemanticConventions.AttributeHttpMethod] = context.Request.Method;
alanwest marked this conversation as resolved.
Show resolved Hide resolved

labels[SemanticConventions.AttributeHttpScheme] = context.Request.Scheme;

if (context.Request.Host.HasValue)
{
labels[SemanticConventions.AttributeHttpHost] = context.Request.Host.Value;
labels[SemanticConventions.AttributeNetHostName] = context.Request.Host.Host;

if (context.Request.Host.Port.HasValue)
{
labels[SemanticConventions.AttributeNetHostPort] = context.Request.Host.Port.ToString();
}
}

labels[SemanticConventions.AttributeHttpStatusCode] = context.Response.StatusCode.ToString();
labels[SemanticConventions.AttributeHttpFlavor] = context.Request.Protocol;

// TODO: Decide if/how to handle http.server_name. Spec seems to suggest
// preference for net.host.name.
// labels[SemanticConventions.AttributeHttpServerName] = string.Empty;

// TODO: Decide if we want http.url. It seems awful from a cardinality perspective.
// Retrieving the route data and setting http.target probably makes more sense.
// labels[SemanticConventions.AttributeHttpUrl] = string.Empty;

// TODO: Retrieve the route.
// labels[SemanticConventions.AttributeHttpTarget] = string.Empty;

// TODO: Ideally we could do this in the constructor. However, the instrumentation is usually instantiated
// prior to invoking MeterProvider.SetDefault. Setting the default meter provider is required before metrics
// can be created.
// This should be safe in the meantime since CreateDoubleMeasure uses a ConcurrentDictionary behind the scenes.
if (this.httpServerDuration == null)
{
this.httpServerDuration = this.meter.CreateDoubleMeasure(SemanticConventions.MetricHttpServerDuration);
}

this.httpServerDuration.Record(new SpanContext(activity.Context), activity.Duration.TotalMilliseconds, labels);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// <copyright file="MeterProviderBuilderExtensions.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using System;
using OpenTelemetry.Instrumentation.AspNetCore;

namespace OpenTelemetry.Metrics
{
/// <summary>
/// Extension methods to simplify registering of ASP.NET Core request instrumentation.
/// </summary>
public static class MeterProviderBuilderExtensions
{
/// <summary>
/// Enables the incoming requests automatic data collection for ASP.NET Core.
/// </summary>
/// <param name="builder"><see cref="MeterProviderBuilder"/> being configured.</param>
/// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
public static MeterProviderBuilder AddAspNetCoreInstrumentation(
this MeterProviderBuilder builder)
{
if (builder == null)
{
throw new ArgumentNullException(nameof(builder));
}

builder.AddInstrumentation((meter) => new AspNetCoreMetrics(meter));

return builder;
}
}
}
35 changes: 34 additions & 1 deletion src/OpenTelemetry/Metrics/MeterProviderBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace OpenTelemetry.Metrics
public class MeterProviderBuilder
{
private static readonly TimeSpan DefaultPushInterval = TimeSpan.FromSeconds(60);
private readonly List<InstrumentationFactory> instrumentationFactories = new List<InstrumentationFactory>();
private MetricProcessor metricProcessor;
private MetricExporter metricExporter;
private TimeSpan metricPushInterval;
Expand Down Expand Up @@ -73,6 +74,24 @@ public MeterProviderBuilder SetPushInterval(TimeSpan pushInterval)
return this;
}

public MeterProviderBuilder AddInstrumentation<TInstrumentation>(
Func<Meter, TInstrumentation> instrumentationFactory)
where TInstrumentation : class
{
if (instrumentationFactory == null)
{
throw new ArgumentNullException(nameof(instrumentationFactory));
}

this.instrumentationFactories.Add(
new InstrumentationFactory(
typeof(TInstrumentation).Name,
"semver:" + typeof(TInstrumentation).Assembly.GetName().Version,
instrumentationFactory));

return this;
}

public MeterProvider Build()
{
var cancellationTokenSource = new CancellationTokenSource();
Expand All @@ -85,7 +104,21 @@ public MeterProvider Build()
this.metricPushInterval,
cancellationTokenSource);

return new MeterProviderSdk(this.metricProcessor, meterRegistry, controller, cancellationTokenSource);
return new MeterProviderSdk(this.metricProcessor, this.instrumentationFactories, meterRegistry, controller, cancellationTokenSource);
}

internal readonly struct InstrumentationFactory
{
public readonly string Name;
public readonly string Version;
public readonly Func<Meter, object> Factory;

internal InstrumentationFactory(string name, string version, Func<Meter, object> factory)
{
this.Name = name;
this.Version = version;
this.Factory = factory;
}
}
}
}
28 changes: 27 additions & 1 deletion src/OpenTelemetry/Metrics/MeterProviderSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using OpenTelemetry.Metrics.Export;

Expand All @@ -28,17 +29,32 @@ internal class MeterProviderSdk : MeterProvider
public CancellationTokenSource CancellationTokenSource;
public Dictionary<MeterRegistryKey, MeterSdk> MeterRegistry;

private readonly List<object> instrumentations = new List<object>();
private readonly object syncObject = new object();
private MeterSdk defaultMeter;

internal MeterProviderSdk(MetricProcessor metricProcessor, Dictionary<MeterRegistryKey, MeterSdk> registry, PushMetricController controller, CancellationTokenSource cts)
internal MeterProviderSdk(
MetricProcessor metricProcessor,
IEnumerable<MeterProviderBuilder.InstrumentationFactory> instrumentationFactories,
Dictionary<MeterRegistryKey, MeterSdk> registry,
PushMetricController controller,
CancellationTokenSource cts)
{
this.MetricProcessor = metricProcessor;
this.PushMetricController = controller;
this.CancellationTokenSource = cts;
this.defaultMeter = new MeterSdk(string.Empty, this.MetricProcessor);
this.MeterRegistry = registry;
this.MeterRegistry.Add(new MeterRegistryKey(string.Empty, null), this.defaultMeter);

if (instrumentationFactories.Any())
{
foreach (var instrumentationFactory in instrumentationFactories)
{
var meter = Default.GetMeter(instrumentationFactory.Name, instrumentationFactory.Version);
this.instrumentations.Add(instrumentationFactory.Factory(meter));
alanwest marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

public override Meter GetMeter(string name, string version = null)
Expand All @@ -64,6 +80,16 @@ public override Meter GetMeter(string name, string version = null)

protected override void Dispose(bool disposing)
{
if (this.instrumentations != null)
{
foreach (var item in this.instrumentations)
{
(item as IDisposable)?.Dispose();
}

this.instrumentations.Clear();
}

this.CancellationTokenSource.Dispose();

// TODO: Actually flush the metric processor/exporer/controllers.
Expand Down
Loading