[HttpClient & HttpWebRequest] Set `http.request.method` as per spec (#5003)

This commit is contained in:
Vishwesh Bankwar 2023-10-31 18:16:23 -07:00 committed by GitHub
parent 5cb7a3fca9
commit d1d6d4b720
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 224 additions and 43 deletions

View File

@ -252,13 +252,15 @@ internal class HttpInListener : ListenerHandler
activity.SetTag(SemanticConventions.AttributeUrlQuery, request.QueryString.Value);
}
if (RequestMethodHelper.TryResolveHttpMethod(request.Method, out var httpMethod))
if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod))
{
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
}
else
{
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, "_OTHER");
activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method);
}

View File

@ -14,7 +14,6 @@
// limitations under the License.
// </copyright>
#if !NET8_0_OR_GREATER
using System.Diagnostics;
using System.Diagnostics.Metrics;
using Microsoft.AspNetCore.Http;
@ -153,8 +152,16 @@ internal sealed class HttpInMetricsListener : ListenerHandler
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, context.Request.Scheme));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode)));
RequestMethodHelper.TryResolveHttpMethod(context.Request.Method, out var httpMethod);
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
if (RequestMethodHelper.KnownMethods.TryGetValue(context.Request.Method, out var httpMethod))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
}
else
{
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, "_OTHER"));
}
#if NET6_0_OR_GREATER
var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
@ -170,4 +177,3 @@ internal sealed class HttpInMetricsListener : ListenerHandler
this.httpServerRequestDuration.Record(Activity.Current.Duration.TotalSeconds, tags);
}
}
#endif

View File

