Skip to content

Commit

Permalink
[HttpClient & HttpWebRequest] Set network protocol version from respo…
Browse files Browse the repository at this point in the history 
…nse object (#5043)
  • Loading branch information
@vishweshbankwar
vishweshbankwar authored Nov 15, 2023
1 parent c337d1e commit e904994
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 23 deletions.
6 changes: 6 additions & 0 deletions src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@
* Fixed `network.protocol.version` attribute values to match the specification.
([#5006](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5006))

* Set `network.protocol.version` value using the protocol version on the
received response. If the request fails without response, then
`network.protocol.version` attribute will not be set on Activity and
`http.client.request.duration` metric.
([#5043](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5043))

## 1.6.0-beta.2

Released 2023-Oct-26
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ public void OnStartActivity(Activity activity, object payload)
}

activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri));
activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(request.Version));

if (request.Headers.TryGetValues("User-Agent", out var userAgentValues))
{
Expand Down Expand Up @@ -283,6 +282,7 @@ public void OnStopActivity(Activity activity, object payload)

if (this.emitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.Version));
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
if (activity.Status == ActivityStatusCode.Error)
{
Expand Down
2 changes: 1 addition & 1 deletion ...OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ public void OnStopEventWritten(Activity activity, object payload)

tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerAddress, request.RequestUri.Host));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(request.Version)));

if (!request.RequestUri.IsDefaultPort)
{
Expand All @@ -139,6 +138,7 @@ public void OnStopEventWritten(Activity activity, object payload)

if (TryFetchResponse(payload, out HttpResponseMessage response))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.Version)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)));

// Set error.type to status code for failed requests
Expand Down
19 changes: 15 additions & 4 deletions src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A
}

activity.SetTag(SemanticConventions.AttributeUrlFull, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri));
activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(request.ProtocolVersion));
}

try
Expand Down Expand Up @@ -201,6 +200,7 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity)

if (tracingEmitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(response.ProtocolVersion));
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
}

Expand Down Expand Up @@ -243,11 +243,12 @@ private static string GetErrorType(Exception exception)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void AddExceptionTags(Exception exception, Activity activity, out HttpStatusCode? statusCode)
private static void AddExceptionTags(Exception exception, Activity activity, out HttpStatusCode? statusCode, out Version protocolVersion)
{
Debug.Assert(activity != null, "Activity must not be null");

statusCode = null;
protocolVersion = null;

if (!activity.IsAllDataRequested)
{
Expand All @@ -259,6 +260,7 @@ private static void AddExceptionTags(Exception exception, Activity activity, out
if (exception is WebException wexc && wexc.Response is HttpWebResponse response)
{
statusCode = response.StatusCode;
protocolVersion = response.ProtocolVersion;

if (tracingEmitOldAttributes)
{
Expand All @@ -267,6 +269,7 @@ private static void AddExceptionTags(Exception exception, Activity activity, out

if (tracingEmitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(protocolVersion));
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode);
}

Expand Down Expand Up @@ -397,6 +400,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
{
HttpStatusCode? httpStatusCode = null;
string errorType = null;
Version protocolVersion = null;

// Activity may be null if we are not tracing in these cases:
// 1. No listeners
Expand All @@ -415,11 +419,12 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
errorType = GetErrorType(ex);
if (activity != null)
{
AddExceptionTags(ex, activity, out httpStatusCode);
AddExceptionTags(ex, activity, out httpStatusCode, out protocolVersion);
}
else if (ex is WebException wexc && wexc.Response is HttpWebResponse response)
{
httpStatusCode = response.StatusCode;
protocolVersion = response.ProtocolVersion;
}
}
else
Expand Down Expand Up @@ -447,6 +452,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
}

httpStatusCode = responseCopy.StatusCode;
protocolVersion = responseCopy.ProtocolVersion;
}
else
{
Expand All @@ -456,6 +462,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
}

httpStatusCode = response.StatusCode;
protocolVersion = response.ProtocolVersion;
}

if (SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)httpStatusCode.Value) == ActivityStatusCode.Error)
Expand Down Expand Up @@ -531,7 +538,11 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC

