Skip to content

Commit

Permalink
Replace http.error.reason with OTel standard error.type (#91910)
Browse files Browse the repository at this point in the history 
Align attribute name and semantics with the OpenTelemetry spec .
  • Loading branch information
@antonfirsov
antonfirsov authored Sep 13, 2023
1 parent 2984b87 commit 9695621
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 39 deletions.
77 changes: 50 additions & 27 deletions src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,12 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon
tags.Add("http.response.status_code", GetBoxedStatusCode((int)response.StatusCode));
tags.Add("network.protocol.version", GetProtocolVersionString(response.Version));
}
else

if (TryGetErrorType(response, exception, out string? errorType))
{
Debug.Assert(exception is not null);
tags.Add("http.error.reason", GetErrorReason(exception));
tags.Add("error.type", errorType);
}

TimeSpan durationTime = Stopwatch.GetElapsedTime(startTimestamp, Stopwatch.GetTimestamp());

HttpMetricsEnrichmentContext? enrichmentContext = HttpMetricsEnrichmentContext.GetEnrichmentContextForRequest(request);
Expand All @@ -130,37 +131,47 @@ private void RequestStop(HttpRequestMessage request, HttpResponseMessage? respon
}
}

private static string GetErrorReason(Exception exception)
private static bool TryGetErrorType(HttpResponseMessage? response, Exception? exception, out string? errorType)
{
if (exception is HttpRequestException e)
if (response is not null)
{
Debug.Assert(Enum.GetValues<HttpRequestError>().Length == 12, "We need to extend the mapping in case new values are added to HttpRequestError.");
int statusCode = (int)response.StatusCode;

string? errorReason = e.HttpRequestError switch
// In case the status code indicates a client or a server error, return the string representation of the status code.
// See the paragraph Status and the definition of 'error.type' in
// /~https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-spans.md
if (statusCode >= 400 && statusCode <= 599)
{
HttpRequestError.NameResolutionError => "name_resolution_error",
HttpRequestError.ConnectionError => "connection_error",
HttpRequestError.SecureConnectionError => "secure_connection_error",
HttpRequestError.HttpProtocolError => "http_protocol_error",
HttpRequestError.ExtendedConnectNotSupported => "extended_connect_not_supported",
HttpRequestError.VersionNegotiationError => "version_negotiation_error",
HttpRequestError.UserAuthenticationError => "user_authentication_error",
HttpRequestError.ProxyTunnelError => "proxy_tunnel_error",
HttpRequestError.InvalidResponse => "invalid_response",
HttpRequestError.ResponseEnded => "response_ended",
HttpRequestError.ConfigurationLimitExceeded => "configuration_limit_exceeded",

// Fall back to the exception type name (including for HttpRequestError.Unknown).
_ => null
};

if (errorReason is not null)
{
return errorReason;
errorType = GetErrorStatusCodeString(statusCode);
return true;
}
}

return exception.GetType().Name;
if (exception is null)
{
errorType = null;
return false;
}

Debug.Assert(Enum.GetValues<HttpRequestError>().Length == 12, "We need to extend the mapping in case new values are added to HttpRequestError.");
errorType = (exception as HttpRequestException)?.HttpRequestError switch
{
HttpRequestError.NameResolutionError => "name_resolution_error",
HttpRequestError.ConnectionError => "connection_error",
HttpRequestError.SecureConnectionError => "secure_connection_error",
HttpRequestError.HttpProtocolError => "http_protocol_error",
HttpRequestError.ExtendedConnectNotSupported => "extended_connect_not_supported",
HttpRequestError.VersionNegotiationError => "version_negotiation_error",
HttpRequestError.UserAuthenticationError => "user_authentication_error",
HttpRequestError.ProxyTunnelError => "proxy_tunnel_error",
HttpRequestError.InvalidResponse => "invalid_response",
HttpRequestError.ResponseEnded => "response_ended",
HttpRequestError.ConfigurationLimitExceeded => "configuration_limit_exceeded",

// Fall back to the exception type name in case of HttpRequestError.Unknown or when exception is not an HttpRequestException.
_ => exception.GetType().Name
};
return true;
}

private static string GetProtocolVersionString(Version httpVersion) => (httpVersion.Major, httpVersion.Minor) switch
Expand Down Expand Up @@ -199,6 +210,7 @@ private static TagList InitializeCommonTags(HttpRequestMessage request)
}

private static object[]? s_boxedStatusCodes;
private static string[]? s_statusCodeStrings;

private static object GetBoxedStatusCode(int statusCode)
{
Expand All @@ -209,6 +221,17 @@ private static object GetBoxedStatusCode(int statusCode)
: statusCode;
}

private static string GetErrorStatusCodeString(int statusCode)
{
Debug.Assert(statusCode >= 400 && statusCode <= 599);

string[] strings = LazyInitializer.EnsureInitialized(ref s_statusCodeStrings, static () => new string[200]);
int index = statusCode - 400;
return (uint)index < (uint)strings.Length
? strings[index] ??= statusCode.ToString()
: statusCode.ToString();
}

