Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ASP.NET Core] Remove OTEL_SEMCONV_STABILITY_OPT_IN #5066

Merged
merged 8 commits into from
Nov 21, 2023
Merged

[ASP.NET Core] Remove OTEL_SEMCONV_STABILITY_OPT_IN #5066

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dab71bc
[ASP.NET Core]Remove OTEL_SEMCONV_STABILITY_OPT_IN
vishweshbankwar Nov 20, 2023
5197005
Merge branch 'main' into vibankwa/remove-semantic-conventions-opt-in-…
vishweshbankwar Nov 21, 2023
2b036bf
changelog
vishweshbankwar Nov 21, 2023
118cb34
Update CHANGELOG.md
vishweshbankwar Nov 21, 2023
e6d4968
Merge branch 'main' into vibankwa/remove-semantic-conventions-opt-in-…
vishweshbankwar Nov 21, 2023
b799d63
Merge branch 'main' into vibankwa/remove-semantic-conventions-opt-in-…
utpilla Nov 21, 2023
7725836
fix test
vishweshbankwar Nov 21, 2023
765cf8e
Merge branch 'vibankwa/remove-semantic-conventions-opt-in-aspnetcore'…
vishweshbankwar Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
using System.Diagnostics.Metrics;
using System.Reflection;
using OpenTelemetry.Instrumentation.AspNetCore.Implementation;
using OpenTelemetry.Internal;

namespace OpenTelemetry.Instrumentation.AspNetCore;

Expand Down Expand Up @@ -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();
}
Expand Down
45 changes: 0 additions & 45 deletions src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs
View file
Open in desktop

This file was deleted.

6 changes: 6 additions & 0 deletions src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

* Removed support for `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. The
library will now emit
[stable](https://github.com/open-telemetry/semantic-conventions/tree/v1.23.0/docs/http)
semantic conventions by default.
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
([#5066](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5066))

## 1.6.0-beta.3

Released 2023-Nov-17
Expand Down
118 changes: 27 additions & 91 deletions src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#endif
using OpenTelemetry.Internal;
using OpenTelemetry.Trace;
using static OpenTelemetry.Internal.HttpSemanticConventionHelper;

namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation;

Expand Down Expand Up @@ -66,19 +65,13 @@ internal class HttpInListener : ListenerHandler
private readonly PropertyFetcher<string> beforeActionTemplateFetcher = new("Template");
#endif
private readonly AspNetCoreInstrumentationOptions options;
private readonly bool emitOldAttributes;
private readonly bool emitNewAttributes;

public HttpInListener(AspNetCoreInstrumentationOptions options)
: base(DiagnosticSourceName)
{
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)
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand All @@ -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)
{
Expand Down
Loading