tags.Add(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
tags.Add(SemanticConventions.AttributeUrlScheme, request.RequestUri.Scheme);
tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(request.ProtocolVersion));
if (protocolVersion != null)
{
tags.Add(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(protocolVersion));
}

if (!request.RequestUri.IsDefaultPort)
{
tags.Add(SemanticConventions.AttributeServerPort, request.RequestUri.Port);
Expand Down
32 changes: 15 additions & 17 deletions test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,13 +339,13 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(

var normalizedAttributes = activity.TagObjects.Where(kv => !kv.Key.StartsWith("otel.")).ToDictionary(x => x.Key, x => x.Value.ToString());

int numberOfNewTags = activity.Status == ActivityStatusCode.Error ? 6 : 5;
int numberOfDupeTags = activity.Status == ActivityStatusCode.Error ? 12 : 11;
int numberOfNewTags = activity.Status == ActivityStatusCode.Error ? 5 : 4;
int numberOfDupeTags = activity.Status == ActivityStatusCode.Error ? 11 : 10;

var expectedAttributeCount = semanticConvention == HttpSemanticConvention.Dupe
? numberOfDupeTags + (tc.ResponseExpected ? 2 : 0)
? numberOfDupeTags + (tc.ResponseExpected ? 3 : 0)
: semanticConvention == HttpSemanticConvention.New
? numberOfNewTags + (tc.ResponseExpected ? 1 : 0)
? numberOfNewTags + (tc.ResponseExpected ? 2 : 0)
: 6 + (tc.ResponseExpected ? 1 : 0);

Assert.Equal(expectedAttributeCount, normalizedAttributes.Count);
Expand Down Expand Up @@ -374,9 +374,9 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]);
Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]);
Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeUrlFull && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpUrl]);
Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]);
if (tc.ResponseExpected)
{
Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]);
Assert.Contains(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]);

if (tc.ResponseCode >= 400)
Expand All @@ -387,6 +387,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
else
{
Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode);
Assert.DoesNotContain(normalizedAttributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion);

#if NET8_0_OR_GREATER
// we are using fake address so it will be "name_resolution_error"
Expand Down Expand Up @@ -532,33 +533,29 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
attributes[tag.Key] = tag.Value;
}

#if !NET8_0_OR_GREATER
var numberOfTags = 6;
#else
// network.protocol.version is not emitted when response if not received.
// https://github.com/open-telemetry/opentelemetry-dotnet/issues/4928
var numberOfTags = 5;
#endif
var numberOfTags = 4;
if (tc.ResponseExpected)
{
var expectedStatusCode = int.Parse(normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]);
numberOfTags = (expectedStatusCode >= 400) ? 6 : 5;
numberOfTags = (expectedStatusCode >= 400) ? 5 : 4; // error.type extra tag
}
else
{
numberOfTags = 5; // error.type would be extra
}

var expectedAttributeCount = numberOfTags + (tc.ResponseExpected ? 1 : 0);
var expectedAttributeCount = numberOfTags + (tc.ResponseExpected ? 2 : 0); // responsecode + protocolversion

Assert.Equal(expectedAttributeCount, attributes.Count);

Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpRequestMethod && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpMethod]);
Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerAddress && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerName]);
Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeServerPort && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeNetPeerPort]);
Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeUrlScheme && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpScheme]);
#if !NET8_0_OR_GREATER
Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]);
#endif

if (tc.ResponseExpected)
{
Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpFlavor]);
Assert.Contains(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode && kvp.Value.ToString() == normalizedAttributesTestCase[SemanticConventions.AttributeHttpStatusCode]);

if (tc.ResponseCode >= 400)
Expand All @@ -568,6 +565,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
}
else
{
Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeNetworkProtocolVersion);
Assert.DoesNotContain(attributes, kvp => kvp.Key == SemanticConventions.AttributeHttpResponseStatusCode);

#if NET8_0_OR_GREATER
Expand Down

0 comments on commit e904994

Please sign in to comment.