diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs index 7c2879b29d3..53e4f8f17ec 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs @@ -18,7 +18,6 @@ using System.Diagnostics.Metrics; using System.Reflection; using OpenTelemetry.Instrumentation.AspNetCore.Implementation; -using OpenTelemetry.Internal; namespace OpenTelemetry.Instrumentation.AspNetCore; @@ -46,11 +45,10 @@ internal sealed class AspNetCoreMetrics : IDisposable private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; private readonly Meter meter; - internal AspNetCoreMetrics(AspNetCoreMetricsInstrumentationOptions options) + internal AspNetCoreMetrics() { - Guard.ThrowIfNull(options); this.meter = new Meter(InstrumentationName, InstrumentationVersion); - var metricsListener = new HttpInMetricsListener("Microsoft.AspNetCore", this.meter, options); + var metricsListener = new HttpInMetricsListener("Microsoft.AspNetCore", this.meter); this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(metricsListener, this.isEnabled, AspNetCoreInstrumentationEventSource.Log.UnknownErrorProcessingEvent); this.diagnosticSourceSubscriber.Subscribe(); } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs deleted file mode 100644 index 2ec3ff8a92c..00000000000 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs +++ /dev/null @@ -1,45 +0,0 @@ -// -// 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. -// - -using System.Diagnostics; -using Microsoft.Extensions.Configuration; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; - -namespace OpenTelemetry.Instrumentation.AspNetCore; - -/// -/// Options for metrics requests instrumentation. -/// -internal sealed class AspNetCoreMetricsInstrumentationOptions -{ - internal readonly HttpSemanticConvention HttpSemanticConvention; - - /// - /// Initializes a new instance of the class. - /// - public AspNetCoreMetricsInstrumentationOptions() - : this(new ConfigurationBuilder().AddEnvironmentVariables().Build()) - { - } - - internal AspNetCoreMetricsInstrumentationOptions(IConfiguration configuration) - { - Debug.Assert(configuration != null, "configuration was null"); - - this.HttpSemanticConvention = GetSemanticConventionOptIn(configuration); - } -} - diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 3d9b09a6a59..67fa080bf8c 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Removed support for `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. The + library will now emit only the + [stable](https://github.com/open-telemetry/semantic-conventions/tree/v1.23.0/docs/http) + semantic conventions. + ([#5066](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5066)) + ## 1.6.0-beta.3 Released 2023-Nov-17 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 0542e146b58..f0ab3611c4a 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -32,7 +32,6 @@ #endif using OpenTelemetry.Internal; using OpenTelemetry.Trace; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation; @@ -66,8 +65,6 @@ internal class HttpInListener : ListenerHandler private readonly PropertyFetcher beforeActionTemplateFetcher = new("Template"); #endif private readonly AspNetCoreInstrumentationOptions options; - private readonly bool emitOldAttributes; - private readonly bool emitNewAttributes; public HttpInListener(AspNetCoreInstrumentationOptions options) : base(DiagnosticSourceName) @@ -75,10 +72,6 @@ public HttpInListener(AspNetCoreInstrumentationOptions options) Guard.ThrowIfNull(options); this.options = options; - - this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - - this.emitNewAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); } public override void OnEventWritten(string name, object payload) @@ -197,67 +190,36 @@ public void OnStartActivity(Activity activity, object payload) var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/"; activity.DisplayName = this.GetDisplayName(request.Method); - // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md - if (this.emitOldAttributes) - { - if (request.Host.HasValue) - { - activity.SetTag(SemanticConventions.AttributeNetHostName, request.Host.Host); - - if (request.Host.Port is not null && request.Host.Port != 80 && request.Host.Port != 443) - { - activity.SetTag(SemanticConventions.AttributeNetHostPort, request.Host.Port); - } - } + // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md - activity.SetTag(SemanticConventions.AttributeHttpMethod, request.Method); - activity.SetTag(SemanticConventions.AttributeHttpScheme, request.Scheme); - activity.SetTag(SemanticConventions.AttributeHttpTarget, path); - activity.SetTag(SemanticConventions.AttributeHttpUrl, GetUri(request)); - activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(request.Protocol)); + if (request.Host.HasValue) + { + activity.SetTag(SemanticConventions.AttributeServerAddress, request.Host.Host); - if (request.Headers.TryGetValue("User-Agent", out var values)) + if (request.Host.Port is not null && request.Host.Port != 80 && request.Host.Port != 443) { - var userAgent = values.Count > 0 ? values[0] : null; - if (!string.IsNullOrEmpty(userAgent)) - { - activity.SetTag(SemanticConventions.AttributeHttpUserAgent, userAgent); - } + activity.SetTag(SemanticConventions.AttributeServerPort, request.Host.Port); } } - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md - if (this.emitNewAttributes) + if (request.QueryString.HasValue) { - if (request.Host.HasValue) - { - activity.SetTag(SemanticConventions.AttributeServerAddress, request.Host.Host); - - if (request.Host.Port is not null && request.Host.Port != 80 && request.Host.Port != 443) - { - activity.SetTag(SemanticConventions.AttributeServerPort, request.Host.Port); - } - } - - if (request.QueryString.HasValue) - { - // QueryString should be sanitized. see: https://github.com/open-telemetry/opentelemetry-dotnet/issues/4571 - activity.SetTag(SemanticConventions.AttributeUrlQuery, request.QueryString.Value); - } + // QueryString should be sanitized. see: https://github.com/open-telemetry/opentelemetry-dotnet/issues/4571 + activity.SetTag(SemanticConventions.AttributeUrlQuery, request.QueryString.Value); + } - RequestMethodHelper.SetHttpMethodTag(activity, request.Method); + RequestMethodHelper.SetHttpMethodTag(activity, request.Method); - activity.SetTag(SemanticConventions.AttributeUrlScheme, request.Scheme); - activity.SetTag(SemanticConventions.AttributeUrlPath, path); - activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocol(request.Protocol)); + activity.SetTag(SemanticConventions.AttributeUrlScheme, request.Scheme); + activity.SetTag(SemanticConventions.AttributeUrlPath, path); + activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocol(request.Protocol)); - if (request.Headers.TryGetValue("User-Agent", out var values)) + if (request.Headers.TryGetValue("User-Agent", out var values)) + { + var userAgent = values.Count > 0 ? values[0] : null; + if (!string.IsNullOrEmpty(userAgent)) { - var userAgent = values.Count > 0 ? values[0] : null; - if (!string.IsNullOrEmpty(userAgent)) - { - activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, userAgent); - } + activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, userAgent); } } @@ -294,15 +256,7 @@ public void OnStopActivity(Activity activity, object payload) } #endif - if (this.emitOldAttributes) - { - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); - } - - if (this.emitNewAttributes) - { - activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); - } + activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); #if !NETSTANDARD2_0 if (this.options.EnableGrpcAspNetCoreSupport && TryGetGrpcMethod(activity, out var grpcMethod)) @@ -366,10 +320,7 @@ public void OnException(Activity activity, object payload) return; } - if (this.emitNewAttributes) - { - activity.SetTag(SemanticConventions.AttributeErrorType, exc.GetType().FullName); - } + activity.SetTag(SemanticConventions.AttributeErrorType, exc.GetType().FullName); if (this.options.RecordException) { @@ -454,9 +405,7 @@ private static bool TryGetGrpcMethod(Activity activity, out string grpcMethod) private string GetDisplayName(string httpMethod, string httpRoute = null) { - var normalizedMethod = this.emitNewAttributes - ? RequestMethodHelper.GetNormalizedHttpMethod(httpMethod) - : httpMethod; + var normalizedMethod = RequestMethodHelper.GetNormalizedHttpMethod(httpMethod); return string.IsNullOrEmpty(httpRoute) ? normalizedMethod @@ -474,28 +423,15 @@ private void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc); - if (this.emitOldAttributes) - { - if (context.Connection.RemoteIpAddress != null) - { - // TODO: This attribute was changed in v1.13.0 https://github.com/open-telemetry/opentelemetry-specification/pull/2614 - activity.SetTag(SemanticConventions.AttributeNetPeerIp, context.Connection.RemoteIpAddress.ToString()); - } - - activity.SetTag(SemanticConventions.AttributeNetPeerPort, context.Connection.RemotePort); - } + // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/rpc/rpc-spans.md - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/rpc/rpc-spans.md - if (this.emitNewAttributes) + if (context.Connection.RemoteIpAddress != null) { - if (context.Connection.RemoteIpAddress != null) - { - activity.SetTag(SemanticConventions.AttributeClientAddress, context.Connection.RemoteIpAddress.ToString()); - } - - activity.SetTag(SemanticConventions.AttributeClientPort, context.Connection.RemotePort); + activity.SetTag(SemanticConventions.AttributeClientAddress, context.Connection.RemoteIpAddress.ToString()); } + activity.SetTag(SemanticConventions.AttributeClientPort, context.Connection.RemotePort); + bool validConversion = GrpcTagHelper.TryGetGrpcStatusCodeFromActivity(activity, out int status); if (validConversion) { diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs index b442a429a2d..a47ee19858e 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs @@ -24,13 +24,11 @@ using Microsoft.AspNetCore.Routing; #endif using OpenTelemetry.Trace; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation; 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"; @@ -43,31 +41,13 @@ internal sealed class HttpInMetricsListener : ListenerHandler private static readonly object ErrorTypeHttpContextItemsKey = new(); private readonly Meter meter; - private readonly AspNetCoreMetricsInstrumentationOptions options; - private readonly Histogram httpServerDuration; private readonly Histogram httpServerRequestDuration; - private readonly bool emitOldAttributes; - private readonly bool emitNewAttributes; - internal HttpInMetricsListener(string name, Meter meter, AspNetCoreMetricsInstrumentationOptions options) + internal HttpInMetricsListener(string name, Meter meter) : base(name) { this.meter = meter; - this.options = options; - - this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - - this.emitNewAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); - - if (this.emitOldAttributes) - { - this.httpServerDuration = meter.CreateHistogram(HttpServerDurationMetricName, "ms", "Measures the duration of inbound HTTP requests."); - } - - if (this.emitNewAttributes) - { - this.httpServerRequestDuration = meter.CreateHistogram(HttpServerRequestDurationMetricName, "s", "Duration of HTTP server requests."); - } + this.httpServerRequestDuration = meter.CreateHistogram(HttpServerRequestDurationMetricName, "s", "Duration of HTTP server requests."); } public override void OnEventWritten(string name, object payload) @@ -77,24 +57,13 @@ public override void OnEventWritten(string name, object payload) case OnUnhandledDiagnosticsExceptionEvent: case OnUnhandledHostingExceptionEvent: { - if (this.emitNewAttributes) - { - this.OnExceptionEventWritten(name, payload); - } + this.OnExceptionEventWritten(name, payload); } break; case OnStopEvent: { - if (this.emitOldAttributes) - { - this.OnEventWritten_Old(name, payload); - } - - if (this.emitNewAttributes) - { - this.OnEventWritten_New(name, payload); - } + this.OnStopEventWritten(name, payload); } break; @@ -106,7 +75,7 @@ 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); + AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(this.OnExceptionEventWritten), HttpServerRequestDurationMetricName); return; } @@ -127,56 +96,7 @@ 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); - 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). - // 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; - } - - TagList tags = default; - - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol))); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpScheme, context.Request.Scheme)); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpMethod, context.Request.Method)); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode))); - - if (context.Request.Host.HasValue) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeNetHostName, context.Request.Host.Host)); - - if (context.Request.Host.Port is not null && context.Request.Host.Port != 80 && context.Request.Host.Port != 443) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeNetHostPort, context.Request.Host.Port)); - } - } - -#if NET6_0_OR_GREATER - var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; - if (!string.IsNullOrEmpty(route)) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRoute, route)); - } -#endif - - // 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 - // TODO: Follow up with .NET team if we can continue to rely on this behavior. - this.httpServerDuration.Record(Activity.Current.Duration.TotalMilliseconds, tags); - } - - public void OnEventWritten_New(string name, object payload) + public void OnStopEventWritten(string name, object payload) { var context = payload as HttpContext; if (context == null) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs index 6118642b2ed..c394a3590ee 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs @@ -15,8 +15,6 @@ // #if !NET8_0_OR_GREATER -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; using OpenTelemetry.Instrumentation.AspNetCore; using OpenTelemetry.Instrumentation.AspNetCore.Implementation; #endif @@ -46,22 +44,9 @@ public static MeterProviderBuilder AddAspNetCoreInstrumentation( _ = TelemetryHelper.BoxedStatusCodes; _ = RequestMethodHelper.KnownMethods; - builder.ConfigureServices(services => - { - services.RegisterOptionsFactory(configuration => new AspNetCoreMetricsInstrumentationOptions(configuration)); - }); - builder.AddMeter(AspNetCoreMetrics.InstrumentationName); - builder.AddInstrumentation(sp => - { - var options = sp.GetRequiredService>().Get(Options.DefaultName); - - // TODO: Add additional options to AspNetCoreMetricsInstrumentationOptions ? - // RecordException - probably doesn't make sense for metric instrumentation - // EnableGrpcAspNetCoreSupport - this instrumentation will also need to also handle gRPC requests - return new AspNetCoreMetrics(options); - }); + builder.AddInstrumentation(new AspNetCoreMetrics()); return builder; #endif diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md index c62dd0050d6..5f32153a444 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md @@ -90,31 +90,51 @@ public void ConfigureServices(IServiceCollection services) #### List of metrics produced -A different metric is emitted depending on whether a user opts-in to the new -Http Semantic Conventions using `OTEL_SEMCONV_STABILITY_OPT_IN`. - -* By default, the instrumentation emits the following metric. - - | Name | Instrument Type | Unit | Description | Attributes | - |-------|-----------------|------|-------------|------------| - | `http.server.duration` | Histogram | `ms` | Measures the duration of inbound HTTP requests. | http.flavor, http.scheme, http.method, http.status_code, net.host.name, net.host.port, http.route | - -* If user sets the environment variable to `http`, the instrumentation emits - the following metric. - - | Name | Instrument Type | Unit | Description | Attributes | - |-------|-----------------|------|-------------|------------| - | `http.server.request.duration` | Histogram | `s` | Measures the duration of inbound HTTP requests. | network.protocol.version, url.scheme, http.request.method, http.response.status_code, http.route | - - This metric is emitted in `seconds` as per the semantic convention. While - the convention [recommends using custom histogram buckets](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md) - , this feature is not yet available via .NET Metrics API. - A [workaround](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4820) +When the application targets `.NET6.0` or `.NET7.0`, the instrumentation emits +the following metric: + +| Name | Details | +|-----------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------| +| `http.server.request.duration` | [Specification](https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md#metric-httpserverrequestduration) | + +Starting from `.NET8.0`, metrics instrumentation is natively implemented, and +the ASP.NET Core library has incorporated support for [built-in +metrics](https://learn.microsoft.com/dotnet/core/diagnostics/built-in-metrics-aspnetcore) +following the OpenTelemetry semantic conventions. The library includes additional +metrics beyond those defined in the +[specification](https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md), +covering additional scenarios for ASP.NET Core users. When the application +targets `.NET8.0` and newer versions, the instrumentation library automatically +enables all `built-in` metrics by default. + +Note that the `AddAspNetCoreInstrumentation()` extension simplifies the process +of enabling all built-in metrics via a single line of code. Alternatively, for +more granular control over emitted metrics, you can utilize the `AddMeter()` +extension on `MeterProviderBuilder` for meters listed in +[built-in-metrics-aspnetcore](https://learn.microsoft.com/dotnet/core/diagnostics/built-in-metrics-aspnetcore). +Using `AddMeter()` for metrics activation eliminates the need to take dependency +on the instrumentation library package and calling +`AddAspNetCoreInstrumentation()`. + +If you utilize `AddAspNetCoreInstrumentation()` and wish to exclude unnecessary +metrics, you can utilize +[Views](https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/metrics/customizing-the-sdk#drop-an-instrument) +to achieve this. + +**Note:** There is no difference in features or emitted metrics when enabling +metrics using `AddMeter()` or `AddAspNetCoreInstrumentation()` on `.NET8.0` and +newer versions. + +> **Note** +> The `http.server.request.duration` metric is emitted in `seconds` as + per the semantic convention. While the convention [recommends using custom + histogram + buckets](https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md) + , this feature is not yet available via .NET Metrics API. A + [workaround](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4820) has been included in OTel SDK starting version `1.6.0` which applies - recommended buckets by default for `http.server.request.duration`. - -* If user sets the environment variable to `http/dup`, the instrumentation - emits both `http.server.duration` and `http.server.request.duration`. + recommended buckets by default for `http.server.request.duration`. This + applies to all targeted frameworks. ## Advanced configuration diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 21cd9be57af..c54b2eefd09 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Removed support for `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. The + library will now emit only the + [stable](https://github.com/open-telemetry/semantic-conventions/tree/v1.23.0/docs/http) + semantic conventions. + ([#5068](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5068)) + ## 1.6.0-beta.3 Released 2023-Nov-17 diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientMetricInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientMetricInstrumentationOptions.cs deleted file mode 100644 index a35b615eec6..00000000000 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientMetricInstrumentationOptions.cs +++ /dev/null @@ -1,41 +0,0 @@ -// -// 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. -// - -using System.Diagnostics; -using Microsoft.Extensions.Configuration; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; - -namespace OpenTelemetry.Instrumentation.Http; - -internal sealed class HttpClientMetricInstrumentationOptions -{ - internal readonly HttpSemanticConvention HttpSemanticConvention; - - /// - /// Initializes a new instance of the class. - /// - public HttpClientMetricInstrumentationOptions() - : this(new ConfigurationBuilder().AddEnvironmentVariables().Build()) - { - } - - internal HttpClientMetricInstrumentationOptions(IConfiguration configuration) - { - Debug.Assert(configuration != null, "configuration was null"); - - this.HttpSemanticConvention = GetSemanticConventionOptIn(configuration); - } -} diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs index 2f4db9ec9b2..6472ab91cff 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientMetrics.cs @@ -37,11 +37,10 @@ internal sealed class HttpClientMetrics : IDisposable /// /// Initializes a new instance of the class. /// - /// HttpClient metric instrumentation options. - public HttpClientMetrics(HttpClientMetricInstrumentationOptions options) + public HttpClientMetrics() { this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( - new HttpHandlerMetricsDiagnosticListener("HttpHandlerDiagnosticListener", options), + new HttpHandlerMetricsDiagnosticListener("HttpHandlerDiagnosticListener"), this.isEnabled, HttpInstrumentationEventSource.Log.UnknownErrorProcessingEvent); this.diagnosticSourceSubscriber.Subscribe(); diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index f5fb182211e..68e08949228 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -25,7 +25,6 @@ using OpenTelemetry.Context.Propagation; using OpenTelemetry.Internal; using OpenTelemetry.Trace; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.Http.Implementation; @@ -49,8 +48,6 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler private static readonly PropertyFetcher StopExceptionFetcher = new("Exception"); private static readonly PropertyFetcher StopRequestStatusFetcher = new("RequestTaskStatus"); private readonly HttpClientInstrumentationOptions options; - private readonly bool emitOldAttributes; - private readonly bool emitNewAttributes; static HttpHandlerDiagnosticListener() { @@ -68,10 +65,6 @@ public HttpHandlerDiagnosticListener(HttpClientInstrumentationOptions options) : base("HttpHandlerDiagnosticListener") { this.options = options; - - this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - - this.emitNewAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); } public override void OnEventWritten(string name, object payload) @@ -168,51 +161,33 @@ public void OnStartActivity(Activity activity, object payload) ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client); } - // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md - if (this.emitOldAttributes) + // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md + if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.Method, out var httpMethod)) { - activity.SetTag(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme); - activity.SetTag(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method)); - activity.SetTag(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host); - if (!request.RequestUri.IsDefaultPort) - { - activity.SetTag(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port); - } - - activity.SetTag(SemanticConventions.AttributeHttpUrl, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); - activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version)); + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); } - - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md - if (this.emitNewAttributes) + else { - 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); - } + // 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) - { - activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port); - } + activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); + if (!request.RequestUri.IsDefaultPort) + { + activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port); + } - activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); + activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); - if (request.Headers.TryGetValues("User-Agent", out var userAgentValues)) + if (request.Headers.TryGetValues("User-Agent", out var userAgentValues)) + { + var userAgent = userAgentValues.FirstOrDefault(); + if (!string.IsNullOrEmpty(userAgent)) { - var userAgent = userAgentValues.FirstOrDefault(); - if (!string.IsNullOrEmpty(userAgent)) - { - activity.SetTag(SemanticConventions.AttributeHttpUserAgent, userAgent); - } + activity.SetTag(SemanticConventions.AttributeHttpUserAgent, userAgent); } } @@ -275,19 +250,11 @@ public void OnStopActivity(Activity activity, object payload) activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode)); } - if (this.emitOldAttributes) - { - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); - } - - if (this.emitNewAttributes) + activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.Version)); + activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); + if (activity.Status == ActivityStatusCode.Error) { - activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.Version)); - activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); - if (activity.Status == ActivityStatusCode.Error) - { - activity.SetTag(SemanticConventions.AttributeErrorType, TelemetryHelper.GetStatusCodeString(response.StatusCode)); - } + activity.SetTag(SemanticConventions.AttributeErrorType, TelemetryHelper.GetStatusCodeString(response.StatusCode)); } try @@ -341,10 +308,7 @@ public void OnException(Activity activity, object payload) return; } - if (this.emitNewAttributes) - { - activity.SetTag(SemanticConventions.AttributeErrorType, GetErrorType(exc)); - } + activity.SetTag(SemanticConventions.AttributeErrorType, GetErrorType(exc)); if (this.options.RecordException) { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index 82bdc004ef3..b3d45af15e8 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -25,7 +25,6 @@ using System.Reflection; using OpenTelemetry.Internal; using OpenTelemetry.Trace; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.Http.Implementation; @@ -38,7 +37,6 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler internal static readonly string MeterVersion = AssemblyName.Version.ToString(); internal static readonly Meter Meter = new(MeterName, MeterVersion); private const string OnUnhandledExceptionEvent = "System.Net.Http.Exception"; - private static readonly Histogram HttpClientDuration = Meter.CreateHistogram("http.client.duration", "ms", "Measures the duration of outbound HTTP requests."); private static readonly Histogram HttpClientRequestDuration = Meter.CreateHistogram("http.client.request.duration", "s", "Duration of HTTP client requests."); private static readonly PropertyFetcher StopRequestFetcher = new("Request"); @@ -49,27 +47,16 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler private static readonly HttpRequestOptionsKey HttpRequestOptionsErrorKey = new HttpRequestOptionsKey(SemanticConventions.AttributeErrorType); #endif - private readonly HttpClientMetricInstrumentationOptions options; - private readonly bool emitOldAttributes; - private readonly bool emitNewAttributes; - - public HttpHandlerMetricsDiagnosticListener(string name, HttpClientMetricInstrumentationOptions options) + public HttpHandlerMetricsDiagnosticListener(string name) : base(name) { - this.options = options; - - this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - this.emitNewAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); } public override void OnEventWritten(string name, object payload) { if (name == OnUnhandledExceptionEvent) { - if (this.emitNewAttributes) - { - this.OnExceptionEventWritten(Activity.Current, payload); - } + this.OnExceptionEventWritten(Activity.Current, payload); } else if (name == OnStopEvent) { @@ -86,90 +73,61 @@ public void OnStopEventWritten(Activity activity, object payload) if (TryFetchRequest(payload, out HttpRequestMessage request)) { - // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md - if (this.emitOldAttributes) - { - TagList tags = default; - - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method))); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme)); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version))); - tags.Add(new KeyValuePair(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host)); + // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md + TagList tags = default; - if (!request.RequestUri.IsDefaultPort) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port)); - } - - if (TryFetchResponse(payload, out HttpResponseMessage response)) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode))); - } - - // We are relying here on HttpClient library to set duration before writing the stop event. - // https://github.com/dotnet/runtime/blob/90603686d314147017c8bbe1fa8965776ce607d0/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L178 - // TODO: Follow up with .NET team if we can continue to rely on this behavior. - HttpClientDuration.Record(activity.Duration.TotalMilliseconds, tags); + if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.Method, out var httpMethod)) + { + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); } - - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md - if (this.emitNewAttributes) + else { - TagList tags = default; + // 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(SemanticConventions.AttributeHttpRequestMethod, "_OTHER")); + } - if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method.Method, out var httpMethod)) - { - tags.Add(new KeyValuePair(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(SemanticConventions.AttributeHttpRequestMethod, "_OTHER")); - } + tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); + tags.Add(new KeyValuePair(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme)); - tags.Add(new KeyValuePair(SemanticConventions.AttributeServerAddress, request.RequestUri.Host)); - tags.Add(new KeyValuePair(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme)); + if (!request.RequestUri.IsDefaultPort) + { + tags.Add(new KeyValuePair(SemanticConventions.AttributeServerPort, request.RequestUri.Port)); + } - if (!request.RequestUri.IsDefaultPort) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeServerPort, request.RequestUri.Port)); - } + if (TryFetchResponse(payload, out HttpResponseMessage response)) + { + tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.Version))); + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode))); - if (TryFetchResponse(payload, out HttpResponseMessage response)) + // Set error.type to status code for failed requests + // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md#common-attributes + if (SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)response.StatusCode) == ActivityStatusCode.Error) { - tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.Version))); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode))); - - // Set error.type to status code for failed requests - // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md#common-attributes - if (SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)response.StatusCode) == ActivityStatusCode.Error) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeErrorType, TelemetryHelper.GetStatusCodeString(response.StatusCode))); - } + tags.Add(new KeyValuePair(SemanticConventions.AttributeErrorType, TelemetryHelper.GetStatusCodeString(response.StatusCode))); } + } - if (response == null) - { + if (response == null) + { #if !NET6_0_OR_GREATER - request.Properties.TryGetValue(SemanticConventions.AttributeErrorType, out var errorType); + request.Properties.TryGetValue(SemanticConventions.AttributeErrorType, out var errorType); #else - request.Options.TryGetValue(HttpRequestOptionsErrorKey, out var errorType); + request.Options.TryGetValue(HttpRequestOptionsErrorKey, out var errorType); #endif - // Set error.type to exception type if response was not received. - // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md#common-attributes - if (errorType != null) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeErrorType, errorType)); - } + // Set error.type to exception type if response was not received. + // https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md#common-attributes + if (errorType != null) + { + tags.Add(new KeyValuePair(SemanticConventions.AttributeErrorType, errorType)); } - - // We are relying here on HttpClient library to set duration before writing the stop event. - // https://github.com/dotnet/runtime/blob/90603686d314147017c8bbe1fa8965776ce607d0/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L178 - // TODO: Follow up with .NET team if we can continue to rely on this behavior. - HttpClientRequestDuration.Record(activity.Duration.TotalSeconds, tags); } + + // We are relying here on HttpClient library to set duration before writing the stop event. + // https://github.com/dotnet/runtime/blob/90603686d314147017c8bbe1fa8965776ce607d0/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L178 + // TODO: Follow up with .NET team if we can continue to rely on this behavior. + HttpClientRequestDuration.Record(activity.Duration.TotalSeconds, tags); } // The AOT-annotation DynamicallyAccessedMembers in System.Net.Http library ensures that top-level properties on the payload object are always preserved. diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs index 892f6fa6c5a..08c50229db8 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs @@ -28,12 +28,10 @@ internal static class HttpTagHelper private static readonly ConcurrentDictionary MethodOperationNameCache = new(); private static readonly ConcurrentDictionary HttpMethodOperationNameCache = new(); private static readonly ConcurrentDictionary HttpMethodNameCache = new(); - private static readonly ConcurrentDictionary ProtocolVersionToStringCache = new(); private static readonly Func ConvertMethodToOperationNameRef = ConvertMethodToOperationName; private static readonly Func ConvertHttpMethodToOperationNameRef = ConvertHttpMethodToOperationName; private static readonly Func ConvertHttpMethodToNameRef = ConvertHttpMethodToName; - private static readonly Func ConvertProtocolVersionToStringRef = ConvertProtocolVersionToString; /// /// Gets the OpenTelemetry standard name for an activity based on its Http method. @@ -56,13 +54,6 @@ internal static class HttpTagHelper /// Span method name. public static string GetNameForHttpMethod(HttpMethod method) => HttpMethodNameCache.GetOrAdd(method, ConvertHttpMethodToNameRef); - /// - /// Gets the OpenTelemetry standard version tag value for a span based on its protocol . - /// - /// . - /// Span flavor value. - public static string GetFlavorTagValueFromProtocolVersion(Version protocolVersion) => ProtocolVersionToStringCache.GetOrAdd(protocolVersion, ConvertProtocolVersionToStringRef); - /// /// Gets the OpenTelemetry standard uri tag value for a span based on its request . /// @@ -92,6 +83,4 @@ public static string GetUriTagValueFromRequestUri(Uri uri) private static string ConvertHttpMethodToOperationName(HttpMethod method) => $"HTTP {method}"; private static string ConvertHttpMethodToName(HttpMethod method) => method.ToString(); - - private static string ConvertProtocolVersionToString(Version protocolVersion) => protocolVersion.ToString(); } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index d37dd07070d..800eb5cb106 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -25,7 +25,6 @@ using OpenTelemetry.Context.Propagation; using OpenTelemetry.Internal; using OpenTelemetry.Trace; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.Http.Implementation; @@ -49,16 +48,9 @@ internal static class HttpWebRequestActivitySource private static readonly string Version = AssemblyName.Version.ToString(); private static readonly ActivitySource WebRequestActivitySource = new(ActivitySourceName, Version); private static readonly Meter WebRequestMeter = new(MeterName, Version); - private static readonly Histogram HttpClientDuration = WebRequestMeter.CreateHistogram("http.client.duration", "ms", "Measures the duration of outbound HTTP requests."); private static readonly Histogram HttpClientRequestDuration = WebRequestMeter.CreateHistogram("http.client.request.duration", "s", "Measures the duration of outbound HTTP requests."); private static HttpClientInstrumentationOptions tracingOptions; - private static HttpClientMetricInstrumentationOptions metricsOptions; - - private static bool tracingEmitOldAttributes; - private static bool tracingEmitNewAttributes; - private static bool metricsEmitOldAttributes; - private static bool metricsEmitNewAttributes; // Fields for reflection private static FieldInfo connectionGroupListField; @@ -96,7 +88,6 @@ static HttpWebRequestActivitySource() PerformInjection(); TracingOptions = new HttpClientInstrumentationOptions(); - MetricsOptions = new HttpClientMetricInstrumentationOptions(); } catch (Exception ex) { @@ -111,21 +102,6 @@ internal static HttpClientInstrumentationOptions TracingOptions set { tracingOptions = value; - - tracingEmitOldAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - tracingEmitNewAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); - } - } - - internal static HttpClientMetricInstrumentationOptions MetricsOptions - { - get => metricsOptions; - set - { - metricsOptions = value; - - metricsEmitOldAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - metricsEmitNewAttributes = value.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); } } @@ -136,45 +112,27 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A if (activity.IsAllDataRequested) { - // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md - if (tracingEmitOldAttributes) + // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md + if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod)) { - activity.SetTag(SemanticConventions.AttributeHttpMethod, request.Method); - activity.SetTag(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host); - if (!request.RequestUri.IsDefaultPort) - { - activity.SetTag(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port); - } - - activity.SetTag(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme); - activity.SetTag(SemanticConventions.AttributeHttpUrl, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); - activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion)); + activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod); } - - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md - if (tracingEmitNewAttributes) + else { - 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) - { - activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port); - } + // 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.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); + activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); + if (!request.RequestUri.IsDefaultPort) + { + activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port); } + activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); + try { TracingOptions.EnrichWithHttpWebRequest?.Invoke(activity, request); @@ -193,16 +151,8 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity) if (activity.IsAllDataRequested) { - if (tracingEmitOldAttributes) - { - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); - } - - if (tracingEmitNewAttributes) - { - activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.ProtocolVersion)); - activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); - } + activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.ProtocolVersion)); + activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); try { @@ -280,7 +230,7 @@ private static void ProcessRequest(HttpWebRequest request) var enableTracing = WebRequestActivitySource.HasListeners() && TracingOptions.EventFilterHttpWebRequest(request); - if (!enableTracing && !HttpClientDuration.Enabled && !HttpClientRequestDuration.Enabled) + if (!enableTracing && !HttpClientRequestDuration.Enabled) { // Tracing and metrics are not enabled, so we can skip generating signals // Propagation must still be done in such cases, to allow @@ -390,7 +340,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC { httpStatusCode = response.StatusCode; protocolVersion = response.ProtocolVersion; - activityStatus = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode); + activityStatus = SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)response.StatusCode); if (activityStatus == ActivityStatusCode.Error) { // override the errorType to statusCode for failures. @@ -450,7 +400,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC protocolVersion = response.ProtocolVersion; } - activityStatus = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)httpStatusCode.Value); + activityStatus = SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)httpStatusCode.Value); if (activityStatus == ActivityStatusCode.Error) { @@ -467,7 +417,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC if (activity != null && activity.IsAllDataRequested) { activity.SetStatus(activityStatus); - if (tracingEmitNewAttributes && errorType != null) + if (errorType != null) { activity.SetTag(SemanticConventions.AttributeErrorType, errorType); } @@ -475,82 +425,55 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC activity?.Stop(); - if (HttpClientDuration.Enabled || HttpClientRequestDuration.Enabled) + if (HttpClientRequestDuration.Enabled) { double durationS; - double durationMs; if (activity != null) { durationS = activity.Duration.TotalSeconds; - durationMs = activity.Duration.TotalMilliseconds; } else { var endTimestamp = Stopwatch.GetTimestamp(); durationS = (endTimestamp - startTimestamp) / (double)Stopwatch.Frequency; - durationMs = durationS * 1000; } - if (metricsEmitOldAttributes) - { - TagList tags = default; - - tags.Add(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion)); - tags.Add(SemanticConventions.AttributeHttpMethod, request.Method); - tags.Add(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme); - tags.Add(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host); - if (!request.RequestUri.IsDefaultPort) - { - tags.Add(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port); - } - - if (httpStatusCode.HasValue) - { - tags.Add(SemanticConventions.AttributeHttpStatusCode, (int)httpStatusCode.Value); - } + TagList tags = default; - HttpClientDuration.Record(durationMs, tags); + if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod)) + { + tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); } - - if (metricsEmitNewAttributes) + else { - TagList tags = default; - - if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod)) - { - tags.Add(new KeyValuePair(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(SemanticConventions.AttributeHttpRequestMethod, "_OTHER")); - } - - tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); - tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme); - if (protocolVersion != null) - { - tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(protocolVersion)); - } + // 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(SemanticConventions.AttributeHttpRequestMethod, "_OTHER")); + } - if (!request.RequestUri.IsDefaultPort) - { - tags.Add(SemanticConventions.AttributeServerPort, request.RequestUri.Port); - } + tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); + tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme); + if (protocolVersion != null) + { + tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(protocolVersion)); + } - if (httpStatusCode.HasValue) - { - tags.Add(SemanticConventions.AttributeHttpResponseStatusCode, (int)httpStatusCode.Value); - } + if (!request.RequestUri.IsDefaultPort) + { + tags.Add(SemanticConventions.AttributeServerPort, request.RequestUri.Port); + } - if (errorType != null) - { - tags.Add(SemanticConventions.AttributeErrorType, errorType); - } + if (httpStatusCode.HasValue) + { + tags.Add(SemanticConventions.AttributeHttpResponseStatusCode, (int)httpStatusCode.Value); + } - HttpClientRequestDuration.Record(durationS, tags); + if (errorType != null) + { + tags.Add(SemanticConventions.AttributeErrorType, errorType); } + + HttpClientRequestDuration.Record(durationS, tags); } } diff --git a/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs index 73b7ca44c0a..3d6acde2c24 100644 --- a/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/MeterProviderBuilderExtensions.cs @@ -15,9 +15,9 @@ // #if !NET8_0_OR_GREATER -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; +#if !NETFRAMEWORK using OpenTelemetry.Instrumentation.Http; +#endif using OpenTelemetry.Instrumentation.Http.Implementation; #endif @@ -49,28 +49,12 @@ public static MeterProviderBuilder AddHttpClientInstrumentation( _ = TelemetryHelper.BoxedStatusCodes; _ = RequestMethodHelper.KnownMethods; - builder.ConfigureServices(services => - { - services.RegisterOptionsFactory(configuration => new HttpClientMetricInstrumentationOptions(configuration)); - }); - #if NETFRAMEWORK builder.AddMeter(HttpWebRequestActivitySource.MeterName); - - if (builder is IDeferredMeterProviderBuilder deferredMeterProviderBuilder) - { - deferredMeterProviderBuilder.Configure((sp, builder) => - { - var options = sp.GetRequiredService>().Get(Options.DefaultName); - - HttpWebRequestActivitySource.MetricsOptions = options; - }); - } #else builder.AddMeter(HttpHandlerMetricsDiagnosticListener.MeterName); - builder.AddInstrumentation(sp => new HttpClientMetrics( - sp.GetRequiredService>().Get(Options.DefaultName))); + builder.AddInstrumentation(new HttpClientMetrics()); #endif return builder; #endif diff --git a/src/OpenTelemetry.Instrumentation.Http/README.md b/src/OpenTelemetry.Instrumentation.Http/README.md index 1331d7ad68b..8e502ad7968 100644 --- a/src/OpenTelemetry.Instrumentation.Http/README.md +++ b/src/OpenTelemetry.Instrumentation.Http/README.md @@ -99,31 +99,51 @@ to see how to enable this instrumentation in an ASP.NET application. #### List of metrics produced -A different metric is emitted depending on whether a user opts-in to the new -Http Semantic Conventions using `OTEL_SEMCONV_STABILITY_OPT_IN`. - -* By default, the instrumentation emits the following metric. - - | Name | Instrument Type | Unit | Description | - |-------|-----------------|------|-------------| - | `http.client.duration` | Histogram | `ms` | Measures the duration of outbound HTTP requests. | - -* If user sets the environment variable to `http`, the instrumentation emits - the following metric. - - | Name | Instrument Type | Unit | Description | - |-------|-----------------|------|-------------| - | `http.client.request.duration` | Histogram | `s` | Measures the duration of outbound HTTP requests. | - - This metric is emitted in `seconds` as per the semantic convention. While - the convention [recommends using custom histogram buckets](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md) - , this feature is not yet available via .NET Metrics API. - A [workaround](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4820) +When the application targets `NETFRAMEWORK`, `.NET6.0` or `.NET7.0`, the +instrumentation emits the following metric: + +| Name | Details | +|-----------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------| +| `http.client.request.duration` | [Specification](https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md#metric-httpclientrequestduration) | + +Starting from `.NET8.0`, metrics instrumentation is natively implemented, and +the HttpClient library has incorporated support for [built-in +metrics](https://learn.microsoft.com/dotnet/core/diagnostics/built-in-metrics-system-net) +following the OpenTelemetry semantic conventions. The library includes additional +metrics beyond those defined in the +[specification](https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md), +covering additional scenarios for HttpClient users. When the application targets +`.NET8.0` and newer versions, the instrumentation library automatically enables +all `built-in` metrics by default. + +Note that the `AddHttpClientInstrumentation()` extension simplifies the process +of enabling all built-in metrics via a single line of code. Alternatively, for +more granular control over emitted metrics, you can utilize the `AddMeter()` +extension on `MeterProviderBuilder` for meters listed in +[built-in-metrics-system-net](https://learn.microsoft.com/dotnet/core/diagnostics/built-in-metrics-system-net). +Using `AddMeter()` for metrics activation eliminates the need to take dependency +on the instrumentation library package and calling +`AddHttpClientInstrumentation()`. + +If you utilize `AddHttpClientInstrumentation()` and wish to exclude unnecessary +metrics, you can utilize +[Views](https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/metrics/customizing-the-sdk#drop-an-instrument) +to achieve this. + +**Note:** There is no difference in features or emitted metrics when enabling +metrics using `AddMeter()` or `AddHttpClientInstrumentation()` on `.NET8.0` and +newer versions. + +> **Note** +> The `http.client.request.duration` metric is emitted in `seconds` as + per the semantic convention. While the convention [recommends using custom + histogram + buckets](https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md) + , this feature is not yet available via .NET Metrics API. A + [workaround](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4820) has been included in OTel SDK starting version `1.6.0` which applies - recommended buckets by default for `http.client.request.duration`. - -* If user sets the environment variable to `http/dup`, the instrumentation - emits both `http.client.duration` and `http.client.request.duration`. + recommended buckets by default for `http.client.request.duration`. This + applies to all targeted frameworks. ## Advanced configuration diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index d8c66536741..2ebf6ad5f07 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -44,6 +44,15 @@ before setting up the `MeterProvider`. ([#5052](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5052)) +* Update Metrics SDK to override the default histogram buckets for ASP.NET + (.NET Framework). + + Histogram metrics for the meter name `OpenTelemetry.Instrumentation.AspNet` + and instrument name `http.request.server.duration` which have their `Unit` + as `s` (second) will have their default histogram buckets as `[ 0.005, 0.01, + 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10 ]`. + ([#5063](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5063)) + ## 1.7.0-alpha.1 Released 2023-Oct-16 diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 641996ec091..fc9d2bb9099 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -37,6 +37,7 @@ public sealed class Metric ("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request.time_in_queue"), ("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request_lease.duration"), ("Microsoft.AspNetCore.Server.Kestrel", "kestrel.tls_handshake.duration"), + ("OpenTelemetry.Instrumentation.AspNet", "http.server.request.duration"), ("OpenTelemetry.Instrumentation.AspNetCore", "http.server.request.duration"), ("OpenTelemetry.Instrumentation.Http", "http.client.request.duration"), ("System.Net.Http", "http.client.request.duration"), diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 0ceeb2fef73..17a4145542b 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -94,7 +94,7 @@ void ConfigureTestServices(IServiceCollection services) Assert.Single(exportedItems); var activity = exportedItems[0]; - Assert.Equal(200, activity.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); + Assert.Equal(200, activity.GetTagValue(SemanticConventions.AttributeHttpResponseStatusCode)); Assert.Equal(ActivityStatusCode.Unset, activity.Status); ValidateAspNetCoreActivity(activity, "/api/values"); } @@ -643,7 +643,7 @@ void ConfigureTestServices(IServiceCollection services) Assert.Equal(activityName, middlewareActivity.DisplayName); // tag http.method should be added on activity started by asp.net core - Assert.Equal("GET", aspnetcoreframeworkactivity.GetTagValue(SemanticConventions.AttributeHttpMethod) as string); + Assert.Equal("GET", aspnetcoreframeworkactivity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", aspnetcoreframeworkactivity.OperationName); } @@ -761,7 +761,7 @@ public async Task ActivitiesStartedInMiddlewareBySettingHostActivityToNullShould Assert.Equal(activityName, middlewareActivity.DisplayName); // tag http.method should be added on activity started by asp.net core - Assert.Equal("GET", aspnetcoreframeworkactivity.GetTagValue(SemanticConventions.AttributeHttpMethod) as string); + Assert.Equal("GET", aspnetcoreframeworkactivity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string); Assert.Equal("Microsoft.AspNetCore.Hosting.HttpRequestIn", aspnetcoreframeworkactivity.OperationName); } @@ -1091,7 +1091,7 @@ private static void ValidateAspNetCoreActivity(Activity activityToValidate, stri Assert.Equal(HttpInListener.ActivitySourceName, activityToValidate.Source.Name); Assert.Equal(HttpInListener.Version.ToString(), activityToValidate.Source.Version); #endif - Assert.Equal(expectedHttpPath, activityToValidate.GetTagValue(SemanticConventions.AttributeHttpTarget) as string); + Assert.Equal(expectedHttpPath, activityToValidate.GetTagValue(SemanticConventions.AttributeUrlPath) as string); } private static void AssertException(List exportedItems) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs index d731b9f8881..c685f0e2737 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs @@ -39,11 +39,11 @@ public IncomingRequestsCollectionsIsAccordingToTheSpecTests(WebApplicationFactor } [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_Old( + public async Task SuccessfulTemplateControllerCallGeneratesASpan_New( string urlPath, string query, string userAgent, @@ -51,102 +51,94 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan_Old( string reasonPhrase, bool recordException = false) { - try - { - Environment.SetEnvironmentVariable("OTEL_SEMCONV_STABILITY_OPT_IN", "none"); - - var exportedItems = new List(); + var exportedItems = new List(); - // Arrange - using (var client = this.factory - .WithWebHostBuilder(builder => - { - builder.ConfigureTestServices((IServiceCollection services) => - { - services.AddSingleton(new TestCallbackMiddlewareImpl(statusCode, reasonPhrase)); - services.AddOpenTelemetry() - .WithTracing(builder => builder - .AddAspNetCoreInstrumentation(options => options.RecordException = recordException) - .AddInMemoryExporter(exportedItems)); - }); - builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); - }) - .CreateClient()) + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => { - try + builder.ConfigureTestServices((IServiceCollection services) => { - if (!string.IsNullOrEmpty(userAgent)) - { - client.DefaultRequestHeaders.Add("User-Agent", userAgent); - } - - // Act - var path = urlPath; - if (query != null) - { - path += query; - } - - using var response = await client.GetAsync(path); - } - catch (Exception) + services.AddSingleton(new TestCallbackMiddlewareImpl(statusCode, reasonPhrase)); + services.AddOpenTelemetry() + .WithTracing(builder => builder + .AddAspNetCoreInstrumentation(options => options.RecordException = recordException) + .AddInMemoryExporter(exportedItems)); + }); + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + }) + .CreateClient()) + { + try + { + if (!string.IsNullOrEmpty(userAgent)) { - // ignore errors + client.DefaultRequestHeaders.Add("User-Agent", userAgent); } - for (var i = 0; i < 10; i++) + // Act + var path = urlPath; + if (query != null) { - if (exportedItems.Count == 1) - { - break; - } - - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - await Task.Delay(TimeSpan.FromSeconds(1)); + path += query; } - } - - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - Assert.Equal(ActivityKind.Server, activity.Kind); - Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeNetHostName)); - Assert.Equal("GET", activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); - Assert.Equal("1.1", activity.GetTagValue(SemanticConventions.AttributeHttpFlavor)); - Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeHttpScheme)); - Assert.Equal(urlPath, activity.GetTagValue(SemanticConventions.AttributeHttpTarget)); - Assert.Equal($"http://localhost{urlPath}{query}", activity.GetTagValue(SemanticConventions.AttributeHttpUrl)); - Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); - if (statusCode == 503) - { - Assert.Equal(ActivityStatusCode.Error, activity.Status); + using var response = await client.GetAsync(path); } - else + catch (Exception) { - Assert.Equal(ActivityStatusCode.Unset, activity.Status); + // ignore errors } - // Instrumentation is not expected to set status description - // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.Null(activity.StatusDescription); - - if (recordException) + for (var i = 0; i < 10; i++) { - Assert.Single(activity.Events); - Assert.Equal("exception", activity.Events.First().Name); + if (exportedItems.Count == 1) + { + break; + } + + // We need to let End callback execute as it is executed AFTER response was returned. + // In unit tests environment there may be a lot of parallel unit tests executed, so + // giving some breezing room for the End callback to complete + await Task.Delay(TimeSpan.FromSeconds(1)); } + } - ValidateTagValue(activity, SemanticConventions.AttributeHttpUserAgent, userAgent); + Assert.Single(exportedItems); + var activity = exportedItems[0]; - activity.Dispose(); + Assert.Equal(ActivityKind.Server, activity.Kind); + Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeServerAddress)); + Assert.Equal("GET", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); + Assert.Equal("1.1", activity.GetTagValue(SemanticConventions.AttributeNetworkProtocolVersion)); + Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeUrlScheme)); + Assert.Equal(urlPath, activity.GetTagValue(SemanticConventions.AttributeUrlPath)); + Assert.Equal(query, activity.GetTagValue(SemanticConventions.AttributeUrlQuery)); + Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpResponseStatusCode)); + + if (statusCode == 503) + { + Assert.Equal(ActivityStatusCode.Error, activity.Status); + Assert.Equal("System.Exception", activity.GetTagValue(SemanticConventions.AttributeErrorType)); + } + else + { + Assert.Equal(ActivityStatusCode.Unset, activity.Status); } - finally + + // Instrumentation is not expected to set status description + // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode + Assert.Null(activity.StatusDescription); + + if (recordException) { - Environment.SetEnvironmentVariable("OTEL_SEMCONV_STABILITY_OPT_IN", null); + Assert.Single(activity.Events); + Assert.Equal("exception", activity.Events.First().Name); } + + ValidateTagValue(activity, SemanticConventions.AttributeUserAgentOriginal, userAgent); + + activity.Dispose(); } private static void ValidateTagValue(Activity activity, string attribute, string expectedValue) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs deleted file mode 100644 index bb48d42c920..00000000000 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs +++ /dev/null @@ -1,196 +0,0 @@ -// -// 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. -// - -using System.Diagnostics; -using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Mvc.Testing; -using Microsoft.AspNetCore.TestHost; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using OpenTelemetry.Trace; -using TestApp.AspNetCore; -using Xunit; - -namespace OpenTelemetry.Instrumentation.AspNetCore.Tests; - -public class IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe - : IClassFixture> -{ - private readonly WebApplicationFactory factory; - - public IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe(WebApplicationFactory factory) - { - this.factory = factory; - } - - [Theory] - [InlineData("/api/values", null, "user-agent", 503, "503")] - [InlineData("/api/values", "?query=1", null, 503, null)] - [InlineData("/api/exception", null, null, 503, null)] - [InlineData("/api/exception", null, null, 503, null, true)] - public async Task SuccessfulTemplateControllerCallGeneratesASpan_Dupe( - string urlPath, - string query, - string userAgent, - int statusCode, - string reasonPhrase, - bool recordException = false) - { - try - { - Environment.SetEnvironmentVariable("OTEL_SEMCONV_STABILITY_OPT_IN", "http/dup"); - - var exportedItems = new List(); - - // Arrange - using (var client = this.factory - .WithWebHostBuilder(builder => - { - builder.ConfigureTestServices((IServiceCollection services) => - { - services.AddSingleton(new TestCallbackMiddlewareImpl(statusCode, reasonPhrase)); - services.AddOpenTelemetry() - .WithTracing(builder => builder - .AddAspNetCoreInstrumentation(options => options.RecordException = recordException) - .AddInMemoryExporter(exportedItems)); - }); - builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); - }) - .CreateClient()) - { - try - { - if (!string.IsNullOrEmpty(userAgent)) - { - client.DefaultRequestHeaders.Add("User-Agent", userAgent); - } - - // Act - var path = urlPath; - if (query != null) - { - path += query; - } - - using var response = await client.GetAsync(path); - } - catch (Exception) - { - // ignore errors - } - - for (var i = 0; i < 10; i++) - { - if (exportedItems.Count == 1) - { - break; - } - - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - await Task.Delay(TimeSpan.FromSeconds(1)); - } - } - - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - Assert.Equal(ActivityKind.Server, activity.Kind); - Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeNetHostName)); - Assert.Equal("GET", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); - Assert.Equal("GET", activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); - Assert.Equal("1.1", activity.GetTagValue(SemanticConventions.AttributeNetworkProtocolVersion)); - Assert.Equal("1.1", activity.GetTagValue(SemanticConventions.AttributeHttpFlavor)); - Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeUrlScheme)); - Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeHttpScheme)); - Assert.Equal(urlPath, activity.GetTagValue(SemanticConventions.AttributeUrlPath)); - Assert.Equal(urlPath, activity.GetTagValue(SemanticConventions.AttributeHttpTarget)); - Assert.Equal($"http://localhost{urlPath}{query}", activity.GetTagValue(SemanticConventions.AttributeHttpUrl)); - Assert.Equal(query, activity.GetTagValue(SemanticConventions.AttributeUrlQuery)); - Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpResponseStatusCode)); - Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); - - if (statusCode == 503) - { - Assert.Equal(ActivityStatusCode.Error, activity.Status); - } - else - { - Assert.Equal(ActivityStatusCode.Unset, activity.Status); - } - - // Instrumentation is not expected to set status description - // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.Null(activity.StatusDescription); - - if (recordException) - { - Assert.Single(activity.Events); - Assert.Equal("exception", activity.Events.First().Name); - } - - ValidateTagValue(activity, SemanticConventions.AttributeUserAgentOriginal, userAgent); - - activity.Dispose(); - } - finally - { - Environment.SetEnvironmentVariable("OTEL_SEMCONV_STABILITY_OPT_IN", null); - } - } - - private static void ValidateTagValue(Activity activity, string attribute, string expectedValue) - { - if (string.IsNullOrEmpty(expectedValue)) - { - Assert.Null(activity.GetTagValue(attribute)); - } - else - { - Assert.Equal(expectedValue, activity.GetTagValue(attribute)); - } - } - - public class TestCallbackMiddlewareImpl : CallbackMiddleware.CallbackMiddlewareImpl - { - private readonly int statusCode; - private readonly string reasonPhrase; - - public TestCallbackMiddlewareImpl(int statusCode, string reasonPhrase) - { - this.statusCode = statusCode; - this.reasonPhrase = reasonPhrase; - } - - public override async Task ProcessAsync(HttpContext context) - { - context.Response.StatusCode = this.statusCode; - context.Response.HttpContext.Features.Get().ReasonPhrase = this.reasonPhrase; - await context.Response.WriteAsync("empty"); - - if (context.Request.Path.Value.EndsWith("exception")) - { - throw new Exception("exception description"); - } - - return false; - } - } -} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs deleted file mode 100644 index 00da0c2ebd7..00000000000 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs +++ /dev/null @@ -1,190 +0,0 @@ -// -// 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. -// - -using System.Diagnostics; -using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Mvc.Testing; -using Microsoft.AspNetCore.TestHost; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using OpenTelemetry.Trace; -using TestApp.AspNetCore; -using Xunit; - -namespace OpenTelemetry.Instrumentation.AspNetCore.Tests; - -public class IncomingRequestsCollectionsIsAccordingToTheSpecTests_New - : IClassFixture> -{ - private readonly WebApplicationFactory factory; - - public IncomingRequestsCollectionsIsAccordingToTheSpecTests_New(WebApplicationFactory factory) - { - this.factory = factory; - } - - [Theory] - [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( - string urlPath, - string query, - string userAgent, - int statusCode, - string reasonPhrase, - bool recordException = false) - { - try - { - Environment.SetEnvironmentVariable("OTEL_SEMCONV_STABILITY_OPT_IN", "http"); - - var exportedItems = new List(); - - // Arrange - using (var client = this.factory - .WithWebHostBuilder(builder => - { - builder.ConfigureTestServices((IServiceCollection services) => - { - services.AddSingleton(new TestCallbackMiddlewareImpl(statusCode, reasonPhrase)); - services.AddOpenTelemetry() - .WithTracing(builder => builder - .AddAspNetCoreInstrumentation(options => options.RecordException = recordException) - .AddInMemoryExporter(exportedItems)); - }); - builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); - }) - .CreateClient()) - { - try - { - if (!string.IsNullOrEmpty(userAgent)) - { - client.DefaultRequestHeaders.Add("User-Agent", userAgent); - } - - // Act - var path = urlPath; - if (query != null) - { - path += query; - } - - using var response = await client.GetAsync(path); - } - catch (Exception) - { - // ignore errors - } - - for (var i = 0; i < 10; i++) - { - if (exportedItems.Count == 1) - { - break; - } - - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - await Task.Delay(TimeSpan.FromSeconds(1)); - } - } - - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - Assert.Equal(ActivityKind.Server, activity.Kind); - Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - Assert.Equal("GET", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); - Assert.Equal("1.1", activity.GetTagValue(SemanticConventions.AttributeNetworkProtocolVersion)); - Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeUrlScheme)); - Assert.Equal(urlPath, activity.GetTagValue(SemanticConventions.AttributeUrlPath)); - Assert.Equal(query, activity.GetTagValue(SemanticConventions.AttributeUrlQuery)); - Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpResponseStatusCode)); - - if (statusCode == 503) - { - Assert.Equal(ActivityStatusCode.Error, activity.Status); - Assert.Equal("System.Exception", activity.GetTagValue(SemanticConventions.AttributeErrorType)); - } - else - { - Assert.Equal(ActivityStatusCode.Unset, activity.Status); - } - - // Instrumentation is not expected to set status description - // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.Null(activity.StatusDescription); - - if (recordException) - { - Assert.Single(activity.Events); - Assert.Equal("exception", activity.Events.First().Name); - } - - ValidateTagValue(activity, SemanticConventions.AttributeUserAgentOriginal, userAgent); - - activity.Dispose(); - } - finally - { - Environment.SetEnvironmentVariable("OTEL_SEMCONV_STABILITY_OPT_IN", null); - } - } - - private static void ValidateTagValue(Activity activity, string attribute, string expectedValue) - { - if (string.IsNullOrEmpty(expectedValue)) - { - Assert.Null(activity.GetTagValue(attribute)); - } - else - { - Assert.Equal(expectedValue, activity.GetTagValue(attribute)); - } - } - - public class TestCallbackMiddlewareImpl : CallbackMiddleware.CallbackMiddlewareImpl - { - private readonly int statusCode; - private readonly string reasonPhrase; - - public TestCallbackMiddlewareImpl(int statusCode, string reasonPhrase) - { - this.statusCode = statusCode; - this.reasonPhrase = reasonPhrase; - } - - public override async Task ProcessAsync(HttpContext context) - { - context.Response.StatusCode = this.statusCode; - context.Response.HttpContext.Features.Get().ReasonPhrase = this.reasonPhrase; - await context.Response.WriteAsync("empty"); - - if (context.Request.Path.Value.EndsWith("exception")) - { - throw new Exception("exception description"); - } - - return false; - } - } -} diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 1cb0a6ff2fb..6497fed98de 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -26,8 +26,6 @@ #if NET8_0_OR_GREATER using Microsoft.AspNetCore.RateLimiting; #endif -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; @@ -38,8 +36,6 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Tests; public class MetricTests : IClassFixture>, IDisposable { - public const string SemanticConventionOptInKeyName = "OTEL_SEMCONV_STABILITY_OPT_IN"; - private const int StandardTagsCount = 6; private readonly WebApplicationFactory factory; @@ -188,16 +184,11 @@ public async Task ValidateNet8RateLimitingMetricsAsync() [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) + public async Task RequestMetricIsCaptured(string api, string expectedRoute, string expectedErrorType, int expectedStatusCode) { - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) - .Build(); - var metricItems = new List(); this.meterProvider = Sdk.CreateMeterProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) .AddAspNetCoreInstrumentation() .AddInMemoryExporter(metricItems) .Build(); @@ -237,7 +228,7 @@ public async Task RequestMetricIsCaptured_New(string api, string expectedRoute, var metricPoints = GetMetricPoints(metric); Assert.Single(metricPoints); - AssertMetricPoints_New( + AssertMetricPoints( metricPoints: metricPoints, expectedRoutes: new List { expectedRoute }, expectedErrorType, @@ -259,14 +250,9 @@ public async Task RequestMetricIsCaptured_New(string api, string expectedRoute, [InlineData("CUSTOM", "_OTHER")] public async Task HttpRequestMethodIsCapturedAsPerSpec(string originalMethod, string expectedMethod) { - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) - .Build(); - var metricItems = new List(); this.meterProvider = Sdk.CreateMeterProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) .AddAspNetCoreInstrumentation() .AddInMemoryExporter(metricItems) .Build(); @@ -321,129 +307,6 @@ public async Task HttpRequestMethodIsCapturedAsPerSpec(string originalMethod, st Assert.DoesNotContain(attributes, t => t.Key == SemanticConventions.AttributeHttpRequestMethodOriginal); } -#if !NET8_0_OR_GREATER - [Fact] - public async Task RequestMetricIsCaptured_Old() - { - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = null }) - .Build(); - - var metricItems = new List(); - - this.meterProvider = Sdk.CreateMeterProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddAspNetCoreInstrumentation() - .AddInMemoryExporter(metricItems) - .Build(); - - using (var client = this.factory - .WithWebHostBuilder(builder => - { - builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); - }) - .CreateClient()) - { - using var response1 = await client.GetAsync("/api/values"); - using var response2 = await client.GetAsync("/api/values/2"); - - response1.EnsureSuccessStatusCode(); - response2.EnsureSuccessStatusCode(); - } - - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - await Task.Delay(TimeSpan.FromSeconds(1)); - - this.meterProvider.Dispose(); - - var requestMetrics = metricItems - .Where(item => item.Name == "http.server.duration") - .ToArray(); - - var metric = Assert.Single(requestMetrics); - Assert.Equal("ms", metric.Unit); - var metricPoints = GetMetricPoints(metric); - Assert.Equal(2, metricPoints.Count); - - AssertMetricPoints_Old( - metricPoints: metricPoints, - expectedRoutes: new List { "api/Values", "api/Values/{id}" }, - expectedTagsCount: 6); - } - - [Fact] - public async Task RequestMetricIsCaptured_Dup() - { - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http/dup" }) - .Build(); - - var metricItems = new List(); - - this.meterProvider = Sdk.CreateMeterProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddAspNetCoreInstrumentation() - .AddInMemoryExporter(metricItems) - .Build(); - - using (var client = this.factory - .WithWebHostBuilder(builder => - { - builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); - }) - .CreateClient()) - { - using var response1 = await client.GetAsync("/api/values"); - using var response2 = await client.GetAsync("/api/values/2"); - - response1.EnsureSuccessStatusCode(); - response2.EnsureSuccessStatusCode(); - } - - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - await Task.Delay(TimeSpan.FromSeconds(1)); - - this.meterProvider.Dispose(); - - // Validate Old Semantic Convention - var requestMetrics = metricItems - .Where(item => item.Name == "http.server.duration") - .ToArray(); - - var metric = Assert.Single(requestMetrics); - Assert.Equal("ms", metric.Unit); - var metricPoints = GetMetricPoints(metric); - Assert.Equal(2, metricPoints.Count); - - AssertMetricPoints_Old( - metricPoints: metricPoints, - expectedRoutes: new List { "api/Values", "api/Values/{id}" }, - expectedTagsCount: 6); - - // Validate New Semantic Convention - requestMetrics = metricItems - .Where(item => item.Name == "http.server.request.duration") - .ToArray(); - - metric = Assert.Single(requestMetrics); - - Assert.Equal("s", metric.Unit); - metricPoints = GetMetricPoints(metric); - Assert.Equal(2, metricPoints.Count); - - AssertMetricPoints_New( - metricPoints: metricPoints, - expectedRoutes: new List { "api/Values", "api/Values/{id}" }, - null, - 200, - expectedTagsCount: 5); - } -#endif - public void Dispose() { this.meterProvider?.Dispose(); @@ -463,7 +326,7 @@ private static List GetMetricPoints(Metric metric) return metricPoints; } - private static void AssertMetricPoints_New( + private static void AssertMetricPoints( List metricPoints, List expectedRoutes, string expectedErrorType, @@ -488,7 +351,7 @@ private static void AssertMetricPoints_New( if (metricPoint.HasValue) { - AssertMetricPoint_New(metricPoint.Value, expectedStatusCode, expectedRoute, expectedErrorType, expectedTagsCount); + AssertMetricPoint(metricPoint.Value, expectedStatusCode, expectedRoute, expectedErrorType, expectedTagsCount); } else { @@ -497,39 +360,7 @@ private static void AssertMetricPoints_New( } } - private static void AssertMetricPoints_Old( - List metricPoints, - List expectedRoutes, - int expectedTagsCount) - { - // Assert that one MetricPoint exists for each ExpectedRoute - foreach (var expectedRoute in expectedRoutes) - { - MetricPoint? metricPoint = null; - - foreach (var mp in metricPoints) - { - foreach (var tag in mp.Tags) - { - if (tag.Key == SemanticConventions.AttributeHttpRoute && tag.Value.ToString() == expectedRoute) - { - metricPoint = mp; - } - } - } - - if (metricPoint.HasValue) - { - AssertMetricPoint_Old(metricPoint.Value, expectedRoute, expectedTagsCount); - } - else - { - Assert.Fail($"A metric for route '{expectedRoute}' was not found"); - } - } - } - - private static KeyValuePair[] AssertMetricPoint_New( + private static void AssertMetricPoint( MetricPoint metricPoint, int expectedStatusCode, string expectedRoute, @@ -587,56 +418,5 @@ private static KeyValuePair[] AssertMetricPoint_New( Enumerable.SequenceEqual(expectedHistogramBoundsNew, histogramBounds); Assert.True(histogramBoundsMatchCorrectly); - - return attributes; - } - - private static KeyValuePair[] AssertMetricPoint_Old( - MetricPoint metricPoint, - string expectedRoute = "api/Values", - int expectedTagsCount = StandardTagsCount) - { - var count = metricPoint.GetHistogramCount(); - var sum = metricPoint.GetHistogramSum(); - - Assert.Equal(1L, count); - Assert.True(sum > 0); - - var attributes = new KeyValuePair[metricPoint.Tags.Count]; - int i = 0; - foreach (var tag in metricPoint.Tags) - { - attributes[i++] = tag; - } - - // Inspect Attributes - Assert.Equal(expectedTagsCount, attributes.Length); - - var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, "GET"); - var scheme = new KeyValuePair(SemanticConventions.AttributeHttpScheme, "http"); - var statusCode = new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, 200); - var flavor = new KeyValuePair(SemanticConventions.AttributeHttpFlavor, "1.1"); - var host = new KeyValuePair(SemanticConventions.AttributeNetHostName, "localhost"); - var route = new KeyValuePair(SemanticConventions.AttributeHttpRoute, expectedRoute); - Assert.Contains(method, attributes); - Assert.Contains(scheme, attributes); - Assert.Contains(statusCode, attributes); - Assert.Contains(flavor, attributes); - Assert.Contains(host, attributes); - Assert.Contains(route, attributes); - - // Inspect Histogram Bounds - var histogramBuckets = metricPoint.GetHistogramBuckets(); - var histogramBounds = new List(); - foreach (var t in histogramBuckets) - { - histogramBounds.Add(t.ExplicitBound); - } - - Assert.Equal( - expected: new List { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000, double.PositiveInfinity }, - actual: histogramBounds); - - return attributes; } } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.cs index cd7cf88222d..bf5c40cbcb1 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestCases.cs @@ -49,8 +49,7 @@ private static IEnumerable GetArgumentsFromTestCaseObject(IEnumerable< continue; } - result.Add(new object[] { testCase, true }); - result.Add(new object[] { testCase, false }); + result.Add(new object[] { testCase }); } return result; diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs index dda7df55749..f03b21c0405 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs @@ -17,14 +17,11 @@ #nullable enable using System.Diagnostics; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.DependencyInjection; using OpenTelemetry; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; using RouteTests.TestApplication; using Xunit; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace RouteTests; @@ -49,20 +46,14 @@ public RoutingTests(RoutingTestFixture fixture) [Theory] [MemberData(nameof(TestData))] - public async Task TestHttpRoute(RoutingTestCases.TestCase testCase, bool useLegacyConventions) + public async Task TestHttpRoute(RoutingTestCases.TestCase testCase) { - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = useLegacyConventions ? null : "http" }) - .Build(); - using var tracerProvider = Sdk.CreateTracerProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) .AddAspNetCoreInstrumentation() .AddInMemoryExporter(this.exportedActivities) .Build()!; using var meterProvider = Sdk.CreateMeterProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) .AddAspNetCoreInstrumentation() .AddInMemoryExporter(this.exportedMetrics) .Build()!; @@ -91,8 +82,8 @@ public async Task TestHttpRoute(RoutingTestCases.TestCase testCase, bool useLega var activity = Assert.Single(this.exportedActivities); var metricPoint = Assert.Single(metricPoints); - GetTagsFromActivity(useLegacyConventions, activity, out var activityHttpStatusCode, out var activityHttpMethod, out var activityHttpRoute); - GetTagsFromMetricPoint(useLegacyConventions && Environment.Version.Major < 8, metricPoint, out var metricHttpStatusCode, out var metricHttpMethod, out var metricHttpRoute); + GetTagsFromActivity(activity, out var activityHttpStatusCode, out var activityHttpMethod, out var activityHttpRoute); + GetTagsFromMetricPoint(Environment.Version.Major < 8, metricPoint, out var metricHttpStatusCode, out var metricHttpMethod, out var metricHttpRoute); Assert.Equal(testCase.ExpectedStatusCode, activityHttpStatusCode); Assert.Equal(testCase.ExpectedStatusCode, metricHttpStatusCode); @@ -113,27 +104,23 @@ public async Task TestHttpRoute(RoutingTestCases.TestCase testCase, bool useLega Assert.Equal(expectedActivityDisplayName, activity.DisplayName); - // Only produce README files based on final semantic conventions - if (!useLegacyConventions) + var testResult = new RoutingTestResult { - var testResult = new RoutingTestResult - { - IdealHttpRoute = testCase.ExpectedHttpRoute, - ActivityDisplayName = activity.DisplayName, - ActivityHttpRoute = activityHttpRoute, - MetricHttpRoute = metricHttpRoute, - TestCase = testCase, - RouteInfo = RouteInfo.Current, - }; - - this.fixture.AddTestResult(testResult); - } + IdealHttpRoute = testCase.ExpectedHttpRoute, + ActivityDisplayName = activity.DisplayName, + ActivityHttpRoute = activityHttpRoute, + MetricHttpRoute = metricHttpRoute, + TestCase = testCase, + RouteInfo = RouteInfo.Current, + }; + + this.fixture.AddTestResult(testResult); } - private static void GetTagsFromActivity(bool useLegacyConventions, Activity activity, out int httpStatusCode, out string httpMethod, out string? httpRoute) + private static void GetTagsFromActivity(Activity activity, out int httpStatusCode, out string httpMethod, out string? httpRoute) { - var expectedStatusCodeKey = useLegacyConventions ? OldHttpStatusCode : HttpStatusCode; - var expectedHttpMethodKey = useLegacyConventions ? OldHttpMethod : HttpMethod; + var expectedStatusCodeKey = HttpStatusCode; + var expectedHttpMethodKey = HttpMethod; httpStatusCode = Convert.ToInt32(activity.GetTagItem(expectedStatusCodeKey)); httpMethod = (activity.GetTagItem(expectedHttpMethodKey) as string)!; httpRoute = activity.GetTagItem(HttpRoute) as string ?? string.Empty; @@ -141,8 +128,8 @@ private static void GetTagsFromActivity(bool useLegacyConventions, Activity acti private static void GetTagsFromMetricPoint(bool useLegacyConventions, MetricPoint metricPoint, out int httpStatusCode, out string httpMethod, out string? httpRoute) { - var expectedStatusCodeKey = useLegacyConventions ? OldHttpStatusCode : HttpStatusCode; - var expectedHttpMethodKey = useLegacyConventions ? OldHttpMethod : HttpMethod; + var expectedStatusCodeKey = HttpStatusCode; + var expectedHttpMethodKey = HttpMethod; httpStatusCode = 0; httpMethod = string.Empty; diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs index c8011b4bb84..da734da25f9 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs @@ -51,77 +51,6 @@ public GrpcTests() [InlineData(true)] [InlineData(false)] public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(bool? enableGrpcAspNetCoreSupport) - { - var exportedItems = new List(); - var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder(); - - if (enableGrpcAspNetCoreSupport.HasValue) - { - tracerProviderBuilder.AddAspNetCoreInstrumentation(options => - { - options.EnableGrpcAspNetCoreSupport = enableGrpcAspNetCoreSupport.Value; - }); - } - else - { - tracerProviderBuilder.AddAspNetCoreInstrumentation(); - } - - using var tracerProvider = tracerProviderBuilder - .AddInMemoryExporter(exportedItems) - .Build(); - - var clientLoopbackAddresses = new[] { IPAddress.Loopback.ToString(), IPAddress.IPv6Loopback.ToString() }; - var uri = new Uri($"http://localhost:{this.server.Port}"); - - using var channel = GrpcChannel.ForAddress(uri); - var client = new Greeter.GreeterClient(channel); - var returnMsg = client.SayHello(new HelloRequest()).Message; - Assert.False(string.IsNullOrEmpty(returnMsg)); - - WaitForExporterToReceiveItems(exportedItems, 1); - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - Assert.Equal(ActivityKind.Server, activity.Kind); - - if (!enableGrpcAspNetCoreSupport.HasValue || enableGrpcAspNetCoreSupport.Value) - { - Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem)); - Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService)); - Assert.Equal("SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); - Assert.Contains(activity.GetTagValue(SemanticConventions.AttributeNetPeerIp), clientLoopbackAddresses); - Assert.NotEqual(0, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); - Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); - } - else - { - Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); - } - - Assert.Equal(Status.Unset, activity.GetStatus()); - - // The following are http.* attributes that are also included on the span for the gRPC invocation. - Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeNetHostName)); - Assert.Equal(this.server.Port, activity.GetTagValue(SemanticConventions.AttributeNetHostPort)); - Assert.Equal("POST", activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); - Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpTarget)); - Assert.Equal($"http://localhost:{this.server.Port}/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpUrl)); - Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string); - } - - // Tests for v1.21.0 Semantic Conventions for database client calls. - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md - // This test emits the new attributes. - // This test method can replace the other (old) test method when this library is GA. - [Theory] - [InlineData(null)] - [InlineData(true)] - [InlineData(false)] - public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes_New(bool? enableGrpcAspNetCoreSupport) { var configuration = new ConfigurationBuilder() .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) @@ -190,112 +119,6 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes_New(bool? enableG Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeUserAgentOriginal) as string); } - // Tests for v1.21.0 Semantic Conventions for database client calls. - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md - // This test emits both the new and older attributes. - // This test method can be deleted when this library is GA. - [Theory] - [InlineData(null)] - [InlineData(true)] - [InlineData(false)] - public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes_Dupe(bool? enableGrpcAspNetCoreSupport) - { - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http/dup" }) - .Build(); - - var exportedItems = new List(); - var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)); - - if (enableGrpcAspNetCoreSupport.HasValue) - { - tracerProviderBuilder.AddAspNetCoreInstrumentation(options => - { - options.EnableGrpcAspNetCoreSupport = enableGrpcAspNetCoreSupport.Value; - }); - } - else - { - tracerProviderBuilder.AddAspNetCoreInstrumentation(); - } - - using var tracerProvider = tracerProviderBuilder - .AddInMemoryExporter(exportedItems) - .Build(); - - var clientLoopbackAddresses = new[] { IPAddress.Loopback.ToString(), IPAddress.IPv6Loopback.ToString() }; - var uri = new Uri($"http://localhost:{this.server.Port}"); - - using var channel = GrpcChannel.ForAddress(uri); - var client = new Greeter.GreeterClient(channel); - var returnMsg = client.SayHello(new HelloRequest()).Message; - Assert.False(string.IsNullOrEmpty(returnMsg)); - - WaitForExporterToReceiveItems(exportedItems, 1); - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - Assert.Equal(ActivityKind.Server, activity.Kind); - - // OLD - if (!enableGrpcAspNetCoreSupport.HasValue || enableGrpcAspNetCoreSupport.Value) - { - Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem)); - Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService)); - Assert.Equal("SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); - Assert.Contains(activity.GetTagValue(SemanticConventions.AttributeNetPeerIp), clientLoopbackAddresses); - Assert.NotEqual(0, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); - Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); - } - else - { - Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); - } - - Assert.Equal(Status.Unset, activity.GetStatus()); - - // The following are http.* attributes that are also included on the span for the gRPC invocation. - Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeNetHostName)); - Assert.Equal(this.server.Port, activity.GetTagValue(SemanticConventions.AttributeNetHostPort)); - Assert.Equal("POST", activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); - Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpTarget)); - Assert.Equal($"http://localhost:{this.server.Port}/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpUrl)); - Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string); - - // NEW - if (!enableGrpcAspNetCoreSupport.HasValue || enableGrpcAspNetCoreSupport.Value) - { - Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem)); - Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService)); - Assert.Equal("SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); - Assert.Contains(activity.GetTagValue(SemanticConventions.AttributeClientAddress), clientLoopbackAddresses); - Assert.NotEqual(0, activity.GetTagValue(SemanticConventions.AttributeClientPort)); - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); - Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); - } - else - { - Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); - } - - Assert.Equal(Status.Unset, activity.GetStatus()); - - // The following are http.* attributes that are also included on the span for the gRPC invocation. - Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - Assert.Equal(this.server.Port, activity.GetTagValue(SemanticConventions.AttributeServerPort)); - Assert.Equal("POST", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); - Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeUrlScheme)); - Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeUrlPath)); - Assert.Equal("2", activity.GetTagValue(SemanticConventions.AttributeNetworkProtocolVersion)); - Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeUserAgentOriginal) as string); - } - #if NET6_0_OR_GREATER [Theory(Skip = "Skipping for .NET 6 and higher due to bug #3023")] #endif diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index cac7ec924bd..5fd03c1ddb2 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -15,7 +15,6 @@ // using System.Diagnostics; -using Microsoft.Extensions.Configuration; #if NETFRAMEWORK using System.Net; using System.Net.Http; @@ -30,8 +29,6 @@ using Xunit; using Xunit.Abstractions; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; - namespace OpenTelemetry.Instrumentation.Http.Tests; public partial class HttpClientTests : IDisposable @@ -393,12 +390,7 @@ public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string originalMetho Method = new HttpMethod(originalMethod), }; - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) - .Build(); - using var traceprovider = Sdk.CreateTracerProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) .AddHttpClientInstrumentation() .AddInMemoryExporter(exportedItems) .Build(); @@ -453,12 +445,7 @@ public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string Method = new HttpMethod(originalMethod), }; - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { [SemanticConventionOptInKeyName] = "http" }) - .Build(); - using var meterprovider = Sdk.CreateMeterProviderBuilder() - .ConfigureServices(services => services.AddSingleton(configuration)) .AddHttpClientInstrumentation() .AddInMemoryExporter(metricItems) .Build(); @@ -523,15 +510,15 @@ public async Task RedirectTest() Assert.Equal(5, processor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnShutdown/Dispose called. var firstActivity = (Activity)processor.Invocations[2].Arguments[0]; // First OnEnd - Assert.Contains(firstActivity.TagObjects, t => t.Key == "http.status_code" && (int)t.Value == 200); + Assert.Contains(firstActivity.TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 200); #else Assert.Equal(7, processor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnStart/OnEnd/OnShutdown/Dispose called. var firstActivity = (Activity)processor.Invocations[2].Arguments[0]; // First OnEnd - Assert.Contains(firstActivity.TagObjects, t => t.Key == "http.status_code" && (int)t.Value == 302); + Assert.Contains(firstActivity.TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 302); var secondActivity = (Activity)processor.Invocations[4].Arguments[0]; // Second OnEnd - Assert.Contains(secondActivity.TagObjects, t => t.Key == "http.status_code" && (int)t.Value == 200); + Assert.Contains(secondActivity.TagObjects, t => t.Key == "http.response.status_code" && (int)t.Value == 200); #endif } diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index dfa89738811..b7d381c56ef 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -22,12 +22,9 @@ using System.Reflection; using System.Text.Json; #endif -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; using Xunit; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.Http.Tests; @@ -35,45 +32,16 @@ public partial class HttpClientTests { public static readonly IEnumerable TestData = HttpTestData.ReadTestCases(); -#if !NET8_0_OR_GREATER - [Theory] - [MemberData(nameof(TestData))] - public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsOldSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc) - { - await HttpOutCallsAreCollectedSuccessfullyBodyAsync( - this.host, - this.port, - tc, - enableTracing: true, - enableMetrics: true, - semanticConvention: HttpSemanticConvention.Old); - } - [Theory] [MemberData(nameof(TestData))] - public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsDuplicateSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc) + public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc) { await HttpOutCallsAreCollectedSuccessfullyBodyAsync( this.host, this.port, tc, enableTracing: true, - enableMetrics: true, - semanticConvention: HttpSemanticConvention.Dupe); - } -#endif - - [Theory] - [MemberData(nameof(TestData))] - public async Task HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsNewSemanticConventionsAsync(HttpTestData.HttpOutTestCase tc) - { - await HttpOutCallsAreCollectedSuccessfullyBodyAsync( - this.host, - this.port, - tc, - enableTracing: true, - enableMetrics: true, - semanticConvention: HttpSemanticConvention.New); + enableMetrics: true); } [Theory] @@ -129,20 +97,20 @@ public async Task DebugIndividualTestAsync() ""spanStatus"": ""Unset"", ""spanKind"": ""Client"", ""spanAttributes"": { - ""http.scheme"": ""http"", - ""http.method"": ""GET"", - ""net.peer.name"": ""{host}"", - ""net.peer.port"": ""{port}"", - ""http.status_code"": ""399"", - ""http.flavor"": ""{flavor}"", - ""http.url"": ""http://{host}:{port}/"" + ""url.scheme"": ""http"", + ""http.request.method"": ""GET"", + ""server.address"": ""{host}"", + ""server.port"": ""{port}"", + ""http.response.status_code"": ""399"", + ""network.protocol.version"": ""{flavor}"", + ""url.full"": ""http://{host}:{port}/"" } } ] ", new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); - var t = (Task)this.GetType().InvokeMember(nameof(this.HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsOldSemanticConventionsAsync), BindingFlags.InvokeMethod, null, this, HttpTestData.GetArgumentsFromTestCaseObject(input).First()); + var t = (Task)this.GetType().InvokeMember(nameof(this.HttpOutCallsAreCollectedSuccessfullyTracesAndMetricsSemanticConventionsAsync), BindingFlags.InvokeMethod, null, this, HttpTestData.GetArgumentsFromTestCaseObject(input).First()); await t; } #endif @@ -216,8 +184,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( int port, HttpTestData.HttpOutTestCase tc, bool enableTracing, - bool enableMetrics, - HttpSemanticConvention? semanticConvention = null) + bool enableMetrics) { bool enrichWithHttpWebRequestCalled = false; bool enrichWithHttpWebResponseCalled = false; @@ -232,9 +199,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( if (enableMetrics) { meterProviderBuilder - .AddHttpClientInstrumentation() - .ConfigureServices( - s => s.AddSingleton(BuildConfigurationWithSemanticConventionOptIn(semanticConvention))); + .AddHttpClientInstrumentation(); } var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder(); @@ -250,9 +215,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( opt.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => { enrichWithHttpResponseMessageCalled = true; }; opt.EnrichWithException = (activity, exception) => { enrichWithExceptionCalled = true; }; opt.RecordException = tc.RecordException ?? false; - }) - .ConfigureServices( - s => s.AddSingleton(BuildConfigurationWithSemanticConventionOptIn(semanticConvention))); + }); } var metrics = new List(); @@ -299,7 +262,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( } var requestMetrics = metrics - .Where(metric => metric.Name == "http.client.duration" || metric.Name == "http.client.request.duration") + .Where(metric => metric.Name == "http.client.request.duration") .ToArray(); var normalizedAttributesTestCase = tc.SpanAttributes.ToDictionary(x => x.Key, x => HttpTestData.NormalizeValues(x.Value, host, port)); @@ -339,66 +302,40 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( var normalizedAttributes = activity.TagObjects.Where(kv => !kv.Key.StartsWith("otel.")).ToDictionary(x => x.Key, x => x.Value.ToString()); - int numberOfNewTags = activity.Status == ActivityStatusCode.Error ? 5 : 4; - int numberOfDupeTags = activity.Status == ActivityStatusCode.Error ? 11 : 10; + int numberOfTags = activity.Status == ActivityStatusCode.Error ? 5 : 4; - var expectedAttributeCount = semanticConvention == HttpSemanticConvention.Dupe - ? numberOfDupeTags + (tc.ResponseExpected ? 3 : 0) - : semanticConvention == HttpSemanticConvention.New - ? numberOfNewTags + (tc.ResponseExpected ? 2 : 0) - : 6 + (tc.ResponseExpected ? 1 : 0); + var expectedAttributeCount = numberOfTags + (tc.ResponseExpected ? 2 : 0); Assert.Equal(expectedAttributeCount, normalizedAttributes.Count); - if (semanticConvention == null || semanticConvention.Value.HasFlag(HttpSemanticConvention.Old)) + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpRequestMethod]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeServerAddress]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeServerPort]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeUrlFull && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeUrlFull]); + if (tc.ResponseExpected) { - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerName && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpUrl && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpUrl]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpFlavor && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); - if (tc.ResponseExpected) - { - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); - } - else + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetworkProtocolVersion]); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpResponseStatusCode]); + + if (tc.ResponseCode >= 400) { - Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpResponseStatusCode]); } } - - if (semanticConvention != null && semanticConvention.Value.HasFlag(HttpSemanticConvention.New)) + else { - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeUrlFull && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpUrl]); - if (tc.ResponseExpected) - { - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); - - if (tc.ResponseCode >= 400) - { - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); - } - } - else - { - Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); - Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion); + Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); + Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion); #if NET8_0_OR_GREATER - // we are using fake address so it will be "name_resolution_error" - // TODO: test other error types. - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_error"); + // we are using fake address so it will be "name_resolution_error" + // TODO: test other error types. + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_error"); #elif NETFRAMEWORK - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_failure"); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_failure"); #else - Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "System.Net.Http.HttpRequestException"); + Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "System.Net.Http.HttpRequestException"); #endif - } } if (tc.RecordException.HasValue && tc.RecordException.Value) @@ -414,212 +351,113 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( } else { - if (semanticConvention == HttpSemanticConvention.Dupe) - { - Assert.Equal(2, requestMetrics.Length); - } - else - { - Assert.Single(requestMetrics); - } - -#if !NET8_0_OR_GREATER - if (semanticConvention == null || semanticConvention.Value.HasFlag(HttpSemanticConvention.Old)) - { - var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.duration"); - Assert.NotNull(metric); - Assert.Equal("ms", metric.Unit); - Assert.True(metric.MetricType == MetricType.Histogram); - - var metricPoints = new List(); - foreach (var p in metric.GetMetricPoints()) - { - metricPoints.Add(p); - } - - Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; - - var count = metricPoint.GetHistogramCount(); - var sum = metricPoint.GetHistogramSum(); - - Assert.Equal(1L, count); + Assert.Single(requestMetrics); - if (enableTracing) - { - var activity = Assert.Single(activities); - Assert.Equal(activity.Duration.TotalMilliseconds, sum); - } - else - { - Assert.True(sum > 0); - } + var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.request.duration"); + Assert.NotNull(metric); + Assert.Equal("s", metric.Unit); + Assert.True(metric.MetricType == MetricType.Histogram); - // Inspect Metric Attributes - var attributes = new Dictionary(); - foreach (var tag in metricPoint.Tags) - { - attributes[tag.Key] = tag.Value; - } - - var expectedAttributeCount = 5 + (tc.ResponseExpected ? 1 : 0); - - Assert.Equal(expectedAttributeCount, attributes.Count); - - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerName && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetPeerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpFlavor && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); - if (tc.ResponseExpected) - { - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); - } - else - { - Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpStatusCode); - } - - // Inspect Histogram Bounds - var histogramBuckets = metricPoint.GetHistogramBuckets(); - var histogramBounds = new List(); - foreach (var t in histogramBuckets) - { - histogramBounds.Add(t.ExplicitBound); - } - - Assert.Equal( - expected: new List { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000, double.PositiveInfinity }, - actual: histogramBounds); - } -#endif - if (semanticConvention != null && semanticConvention.Value.HasFlag(HttpSemanticConvention.New)) + var metricPoints = new List(); + foreach (var p in metric.GetMetricPoints()) { - var metric = requestMetrics.FirstOrDefault(m => m.Name == "http.client.request.duration"); - Assert.NotNull(metric); - Assert.Equal("s", metric.Unit); - Assert.True(metric.MetricType == MetricType.Histogram); - - var metricPoints = new List(); - foreach (var p in metric.GetMetricPoints()) - { - metricPoints.Add(p); - } + metricPoints.Add(p); + } - Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; - var count = metricPoint.GetHistogramCount(); - var sum = metricPoint.GetHistogramSum(); + var count = metricPoint.GetHistogramCount(); + var sum = metricPoint.GetHistogramSum(); - Assert.Equal(1L, count); + Assert.Equal(1L, count); - if (enableTracing) - { - var activity = Assert.Single(activities); + if (enableTracing) + { + var activity = Assert.Single(activities); #if !NET8_0_OR_GREATER - Assert.Equal(activity.Duration.TotalSeconds, sum); + Assert.Equal(activity.Duration.TotalSeconds, sum); #endif - } - else - { - Assert.True(sum > 0); - } + } + else + { + Assert.True(sum > 0); + } - // Inspect Metric Attributes - var attributes = new Dictionary(); - foreach (var tag in metricPoint.Tags) - { - attributes[tag.Key] = tag.Value; - } + // Inspect Metric Attributes + var attributes = new Dictionary(); + foreach (var tag in metricPoint.Tags) + { + attributes[tag.Key] = tag.Value; + } - var numberOfTags = 4; - if (tc.ResponseExpected) - { - var expectedStatusCode = int.Parse(normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); - numberOfTags = (expectedStatusCode >= 400) ? 5 : 4; // error.type extra tag - } - else - { - numberOfTags = 5; // error.type would be extra - } + var numberOfTags = 4; + if (tc.ResponseExpected) + { + var expectedStatusCode = int.Parse(normalizedAttributesTestCase[SemanticConventions.AttributeHttpResponseStatusCode]); + numberOfTags = (expectedStatusCode >= 400) ? 5 : 4; // error.type extra tag + } + else + { + numberOfTags = 5; // error.type would be extra + } - var expectedAttributeCount = numberOfTags + (tc.ResponseExpected ? 2 : 0); // responsecode + protocolversion + var expectedAttributeCount = numberOfTags + (tc.ResponseExpected ? 2 : 0); // responsecode + protocolversion - Assert.Equal(expectedAttributeCount, attributes.Count); + Assert.Equal(expectedAttributeCount, attributes.Count); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeUrlScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpRequestMethod]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeServerAddress]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeServerPort]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeUrlScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeUrlScheme]); - if (tc.ResponseExpected) - { - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]); - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); + if (tc.ResponseExpected) + { + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetworkProtocolVersion]); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpResponseStatusCode]); - if (tc.ResponseCode >= 400) - { - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]); - } - } - else + if (tc.ResponseCode >= 400) { - Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion); - Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpResponseStatusCode]); + } + } + else + { + Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion); + Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode); #if NET8_0_OR_GREATER - // we are using fake address so it will be "name_resolution_error" - // TODO: test other error types. - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_error"); + // we are using fake address so it will be "name_resolution_error" + // TODO: test other error types. + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_error"); #elif NETFRAMEWORK - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_failure"); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "name_resolution_failure"); #else - Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "System.Net.Http.HttpRequestException"); + Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeErrorType && kvp.Value.ToString() == "System.Net.Http.HttpRequestException"); #endif - } + } - // Inspect Histogram Bounds - var histogramBuckets = metricPoint.GetHistogramBuckets(); - var histogramBounds = new List(); - foreach (var t in histogramBuckets) - { - histogramBounds.Add(t.ExplicitBound); - } + // Inspect Histogram Bounds + var histogramBuckets = metricPoint.GetHistogramBuckets(); + var histogramBounds = new List(); + foreach (var t in histogramBuckets) + { + histogramBounds.Add(t.ExplicitBound); + } - // TODO: Remove the check for the older bounds once 1.7.0 is released. This is a temporary fix for instrumentation libraries CI workflow. + // TODO: Remove the check for the older bounds once 1.7.0 is released. This is a temporary fix for instrumentation libraries CI workflow. - var expectedHistogramBoundsOld = new List { 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10, double.PositiveInfinity }; - var expectedHistogramBoundsNew = new List { 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10, double.PositiveInfinity }; + var expectedHistogramBoundsOld = new List { 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10, double.PositiveInfinity }; + var expectedHistogramBoundsNew = new List { 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10, double.PositiveInfinity }; - var histogramBoundsMatchCorrectly = Enumerable.SequenceEqual(expectedHistogramBoundsOld, histogramBounds) || - Enumerable.SequenceEqual(expectedHistogramBoundsNew, histogramBounds); + var histogramBoundsMatchCorrectly = Enumerable.SequenceEqual(expectedHistogramBoundsOld, histogramBounds) || + Enumerable.SequenceEqual(expectedHistogramBoundsNew, histogramBounds); - Assert.True(histogramBoundsMatchCorrectly); - } + Assert.True(histogramBoundsMatchCorrectly); } } - private static IConfiguration BuildConfigurationWithSemanticConventionOptIn( - HttpSemanticConvention? semanticConvention) - { - var builder = new ConfigurationBuilder(); - - if (semanticConvention != null && semanticConvention != HttpSemanticConvention.Old) - { - builder.AddInMemoryCollection( - new Dictionary - { - ["OTEL_SEMCONV_STABILITY_OPT_IN"] = semanticConvention == HttpSemanticConvention.Dupe - ? "http/dup" - : "http", - }); - } - - return builder.Build(); - } - private static async Task CheckEnrichment(Sampler sampler, bool enrichExpected, string url) { bool enrichWithHttpWebRequestCalled = false; diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs index 9dbdb162fb7..be4f6a2909e 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs @@ -776,20 +776,20 @@ private static void VerifyHeaders(HttpWebRequest startRequest) private static void VerifyActivityStartTags(string netPeerName, int? netPeerPort, string method, string url, Activity activity) { Assert.NotNull(activity.TagObjects); - Assert.Equal(method, activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); + Assert.Equal(method, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); if (netPeerPort != null) { - Assert.Equal(netPeerPort, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); + Assert.Equal(netPeerPort, activity.GetTagValue(SemanticConventions.AttributeServerPort)); } - Assert.Equal(netPeerName, activity.GetTagValue(SemanticConventions.AttributeNetPeerName)); + Assert.Equal(netPeerName, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - Assert.Equal(url, activity.GetTagValue(SemanticConventions.AttributeHttpUrl)); + Assert.Equal(url, activity.GetTagValue(SemanticConventions.AttributeUrlFull)); } private static void VerifyActivityStopTags(int statusCode, Activity activity) { - Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); + Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpResponseStatusCode)); } private static void ActivityEnrichment(Activity activity, string method, object obj) diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs index 44d79860889..64166560e0a 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs @@ -101,7 +101,7 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc x => x.Key, x => { - if (x.Key == "http.flavor") + if (x.Key == "network.protocol.version") { return "1.1"; } @@ -127,6 +127,12 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc continue; } + if (tag.Key == SemanticConventions.AttributeErrorType) + { + // TODO: Add validation for error.type in test cases. + continue; + } + Assert.Fail($"Tag {tag.Key} was not found in test data."); } @@ -173,13 +179,13 @@ public void DebugIndividualTest() ""spanKind"": ""Client"", ""setHttpFlavor"": true, ""spanAttributes"": { - ""http.scheme"": ""http"", - ""http.method"": ""GET"", - ""net.peer.name"": ""{host}"", - ""net.peer.port"": ""{port}"", - ""http.flavor"": ""1.1"", - ""http.status_code"": ""200"", - ""http.url"": ""http://{host}:{port}/"" + ""url.scheme"": ""http"", + ""http.request.method"": ""GET"", + ""server.address"": ""{host}"", + ""server.port"": ""{port}"", + ""network.protocol.version"": ""1.1"", + ""http.response.status_code"": ""200"", + ""url.full"": ""http://{host}:{port}/"" } } ", diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json index d808d5c00f2..056a4d56098 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json @@ -7,13 +7,13 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "200", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "200", + "url.full": "http://{host}:{port}/" } }, { @@ -24,13 +24,13 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "POST", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "200", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "POST", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "200", + "url.full": "http://{host}:{port}/" } }, { @@ -42,13 +42,13 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "200", - "http.url": "http://{host}:{port}/path/to/resource/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "200", + "url.full": "http://{host}:{port}/path/to/resource/" } }, { @@ -60,17 +60,17 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "200", - "http.url": "http://{host}:{port}/path/to/resource#fragment" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "200", + "url.full": "http://{host}:{port}/path/to/resource#fragment" } }, { - "name": "http.url must not contain username nor password", + "name": "url.full must not contain username nor password", "method": "GET", "url": "http://username:password@{host}:{port}/path/to/resource#fragment", "responseCode": 200, @@ -78,13 +78,13 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "200", - "http.url": "http://{host}:{port}/path/to/resource#fragment" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "200", + "url.full": "http://{host}:{port}/path/to/resource#fragment" } }, { @@ -96,12 +96,12 @@ "responseExpected": false, "recordException": false, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "sdlfaldfjalkdfjlkajdflkajlsdjf", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "sdlfaldfjalkdfjlkajdflkajlsdjf", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "url.full": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/" } }, { @@ -113,12 +113,12 @@ "responseExpected": false, "recordException": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "sdlfaldfjalkdfjlkajdflkajlsdjf", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "sdlfaldfjalkdfjlkajdflkajlsdjf", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "url.full": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/" } }, { @@ -130,13 +130,13 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "200", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "200", + "url.full": "http://{host}:{port}/" } }, { @@ -148,13 +148,13 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "200", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "200", + "url.full": "http://{host}:{port}/" } }, { @@ -166,13 +166,13 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "399", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "399", + "url.full": "http://{host}:{port}/" } }, { @@ -184,13 +184,13 @@ "spanStatus": "Error", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "400", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "400", + "url.full": "http://{host}:{port}/" } }, { @@ -202,13 +202,13 @@ "spanStatus": "Error", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "401", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "401", + "url.full": "http://{host}:{port}/" } }, { @@ -220,13 +220,13 @@ "spanStatus": "Error", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "403", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "403", + "url.full": "http://{host}:{port}/" } }, { @@ -238,13 +238,13 @@ "spanStatus": "Error", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "404", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "404", + "url.full": "http://{host}:{port}/" } }, { @@ -256,13 +256,13 @@ "spanStatus": "Error", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "429", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "429", + "url.full": "http://{host}:{port}/" } }, { @@ -274,13 +274,13 @@ "spanStatus": "Error", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "501", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "501", + "url.full": "http://{host}:{port}/" } }, { @@ -292,13 +292,13 @@ "spanStatus": "Error", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "503", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "503", + "url.full": "http://{host}:{port}/" } }, { @@ -310,13 +310,13 @@ "spanStatus": "Error", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "504", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "504", + "url.full": "http://{host}:{port}/" } }, { @@ -328,13 +328,13 @@ "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { - "http.scheme": "http", - "http.method": "GET", - "net.peer.name": "{host}", - "net.peer.port": "{port}", - "http.flavor": "{flavor}", - "http.status_code": "200", - "http.url": "http://{host}:{port}/" + "url.scheme": "http", + "http.request.method": "GET", + "server.address": "{host}", + "server.port": "{port}", + "network.protocol.version": "{flavor}", + "http.response.status_code": "200", + "url.full": "http://{host}:{port}/" } } ] diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs index 7f47ce6f6e7..f3a171df6a9 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs @@ -247,6 +247,8 @@ public void MultiThreadedHistogramUpdateAndSnapShotTest() [InlineData("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request.time_in_queue", "s", KnownHistogramBuckets.DefaultShortSeconds)] [InlineData("Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration", "s", KnownHistogramBuckets.DefaultLongSeconds)] [InlineData("Microsoft.AspNetCore.Server.Kestrel", "kestrel.tls_handshake.duration", "s", KnownHistogramBuckets.DefaultShortSeconds)] + [InlineData("OpenTelemetry.Instrumentation.AspNet", "http.server.duration", "ms", KnownHistogramBuckets.Default)] + [InlineData("OpenTelemetry.Instrumentation.AspNet", "http.server.request.duration", "s", KnownHistogramBuckets.DefaultShortSeconds)] [InlineData("OpenTelemetry.Instrumentation.AspNetCore", "http.server.duration", "ms", KnownHistogramBuckets.Default)] [InlineData("OpenTelemetry.Instrumentation.Http", "http.client.duration", "ms", KnownHistogramBuckets.Default)] [InlineData("System.Net.Http", "http.client.connection.duration", "s", KnownHistogramBuckets.DefaultLongSeconds)]