@ -2,6 +2,22 @@
## Unreleased
* Updated `http.request.method` to match specification guidelines.
* For activity, if the method does not belong to one of the [known
values](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#:~:text=http.request.method%20has%20the%20following%20list%20of%20well%2Dknown%20values)
then the request method will be set on an additional tag
`http.request.method.original` and `http.request.method` will be set to
`_OTHER`.
* For metrics, if the original method does not belong to one of the [known
values](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#:~:text=http.request.method%20has%20the%20following%20list%20of%20well%2Dknown%20values)
then `http.request.method` on `http.client.request.duration` metric will be
set to `_OTHER`
`http.request.method` is set on `http.client.request.duration` metric or
activity when `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable is set to
`http` or `http/dup`.
([#5003](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5003))
## 1.6.0-beta.2
Released 2023-Oct-26

View File

@ -23,6 +23,7 @@ using System.Net.Http;
#endif
using System.Reflection;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Internal;
using OpenTelemetry.Trace;
using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
@ -185,7 +186,18 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler
// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md
if (this.emitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, HttpTagHelper.GetNameForHttpMethod(request.Method));
if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.Method, out var httpMethod))
{
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
}
else
{
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, "_OTHER");
activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method.Method);
}
activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
if (!request.RequestUri.IsDefaultPort)
{

View File

@ -23,6 +23,7 @@ using System.Diagnostics.Metrics;
using System.Net.Http;
#endif
using System.Reflection;
using OpenTelemetry.Internal;
using OpenTelemetry.Trace;
using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
@ -97,7 +98,17 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler
{
TagList tags = default;
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, HttpTagHelper.GetNameForHttpMethod(request.Method)));
if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.Method, out var httpMethod))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
}
else
{
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, "_OTHER"));
}
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerAddress, request.RequestUri.Host));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme));

View File

@ -23,6 +23,7 @@ using System.Reflection;
using System.Reflection.Emit;
using System.Runtime.CompilerServices;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Internal;
using OpenTelemetry.Trace;
using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
@ -153,7 +154,18 @@ internal static class HttpWebRequestActivitySource
// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md
if (tracingEmitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, request.Method);
if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod))
{
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
}
else
{
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, "_OTHER");
activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method);
}
activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
if (!request.RequestUri.IsDefaultPort)
{
@ -495,7 +507,17 @@ internal static class HttpWebRequestActivitySource
{
TagList tags = default;
tags.Add(SemanticConventions.AttributeHttpRequestMethod, request.Method);
if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
}
else
{
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, "_OTHER"));
}
tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme);
tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));

View File

@ -45,8 +45,9 @@ public static class MeterProviderBuilderExtensions
.AddMeter("System.Net.Http")
.AddMeter("System.Net.NameResolution");
#else
// Note: Warm-up the status code mapping.
// Note: Warm-up the status code and method mapping.
_ = TelemetryHelper.BoxedStatusCodes;
_ = RequestMethodHelper.KnownMethods;
builder.ConfigureServices(services =>
{

View File

@ -16,6 +16,7 @@
<Compile Include="$(RepoRoot)\src\Shared\HttpSemanticConventionHelper.cs" Link="Includes\HttpSemanticConventionHelper.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
<Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
<Compile Include="$(RepoRoot)\src\Shared\RequestMethodHelper.cs" Link="Includes\RequestMethodHelper.cs" />
</ItemGroup>
<ItemGroup Condition="'$(RunningDotNetPack)' != 'true'">

View File

@ -60,8 +60,9 @@ public static class TracerProviderBuilderExtensions
{
Guard.ThrowIfNull(builder);
// Note: Warm-up the status code mapping.
// Note: Warm-up the status code and method mapping.
_ = TelemetryHelper.BoxedStatusCodes;
_ = RequestMethodHelper.KnownMethods;
name ??= Options.DefaultName;

View File

@ -30,23 +30,7 @@ internal static class RequestMethodHelper
static RequestMethodHelper()
{
#if NET8_0_OR_GREATER
KnownMethods = FrozenDictionary.ToFrozenDictionary(
new[]
{
KeyValuePair.Create("GET", "GET"),
KeyValuePair.Create("PUT", "PUT"),
KeyValuePair.Create("POST", "POST"),
KeyValuePair.Create("DELETE", "DELETE"),
KeyValuePair.Create("HEAD", "HEAD"),
KeyValuePair.Create("OPTIONS", "OPTIONS"),
KeyValuePair.Create("TRACE", "TRACE"),
KeyValuePair.Create("PATCH", "PATCH"),
KeyValuePair.Create("CONNECT", "CONNECT"),
},
StringComparer.OrdinalIgnoreCase);
#else
KnownMethods = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
var knownMethodSet = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
{ "GET", "GET" },
{ "PUT", "PUT" },
@ -58,20 +42,12 @@ internal static class RequestMethodHelper
{ "PATCH", "PATCH" },
{ "CONNECT", "CONNECT" },
};
// KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case.
#if NET8_0_OR_GREATER
KnownMethods = FrozenDictionary.ToFrozenDictionary(knownMethodSet, StringComparer.OrdinalIgnoreCase);
#else
KnownMethods = knownMethodSet;
#endif
}
public static bool TryResolveHttpMethod(string method, out string resolvedMethod)
{
if (KnownMethods.TryGetValue(method, out resolvedMethod))
{
// KnownMethods ignores case. Use the value returned by the dictionary to have a consistent case.
return true;
}
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
resolvedMethod = "_OTHER";
return false;
}
}

View File

@ -15,6 +15,7 @@
// </copyright>
using System.Diagnostics;
using Microsoft.Extensions.Configuration;
#if NETFRAMEWORK
using System.Net;
using System.Net.Http;
@ -23,11 +24,14 @@ using Microsoft.Extensions.DependencyInjection;
using Moq;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Instrumentation.Http.Implementation;
using OpenTelemetry.Metrics;
using OpenTelemetry.Tests;
using OpenTelemetry.Trace;
using Xunit;
using Xunit.Abstractions;
using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
namespace OpenTelemetry.Instrumentation.Http.Tests;
public partial class HttpClientTests : IDisposable
@ -368,6 +372,135 @@ public partial class HttpClientTests : IDisposable
Assert.NotEqual(spanid2, spanid3);
}
[Theory]
[InlineData("CONNECT", "CONNECT")]
[InlineData("DELETE", "DELETE")]
[InlineData("GET", "GET")]
[InlineData("PUT", "PUT")]
[InlineData("HEAD", "HEAD")]
[InlineData("OPTIONS", "OPTIONS")]
[InlineData("PATCH", "PATCH")]
[InlineData("Get", "GET")]
[InlineData("POST", "POST")]
[InlineData("TRACE", "TRACE")]
[InlineData("CUSTOM", "_OTHER")]
public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string originalMethod, string expectedMethod)
{
var exportedItems = new List<Activity>();
using var request = new HttpRequestMessage
{
RequestUri = new Uri(this.url),
Method = new HttpMethod(originalMethod),
};
var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string> { [SemanticConventionOptInKeyName] = "http" })
.Build();
using var traceprovider = Sdk.CreateTracerProviderBuilder()
.ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration))
.AddHttpClientInstrumentation()
.AddInMemoryExporter(exportedItems)
.Build();
using var httpClient = new HttpClient();
try
{
await httpClient.SendAsync(request).ConfigureAwait(false);
}
catch
{
// ignore error.
}
Assert.Single(exportedItems);
var activity = exportedItems[0];
Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRequestMethod);
if (originalMethod.Equals(expectedMethod, StringComparison.OrdinalIgnoreCase))
{
Assert.DoesNotContain(activity.TagObjects, t => t.Key == SemanticConventions.AttributeHttpRequestMethodOriginal);
}
else
{
Assert.Equal(originalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string);
}
Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string);
}
[Theory]
[InlineData("CONNECT", "CONNECT")]
[InlineData("DELETE", "DELETE")]
[InlineData("GET", "GET")]
[InlineData("PUT", "PUT")]
[InlineData("HEAD", "HEAD")]
[InlineData("OPTIONS", "OPTIONS")]
[InlineData("PATCH", "PATCH")]
[InlineData("Get", "GET")]
[InlineData("POST", "POST")]
[InlineData("TRACE", "TRACE")]
[InlineData("CUSTOM", "_OTHER")]
public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string originalMethod, string expectedMethod)
{
var metricItems = new List<Metric>();
using var request = new HttpRequestMessage
{
RequestUri = new Uri(this.url),
Method = new HttpMethod(originalMethod),
};
var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string> { [SemanticConventionOptInKeyName] = "http" })
.Build();
using var meterprovider = Sdk.CreateMeterProviderBuilder()
.ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration))
.AddHttpClientInstrumentation()
.AddInMemoryExporter(metricItems)
.Build();
using var httpClient = new HttpClient();
try
{
await httpClient.SendAsync(request).ConfigureAwait(false);
}
catch
{
// ignore error.
}
meterprovider.Dispose();
var metric = metricItems.FirstOrDefault(m => m.Name == "http.client.request.duration");
Assert.NotNull(metric);
var metricPoints = new List<MetricPoint>();
foreach (var p in metric.GetMetricPoints())
{
metricPoints.Add(p);
}
Assert.Single(metricPoints);
var mp = metricPoints[0];
// Inspect Metric Attributes
var attributes = new Dictionary<string, object>();
foreach (var tag in mp.Tags)
{
attributes[tag.Key] = tag.Value;
}
Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == expectedMethod);
Assert.DoesNotContain(attributes, t => t.Key == SemanticConventions.AttributeHttpRequestMethodOriginal);
}
[Fact]
public async Task RedirectTest()
{