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

update HttpSemanticConventions for Instrumentation.Http #4538

Merged
merged 9 commits into from
Jun 22, 2023
Merged

update HttpSemanticConventions for Instrumentation.Http #4538

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d55768a
update HttpSemanticConventions for Instrumentation.Http
TimothyMothra Jun 1, 2023
08bc9d7
comment
TimothyMothra Jun 7, 2023
a313e92
include UserAgent
TimothyMothra Jun 7, 2023
e74a54f
fix typo
TimothyMothra Jun 9, 2023
f520aaa
update semantic conventions comments
TimothyMothra Jun 19, 2023
53cfa60
update url with correct tag
TimothyMothra Jun 19, 2023
2c72f8f
Merge branch 'main' into 4484_http
TimothyMothra Jun 19, 2023
f05cb51
merge main
TimothyMothra Jun 21, 2023
a3d4a9a
update changelog
TimothyMothra Jun 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
10 changes: 10 additions & 0 deletions src/OpenTelemetry.Api/Internal/SemanticConventions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,5 +110,15 @@ internal static class SemanticConventions
public const string AttributeExceptionType = "exception.type";
public const string AttributeExceptionMessage = "exception.message";
public const string AttributeExceptionStacktrace = "exception.stacktrace";

// NEW v1.21.0 Http Semantic Conventions
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean?

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 was asked to separate the new attributes from the others and give it a descriptive comment.
The word "new" isn't needed here, sorry for the confusion.
I've updated this comment in the AspNetCore PR but forgot to update it here.

Copy link
Member

Choose a reason for hiding this comment

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

I was asked to separate the new attributes from the others and give it a descriptive comment.

What does v1.21.0 mean?

The word "new" isn't needed here

👍

public const string AttributeHttpRequestMethod = "http.request.method";
public const string AttributeHttpResponseStatusCode = "http.response.status_code";
public const string AttributeNetworkProtocolVersion = "network.protocol.version";
public const string AttributeServerAddress = "server.address";
public const string AttributeServerPort = "server.port";
public const string AttributeUrlFull = "url.full";
public const string AttributeUrlScheme = "url.scheme";
public const string AttributeUserAgentOriginal = "user_agent.original";
}
}
52 changes: 44 additions & 8 deletions src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,44 @@ public void OnStartActivity(Activity activity, object payload)
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client);
}

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)
// 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))
{
activity.SetTag(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port);
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.AttributeHttpUrl, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri));
activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version));
// see the spec https://github.com/open-telemetry/semantic-conventions/blob/main/specification/trace/semantic_conventions/http.md
TimothyMothra marked this conversation as resolved.
Show resolved Hide resolved
if (this.httpSemanticConvention.HasFlag(HttpSemanticConvention.New))
{
activity.SetTag(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme);
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, HttpTagHelper.GetNameForHttpMethod(request.Method));
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.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version));

if (request.Headers.TryGetValues("User-Agent", out var userAgentValues))
{
var userAgent = userAgentValues.FirstOrDefault();
Copy link
Contributor

Choose a reason for hiding this comment

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

This would cause an allocation. Refer to how AspNetCore instrumentation handles this: https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs#L217-L224

This should be done in a separate PR which updates the benchmarks as well.

if (!string.IsNullOrEmpty(userAgent))
{
activity.SetTag(SemanticConventions.AttributeHttpUserAgent, userAgent);
}
}
}

try
{
Expand Down Expand Up @@ -214,7 +242,15 @@ public void OnStopActivity(Activity activity, object payload)

if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
{
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
if (this.httpSemanticConvention.HasFlag(HttpSemanticConvention.Old))
{
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
}

if (this.httpSemanticConvention.HasFlag(HttpSemanticConvention.New))
{
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
}

if (currentStatusCode == ActivityStatusCode.Unset)
{
Expand Down
40 changes: 32 additions & 8 deletions ...OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,43 @@ public override void OnEventWritten(string name, object payload)
if (this.stopRequestFetcher.TryFetch(payload, out HttpRequestMessage request) && request != null)
{
TagList tags = default;
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host));

if (!request.RequestUri.IsDefaultPort)
// 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))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host));

if (!request.RequestUri.IsDefaultPort)
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port));
}

if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)));
}
}

if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
// see the spec https://github.com/open-telemetry/semantic-conventions/blob/main/specification/trace/semantic_conventions/http.md
if (this.httpSemanticConvention.HasFlag(HttpSemanticConvention.New))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, HttpTagHelper.GetNameForHttpMethod(request.Method)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerAddress, request.RequestUri.Host));

if (!request.RequestUri.IsDefaultPort)
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerPort, request.RequestUri.Port));
}

if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)));
}
}

// We are relying here on HttpClient library to set duration before writing the stop event.
Expand Down