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.

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

[Instrumentation.AspNet] Spans - semantic convention v1.24.0 #1607

Merged
merged 25 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
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
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
* **Breaking Change**: `server.address` and `server.port` no longer added
for `http.server.request.duration` metric.
([#1606](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1606))
* **Breaking change** Spans names and attributes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please include the details on new versus old attributes?

Something like this: https://opentelemetry.io/blog/2023/http-conventions-declared-stable/#summary-of-changes

based on [HTTP semantic convention v1.24.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md).
([#1607](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1607))

## 1.7.0-beta.2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ internal sealed class HttpInListener : IDisposable
{
private readonly HttpRequestRouteHelper routeHelper = new();
private readonly AspNetTraceInstrumentationOptions options;
private readonly RequestDataHelper requestDataHelper = new();

public HttpInListener(AspNetTraceInstrumentationOptions options)
{
Expand All @@ -35,16 +36,6 @@ public void Dispose()
TelemetryHttpModule.Options.OnExceptionCallback -= this.OnException;
}

/// <summary>
/// Gets the OpenTelemetry standard uri tag value for a span based on its request <see cref="Uri"/>.
/// </summary>
/// <param name="uri"><see cref="Uri"/>.</param>
/// <returns>Span uri value.</returns>
private static string GetUriTagValueFromRequestUri(Uri uri)
{
return string.IsNullOrEmpty(uri.UserInfo) ? uri.ToString() : string.Concat(uri.Scheme, Uri.SchemeDelimiter, uri.Authority, uri.PathAndQuery, uri.Fragment);
}

private void OnStartActivity(Activity activity, HttpContext context)
{
if (activity.IsAllDataRequested)
Expand Down Expand Up @@ -76,23 +67,45 @@ private void OnStartActivity(Activity activity, HttpContext context)
var request = context.Request;
var requestValues = request.Unvalidated;

// see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md
var path = requestValues.Path;
activity.DisplayName = path;
// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md
var originalHttpMethod = request.HttpMethod;
var normalizedHttpMethod = this.requestDataHelper.GetNormalizedHttpMethod(originalHttpMethod);
activity.DisplayName = normalizedHttpMethod == "_OTHER" ? "HTTP" : normalizedHttpMethod;
Kielek marked this conversation as resolved.
Show resolved Hide resolved

var url = request.Url;
activity.SetTag(SemanticConventions.AttributeServerAddress, url.Host);
activity.SetTag(SemanticConventions.AttributeServerPort, url.Port);
activity.SetTag(SemanticConventions.AttributeUrlScheme, url.Scheme);

if (request.Url.Port == 80 || request.Url.Port == 443)
this.requestDataHelper.SetHttpMethodTag(activity, originalHttpMethod);

var protocolVersion = RequestDataHelper.GetHttpProtocolVersion(request);
if (!string.IsNullOrEmpty(protocolVersion))
{
activity.SetTag(SemanticConventions.AttributeHttpHost, request.Url.Host);
activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion);
}
else

activity.SetTag(SemanticConventions.AttributeUrlPath, requestValues.Path);

// TODO url.query should be sanitized
var query = url.Query;
if (!string.IsNullOrEmpty(query))
{
activity.SetTag(SemanticConventions.AttributeHttpHost, request.Url.Host + ":" + request.Url.Port);
if (query.StartsWith("?", StringComparison.InvariantCulture))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kielek - Does spec requires it to be without ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if it hard requierment, but all examples are without this sign.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it doesn't include the ? component, https://www.rfc-editor.org/rfc/rfc3986#section-3.4

{
activity.SetTag(SemanticConventions.AttributeUrlQuery, query.Substring(1));
}
else
{
activity.SetTag(SemanticConventions.AttributeUrlQuery, query);
}
}

activity.SetTag(SemanticConventions.AttributeHttpMethod, request.HttpMethod);
activity.SetTag(SemanticConventions.AttributeHttpTarget, path);
activity.SetTag(SemanticConventions.AttributeHttpUserAgent, request.UserAgent);
activity.SetTag(SemanticConventions.AttributeHttpUrl, GetUriTagValueFromRequestUri(request.Url));
var userAgent = request.UserAgent;
if (!string.IsNullOrEmpty(userAgent))
{
activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, userAgent);
}

try
{
Expand All @@ -111,7 +124,7 @@ private void OnStopActivity(Activity activity, HttpContext context)
{
var response = context.Response;

activity.SetTag(SemanticConventions.AttributeHttpStatusCode, response.StatusCode);
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, response.StatusCode);

if (activity.Status == ActivityStatusCode.Unset)
{
Expand All @@ -122,8 +135,8 @@ private void OnStopActivity(Activity activity, HttpContext context)

if (!string.IsNullOrEmpty(template))
{
// Override the name that was previously set to the path part of URL.
activity.DisplayName = template!;
// Override the name that was previously set to the normalized HTTP method/HTTP
activity.DisplayName = $"{activity.DisplayName} {template!}";
activity.SetTag(SemanticConventions.AttributeHttpRoute, template);
}

Expand All @@ -148,6 +161,7 @@ private void OnException(Activity activity, HttpContext context, Exception excep
}

activity.SetStatus(ActivityStatusCode.Error, exception.Message);
activity.SetTag(SemanticConventions.AttributeErrorType, exception.GetType().FullName);

try
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation;
internal sealed class HttpInMetricsListener : IDisposable
{
private readonly HttpRequestRouteHelper routeHelper = new();
private readonly RequestMethodHelper requestMethodHelper = new();
private readonly RequestDataHelper requestDataHelper = new();
private readonly Histogram<double> httpServerDuration;
private readonly AspNetMetricsInstrumentationOptions options;

Expand All @@ -31,18 +31,6 @@ public void Dispose()
TelemetryHttpModule.Options.OnRequestStoppedCallback -= this.OnStopActivity;
}

private static string GetHttpProtocolVersion(HttpRequest request)
{
var protocol = request.ServerVariables["SERVER_PROTOCOL"];
return protocol switch
{
"HTTP/1.1" => "1.1",
"HTTP/2" => "2",
"HTTP/3" => "3",
_ => protocol,
};
}

private void OnStopActivity(Activity activity, HttpContext context)
{
var request = context.Request;
Expand All @@ -59,10 +47,10 @@ private void OnStopActivity(Activity activity, HttpContext context)
tags.Add(SemanticConventions.AttributeServerPort, url.Port);
}

var normalizedMethod = this.requestMethodHelper.GetNormalizedHttpMethod(request.HttpMethod);
var normalizedMethod = this.requestDataHelper.GetNormalizedHttpMethod(request.HttpMethod);
tags.Add(SemanticConventions.AttributeHttpRequestMethod, normalizedMethod);

var protocolVersion = GetHttpProtocolVersion(request);
var protocolVersion = RequestDataHelper.GetHttpProtocolVersion(request);
if (!string.IsNullOrEmpty(protocolVersion))
{
tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Web;
using OpenTelemetry.Trace;

namespace OpenTelemetry.Instrumentation.AspNet.Implementation;

internal sealed class RequestMethodHelper
internal sealed class RequestDataHelper
{
private const string KnownHttpMethodsEnvironmentVariable = "OTEL_INSTRUMENTATION_HTTP_KNOWN_METHODS";

Expand All @@ -20,7 +23,7 @@ internal sealed class RequestMethodHelper
// List of known HTTP methods as per spec.
private readonly Dictionary<string, string> knownHttpMethods;

public RequestMethodHelper()
public RequestDataHelper()
{
var suppliedKnownMethods = Environment.GetEnvironmentVariable(KnownHttpMethodsEnvironmentVariable)
?.Split(SplitChars, StringSplitOptions.RemoveEmptyEntries);
Expand All @@ -40,10 +43,37 @@ public RequestMethodHelper()
};
}

public static string GetHttpProtocolVersion(HttpRequest request)
{
return GetHttpProtocolVersion(request.ServerVariables["SERVER_PROTOCOL"]);
}

public void SetHttpMethodTag(Activity activity, string originalHttpMethod)
{
var normalizedHttpMethod = this.GetNormalizedHttpMethod(originalHttpMethod);
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, normalizedHttpMethod);

if (originalHttpMethod != normalizedHttpMethod)
{
activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, originalHttpMethod);
}
}

public string GetNormalizedHttpMethod(string method)
{
return this.knownHttpMethods.TryGetValue(method, out var normalizedMethod)
? normalizedMethod
: OtherHttpMethod;
}

internal static string GetHttpProtocolVersion(string protocol)
{
return protocol switch
{
"HTTP/1.1" => "1.1",
"HTTP/2" => "2",
"HTTP/3" => "3",
_ => protocol,
};
}
}
5 changes: 5 additions & 0 deletions src/Shared/SemanticConventions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,22 @@ internal static class SemanticConventions
public const string AttributeExceptionType = "exception.type";
public const string AttributeExceptionMessage = "exception.message";
public const string AttributeExceptionStacktrace = "exception.stacktrace";
public const string AttributeErrorType = "error.type";

// v1.21.0
// https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md#http-server
public const string AttributeHttpRequestMethod = "http.request.method"; // replaces: "http.method" (AttributeHttpMethod)
public const string AttributeHttpRequestMethodOriginal = "http.request.method_original";
public const string AttributeHttpResponseStatusCode = "http.response.status_code"; // replaces: "http.status_code" (AttributeHttpStatusCode)
public const string AttributeUrlScheme = "url.scheme"; // replaces: "http.scheme" (AttributeHttpScheme)
public const string AttributeUrlPath = "url.path"; // replaces: "http.target" (AttributeHttpTarget)
public const string AttributeUrlQuery = "url.query"; // replaces: "http.target" (AttributeHttpTarget)

// v1.23.0
// https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md#http-server
public const string AttributeNetworkProtocolVersion = "network.protocol.version"; // replaces: "http.flavor" (AttributeHttpFlavor)
public const string AttributeServerAddress = "server.address"; // replaces: "net.host.name" (AttributeNetHostName)
public const string AttributeServerPort = "server.port"; // replaces: "net.host.port" (AttributeNetHostPort)
public const string AttributeUserAgentOriginal = "user_agent.original"; // replaces: http.user_agent (AttributeHttpUserAgent)
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
}
Loading
Loading