-
Notifications
You must be signed in to change notification settings - Fork 765
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
update HttpSemanticConventions for Instrumentation.AspNetCore #4537
Changes from all commits
c60216f
8b892ac
9b5c800
d47b1f7
9215a07
f78aedf
b1fc737
4de47b4
357af6a
f2203e3
352f939
d03a1a5
14250da
cbd15c7
f2716a2
96ca6b0
85a9120
86ab41c
4da2eb7
808646b
406b24f
8796539
99eabb8
0933da2
8f7934c
2a6d88c
bf807eb
3020b62
e02b8c7
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 |
---|---|---|
|
@@ -2,6 +2,11 @@ | |
|
||
## Unreleased | ||
|
||
* Updated [Http Semantic Conventions](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/trace/semantic_conventions/http.md). | ||
* This library can emit either old, new, or both attributes. Users can control | ||
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. We don't need a sub-bullet here. This change is simple enough to be put under the same bullet. |
||
which attributes are emitted by setting the environment variable | ||
`OTEL_SEMCONV_STABILITY_OPT_IN`. | ||
|
||
## 1.5.0-beta.1 | ||
|
||
Released 2023-Jun-05 | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -82,20 +82,44 @@ public override void OnEventWritten(string name, object payload) | |||
|
||||
TagList tags = default; | ||||
|
||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol))); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpScheme, context.Request.Scheme)); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpMethod, context.Request.Method)); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode))); | ||||
// see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md | ||||
if (this.httpSemanticConvention.HasFlag(HttpSemanticConvention.Old)) | ||||
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. Some thoughts...
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.
+1, I had similar concern here #4538 (comment) 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.
How strongly do you feel about this? :) Also, this is meant to be short lived. When these instrumentation libraries go stable this enum and helper class will be removed.
Can you please share an example of this?
That makes sense to me, but the spec defines OTEL_SEMCONV_STABILITY_OPT_IN 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.
The instrumentation libraries do not use this Note: This does not have to be done in this PR. |
||||
{ | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol))); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpScheme, context.Request.Scheme)); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpMethod, context.Request.Method)); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode))); | ||||
|
||||
if (context.Request.Host.HasValue) | ||||
{ | ||||
tags.Add(new KeyValuePair<string, object>(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<string, object>(SemanticConventions.AttributeNetHostPort, context.Request.Host.Port)); | ||||
} | ||||
} | ||||
} | ||||
|
||||
if (context.Request.Host.HasValue) | ||||
// see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/trace/semantic_conventions/http.md | ||||
if (this.httpSemanticConvention.HasFlag(HttpSemanticConvention.New)) | ||||
{ | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetHostName, context.Request.Host.Host)); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol))); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, context.Request.Scheme)); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, context.Request.Method)); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode))); | ||||
|
||||
if (context.Request.Host.Port is not null && context.Request.Host.Port != 80 && context.Request.Host.Port != 443) | ||||
if (context.Request.Host.HasValue) | ||||
{ | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetHostPort, context.Request.Host.Port)); | ||||
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerAddress, 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<string, object>(SemanticConventions.AttributeServerPort, context.Request.Host.Port)); | ||||
} | ||||
} | ||||
} | ||||
|
||||
#if NET6_0_OR_GREATER | ||||
var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; | ||||
if (!string.IsNullOrEmpty(route)) | ||||
|
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.
// replaces: "net.peer.port" and "net.host.port"