open-telemetry
/
opentelemetry-dotnet
mirror of https://github.com/open-telemetry/opentelemetry-dotnet.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[ASP.NET Core] Add `error.type` attribute for tracing and metrics (#4986)

This commit is contained in:
Vishwesh Bankwar 2023-11-03 11:35:29 -07:00 committed by GitHub
parent 37481f1ee6
commit d91be7751b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 120 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs
View File

@ -36,6 +36,8 @@ internal sealed class AspNetCoreMetrics : IDisposable
"Microsoft.AspNetCore.Hosting.HttpRequestIn",
"Microsoft.AspNetCore.Hosting.HttpRequestIn.Start",
"Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop",
"Microsoft.AspNetCore.Diagnostics.UnhandledException",
"Microsoft.AspNetCore.Hosting.UnhandledException",
};
private readonly Func<string, object, object, bool> isEnabled = (eventName, _, _)

8
src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
View File

@ -18,6 +18,14 @@
`http` or `http/dup`.
([#5001](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5001))
* An additional attribute `error.type` will be added to activity and
`http.server.request.duration` metric when the request results in unhandled
exception. The attribute value will be set to full name of exception type.
The attribute will only be added when `OTEL_SEMCONV_STABILITY_OPT_IN`
environment variable is set to `http` or `http/dup`.
([#4986](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4986))
## 1.6.0-beta.2
Released 2023-Oct-26

5
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
View File

@ -425,6 +425,11 @@ internal class HttpInListener : ListenerHandler
return;
}
if (this.emitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeErrorType, exc.GetType().FullName);
}
if (this.options.RecordException)
{
activity.RecordException(exc);

70
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs
View File

@ -20,6 +20,7 @@ using Microsoft.AspNetCore.Http;
using OpenTelemetry.Internal;
#if NET6_0_OR_GREATER
using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.Routing;
#endif
using OpenTelemetry.Trace;
@ -32,9 +33,14 @@ internal sealed class HttpInMetricsListener : ListenerHandler
internal const string HttpServerDurationMetricName = "http.server.duration";
internal const string HttpServerRequestDurationMetricName = "http.server.request.duration";
internal const string OnUnhandledHostingExceptionEvent = "Microsoft.AspNetCore.Hosting.UnhandledException";
internal const string OnUnhandledDiagnosticsExceptionEvent = "Microsoft.AspNetCore.Diagnostics.UnhandledException";
private const string OnStopEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop";
private const string EventName = "OnStopActivity";
private const string NetworkProtocolName = "http";
private static readonly PropertyFetcher<Exception> ExceptionPropertyFetcher = new("Exception");
private static readonly PropertyFetcher<HttpContext> HttpContextPropertyFetcher = new("HttpContext");
private static readonly object ErrorTypeHttpContextItemsKey = new();
private readonly Meter meter;
private readonly AspNetCoreMetricsInstrumentationOptions options;
@ -66,23 +72,65 @@ internal sealed class HttpInMetricsListener : ListenerHandler
public override void OnEventWritten(string name, object payload)
{
if (name == OnStopEvent)
switch (name)
{
if (this.emitOldAttributes)
{
this.OnEventWritten_Old(name, payload);
}
case OnUnhandledDiagnosticsExceptionEvent:
case OnUnhandledHostingExceptionEvent:
{
if (this.emitNewAttributes)
{
this.OnExceptionEventWritten(name, payload);
}
}
if (this.emitNewAttributes)
{
this.OnEventWritten_New(name, payload);
}
break;
case OnStopEvent:
{
if (this.emitOldAttributes)
{
this.OnEventWritten_Old(name, payload);
}
if (this.emitNewAttributes)
{
this.OnEventWritten_New(name, payload);
}
}
break;
}
}
public void OnExceptionEventWritten(string name, object payload)
{
// We need to use reflection here as the payload type is not a defined public type.
if (!TryFetchException(payload, out Exception exc) || !TryFetchHttpContext(payload, out HttpContext ctx))
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(this.OnExceptionEventWritten), HttpServerDurationMetricName);
return;
}
ctx.Items.Add(ErrorTypeHttpContextItemsKey, exc.GetType().FullName);
// See https://github.com/dotnet/aspnetcore/blob/690d78279e940d267669f825aa6627b0d731f64c/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L252
// and https://github.com/dotnet/aspnetcore/blob/690d78279e940d267669f825aa6627b0d731f64c/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs#L174
// this makes sure that top-level properties on the payload object are always preserved.
#if NET6_0_OR_GREATER
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The ASP.NET Core framework guarantees that top level properties are preserved")]
#endif
static bool TryFetchException(object payload, out Exception exc)
=> ExceptionPropertyFetcher.TryFetch(payload, out exc) && exc != null;
#if NET6_0_OR_GREATER
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The ASP.NET Core framework guarantees that top level properties are preserved")]
#endif
static bool TryFetchHttpContext(object payload, out HttpContext ctx)
=> HttpContextPropertyFetcher.TryFetch(payload, out ctx) && ctx != null;
}
public void OnEventWritten_Old(string name, object payload)
{
var context = payload as HttpContext;
if (context == null)
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName);
@ -170,6 +218,10 @@ internal sealed class HttpInMetricsListener : ListenerHandler
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRoute, route));
}
#endif
if (context.Items.TryGetValue(ErrorTypeHttpContextItemsKey, out var errorType))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeErrorType, errorType));
}
// We are relying here on ASP.NET Core to set duration before writing the stop event.
// https://github.com/dotnet/aspnetcore/blob/d6fa351048617ae1c8b47493ba1abbe94c3a24cf/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L449

