-
Notifications
You must be signed in to change notification settings - Fork 279
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
Changes from 21 commits
957c235
3d3ce2e
e31b687
5677f4f
78e266a
86fdf5a
445a684
a794951
f0ce187
e920468
ccb1d4f
e5d62dc
c9b36e5
ac62f91
7792287
8e846ee
beb1d18
3a108ff
7feba09
87bf97b
a44060b
fdf91b8
c8ced77
fb25884
8d441c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
{ | ||
|
@@ -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) | ||
|
@@ -76,23 +67,41 @@ 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
|
||
|
||
if (request.Url.Port == 80 || request.Url.Port == 443) | ||
var url = request.Url; | ||
activity.SetTag(SemanticConventions.AttributeServerAddress, url.Host); | ||
activity.SetTag(SemanticConventions.AttributeServerPort, url.Port); | ||
activity.SetTag(SemanticConventions.AttributeUrlScheme, url.Scheme); | ||
|
||
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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Kielek - Does spec requires it to be without There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like it doesn't include the |
||
{ | ||
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)); | ||
activity.SetTag(SemanticConventions.AttributeUserAgentOriginal, request.UserAgent); | ||
Kielek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
try | ||
{ | ||
|
@@ -111,7 +120,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) | ||
{ | ||
|
@@ -122,8 +131,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); | ||
} | ||
|
||
|
@@ -148,6 +157,7 @@ private void OnException(Activity activity, HttpContext context, Exception excep | |
} | ||
|
||
activity.SetStatus(ActivityStatusCode.Error, exception.Message); | ||
activity.SetTag(SemanticConventions.AttributeErrorType, exception.GetType().FullName); | ||
|
||
try | ||
{ | ||
|
There was a problem hiding this comment.
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