private sealed class SharedMeter : Meter
{
public static Meter Instance { get; } = new SharedMeter();
Expand Down
47 changes: 35 additions & 12 deletions src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ protected static void VerifyRequestDuration(Measurement<double> measurement,
Version? protocolVersion = null,
int? statusCode = null,
string method = "GET",
string[] acceptedErrorReasons = null) =>
VerifyRequestDuration(InstrumentNames.RequestDuration, measurement.Value, measurement.Tags.ToArray(), uri, protocolVersion, statusCode, method, acceptedErrorReasons);
string[] acceptedErrorTypes = null) =>
VerifyRequestDuration(InstrumentNames.RequestDuration, measurement.Value, measurement.Tags.ToArray(), uri, protocolVersion, statusCode, method, acceptedErrorTypes);

protected static void VerifyRequestDuration(string instrumentName,
double measurement,
Expand All @@ -85,22 +85,22 @@ protected static void VerifyRequestDuration(string instrumentName,
Version? protocolVersion,
int? statusCode,
string method = "GET",
string[] acceptedErrorReasons = null)
string[] acceptedErrorTypes = null)
{
Assert.Equal(InstrumentNames.RequestDuration, instrumentName);
Assert.InRange(measurement, double.Epsilon, 60);
VerifySchemeHostPortTags(tags, uri);
VerifyTag(tags, "http.request.method", method);
VerifyTag(tags, "network.protocol.version", GetVersionString(protocolVersion));
VerifyTag(tags, "http.response.status_code", statusCode);
if (acceptedErrorReasons == null)
if (acceptedErrorTypes == null)
{
Assert.DoesNotContain(tags, t => t.Key == "http.error.reason");
Assert.DoesNotContain(tags, t => t.Key == "error.type");
}
else
{
string errorReason = (string)tags.Single(t => t.Key == "http.error.reason").Value;
Assert.Contains(errorReason, acceptedErrorReasons);
string errorReason = (string)tags.Single(t => t.Key == "error.type").Value;
Assert.Contains(errorReason, acceptedErrorTypes);
}
}

Expand Down Expand Up @@ -659,7 +659,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
: [nameof(TaskCanceledException), nameof(OperationCanceledException)];

Measurement<double> m = Assert.Single(recorder.GetMeasurements());
VerifyRequestDuration(m, uri, acceptedErrorReasons: expectedExceptionTypes);
VerifyRequestDuration(m, uri, acceptedErrorTypes: expectedExceptionTypes);

clientCompleted.SetResult();
},
Expand Down Expand Up @@ -703,7 +703,7 @@ public async Task RequestDuration_ConnectionError_LogsExpectedErrorReason()
_output.WriteLine($"Client exception: {ex}");

Measurement<double> m = Assert.Single(recorder.GetMeasurements());
VerifyRequestDuration(m, uri, acceptedErrorReasons: ["connection_error"]);
VerifyRequestDuration(m, uri, acceptedErrorTypes: ["connection_error"]);
}

protected override void Dispose(bool disposing)
Expand Down Expand Up @@ -793,6 +793,29 @@ await Assert.ThrowsAsync<HttpRequestException>(async () =>
}, content: "x"));
}

[Theory]
[InlineData(400)]
[InlineData(404)]
[InlineData(599)]
public Task RequestDuration_ErrorStatus_ErrorTypeRecorded(int statusCode)
{
return LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
{
using HttpMessageInvoker client = CreateHttpMessageInvoker();
using InstrumentRecorder<double> recorder = SetupInstrumentRecorder<double>(InstrumentNames.RequestDuration);
using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion };

using HttpResponseMessage response = await SendAsync(client, request);

Measurement<double> m = Assert.Single(recorder.GetMeasurements());
VerifyRequestDuration(m, uri, UseVersion, statusCode, "GET", acceptedErrorTypes: new[] { $"{statusCode}" });

}, async server =>
{
await server.AcceptConnectionSendResponseAndCloseAsync(statusCode: (HttpStatusCode)statusCode);
});
}

[Fact]
[SkipOnPlatform(TestPlatforms.Browser, "Browser is relaxed about validating HTTP headers")]
public async Task RequestDuration_ConnectionClosedWhileReceivingHeaders_Recorded()
Expand All @@ -814,7 +837,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
Assert.True(ex is HttpRequestException or TaskCanceledException);

Measurement<double> m = Assert.Single(recorder.GetMeasurements());
VerifyRequestDuration(m, uri, acceptedErrorReasons: [nameof(TaskCanceledException), "response_ended"]);
VerifyRequestDuration(m, uri, acceptedErrorTypes: [nameof(TaskCanceledException), "response_ended"]);
}, async server =>
{
try
Expand Down Expand Up @@ -869,7 +892,7 @@ await server.AcceptConnectionAsync(async connection =>
{
await Assert.ThrowsAsync<HttpRequestException>(() => clientTask);
Measurement<double> m = Assert.Single(recorder.GetMeasurements());
VerifyRequestDuration(m, server.Address, acceptedErrorReasons: ["response_ended"]);
VerifyRequestDuration(m, server.Address, acceptedErrorTypes: ["response_ended"]);
}
else
{
Expand Down Expand Up @@ -967,7 +990,7 @@ await Assert.ThrowsAsync<HttpRequestException>(async () =>
});

Measurement<double> m = Assert.Single(recorder.GetMeasurements());
VerifyRequestDuration(m, server.Address, acceptedErrorReasons: ["http_protocol_error"]);
VerifyRequestDuration(m, server.Address, acceptedErrorTypes: ["http_protocol_error"]);
}
}

Expand Down

0 comments on commit 9695621

Please sign in to comment.