1
src/Shared/SemanticConventions.cs
View File

@ -110,6 +110,7 @@ internal static class SemanticConventions
public const string AttributeExceptionType = "exception.type";
public const string AttributeExceptionMessage = "exception.message";
public const string AttributeExceptionStacktrace = "exception.stacktrace";
public const string AttributeErrorType = "error.type";
// v1.21.0
// https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md

5
test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs
View File

@ -39,8 +39,8 @@ public class IncomingRequestsCollectionsIsAccordingToTheSpecTests_New
}
[Theory]
[InlineData("/api/values", null, "user-agent", 503, "503")]
[InlineData("/api/values", "?query=1", null, 503, null)]
[InlineData("/api/values", null, "user-agent", 200, null)]
[InlineData("/api/values", "?query=1", null, 200, null)]
[InlineData("/api/exception", null, null, 503, null)]
[InlineData("/api/exception", null, null, 503, null, true)]
public async Task SuccessfulTemplateControllerCallGeneratesASpan_New(
@ -123,6 +123,7 @@ public class IncomingRequestsCollectionsIsAccordingToTheSpecTests_New
if (statusCode == 503)
{
Assert.Equal(ActivityStatusCode.Error, activity.Status);
Assert.Equal("System.Exception", activity.GetTagValue(SemanticConventions.AttributeErrorType));
}
else
{

54
test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs
View File

@ -185,8 +185,10 @@ public class MetricTests
}
#endif
[Fact]
public async Task RequestMetricIsCaptured_New()
[Theory]
[InlineData("/api/values/2", "api/Values/{id}", null, 200)]
[InlineData("/api/Error", "api/Error", "System.Exception", 500)]
public async Task RequestMetricIsCaptured_New(string api, string expectedRoute, string expectedErrorType, int expectedStatusCode)
{
var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string> { [SemanticConventionOptInKeyName] = "http" })
@ -207,11 +209,15 @@ public class MetricTests
})
.CreateClient())
{
using var response1 = await client.GetAsync("/api/values").ConfigureAwait(false);
using var response2 = await client.GetAsync("/api/values/2").ConfigureAwait(false);
response1.EnsureSuccessStatusCode();
response2.EnsureSuccessStatusCode();
try
{
using var response = await client.GetAsync(api).ConfigureAwait(false);
response.EnsureSuccessStatusCode();
}
catch
{
// ignore error.
}
}
// We need to let End callback execute as it is executed AFTER response was returned.
@ -229,12 +235,14 @@ public class MetricTests
Assert.Equal("s", metric.Unit);
var metricPoints = GetMetricPoints(metric);
Assert.Equal(2, metricPoints.Count);
Assert.Single(metricPoints);
AssertMetricPoints_New(
metricPoints: metricPoints,
expectedRoutes: new List<string> { "api/Values", "api/Values/{id}" },
expectedTagsCount: 6);
expectedRoutes: new List<string> { expectedRoute },
expectedErrorType,
expectedStatusCode,
expectedTagsCount: expectedErrorType == null ? 6 : 7);
}
[Theory]
@ -430,6 +438,8 @@ public class MetricTests
AssertMetricPoints_New(
metricPoints: metricPoints,
expectedRoutes: new List<string> { "api/Values", "api/Values/{id}" },
null,
200,
expectedTagsCount: 6);
}
#endif
@ -456,6 +466,8 @@ public class MetricTests
private static void AssertMetricPoints_New(
List<MetricPoint> metricPoints,
List<string> expectedRoutes,
string expectedErrorType,
int expectedStatusCode,
int expectedTagsCount)
{
// Assert that one MetricPoint exists for each ExpectedRoute
@ -476,7 +488,7 @@ public class MetricTests
if (metricPoint.HasValue)
{
AssertMetricPoint_New(metricPoint.Value, expectedRoute, expectedTagsCount);
AssertMetricPoint_New(metricPoint.Value, expectedStatusCode, expectedRoute, expectedErrorType, expectedTagsCount);
}
else
{
@ -519,8 +531,10 @@ public class MetricTests
private static KeyValuePair<string, object>[] AssertMetricPoint_New(
MetricPoint metricPoint,
string expectedRoute = "api/Values",
int expectedTagsCount = StandardTagsCount)
int expectedStatusCode,
string expectedRoute,
string expectedErrorType,
int expectedTagsCount)
{
var count = metricPoint.GetHistogramCount();
var sum = metricPoint.GetHistogramSum();
@ -540,7 +554,7 @@ public class MetricTests
var method = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, "GET");
var scheme = new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, "http");
var statusCode = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, 200);
var statusCode = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, expectedStatusCode);
var flavor = new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, "1.1");
var route = new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRoute, expectedRoute);
Assert.Contains(method, attributes);
@ -549,6 +563,18 @@ public class MetricTests
Assert.Contains(flavor, attributes);
Assert.Contains(route, attributes);
if (expectedErrorType != null)
{
#if NET8_0_OR_GREATER
// Expected to change in next release
// https://github.com/dotnet/aspnetcore/issues/51029
var errorType = new KeyValuePair<string, object>("exception.type", expectedErrorType);
#else
var errorType = new KeyValuePair<string, object>(SemanticConventions.AttributeErrorType, expectedErrorType);
#endif
Assert.Contains(errorType, attributes);
}
// Inspect Histogram Bounds
var histogramBuckets = metricPoint.GetHistogramBuckets();
var histogramBounds = new List<double>();