diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index e4be1f9a388..7eefd7d311a 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* **Breaking change** `http.host` will no longer be populated. `net.peer.name` + and `net.peer.port` attributes will be populated instead. + ([#3832](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3832)) + ## 1.0.0-rc9.9 Released 2022-Nov-07 diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index 5e0b2fae944..f2ac51c6432 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -159,7 +159,12 @@ public void OnStartActivity(Activity activity, object payload) activity.SetTag(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme); activity.SetTag(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method)); - activity.SetTag(SemanticConventions.AttributeHttpHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri)); + 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)); diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs index 52bf6dada5c..6616029ef4e 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpTagHelper.cs @@ -27,7 +27,6 @@ internal static class HttpTagHelper private static readonly ConcurrentDictionary MethodOperationNameCache = new(); private static readonly ConcurrentDictionary HttpMethodOperationNameCache = new(); private static readonly ConcurrentDictionary HttpMethodNameCache = new(); - private static readonly ConcurrentDictionary> HostAndPortToStringCache = new(); private static readonly ConcurrentDictionary ProtocolVersionToStringCache = new(); private static readonly Func ConvertMethodToOperationNameRef = ConvertMethodToOperationName; @@ -63,37 +62,6 @@ internal static class HttpTagHelper /// Span flavor value. public static string GetFlavorTagValueFromProtocolVersion(Version protocolVersion) => ProtocolVersionToStringCache.GetOrAdd(protocolVersion, ConvertProtocolVersionToStringRef); - /// - /// Gets the OpenTelemetry standard host tag value for a span based on its request . - /// - /// . - /// Span host value. - public static string GetHostTagValueFromRequestUri(Uri requestUri) - { - string host = requestUri.Host; - - if (requestUri.IsDefaultPort) - { - return host; - } - - int port = requestUri.Port; - - if (!HostAndPortToStringCache.TryGetValue(host, out ConcurrentDictionary portCache)) - { - portCache = new ConcurrentDictionary(); - HostAndPortToStringCache.TryAdd(host, portCache); - } - - if (!portCache.TryGetValue(port, out string hostTagValue)) - { - hostTagValue = $"{requestUri.Host}:{requestUri.Port}"; - portCache.TryAdd(port, hostTagValue); - } - - return hostTagValue; - } - /// /// Gets the OpenTelemetry standard uri tag value for a span based on its request . /// diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 078997b1b10..b011719cb6f 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -99,8 +99,13 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A if (activity.IsAllDataRequested) { activity.SetTag(SemanticConventions.AttributeHttpMethod, request.Method); + activity.SetTag(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host); + if (!request.RequestUri.IsDefaultPort) + { + activity.SetTag(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port); + } + activity.SetTag(SemanticConventions.AttributeHttpScheme, request.RequestUri.Scheme); - activity.SetTag(SemanticConventions.AttributeHttpHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri)); activity.SetTag(SemanticConventions.AttributeHttpUrl, HttpTagHelper.GetUriTagValueFromRequestUri(request.RequestUri)); activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion)); diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index 3798b48328f..5e4d644450c 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -231,7 +231,8 @@ public async Task DebugIndividualTestAsync() ""spanAttributes"": { ""http.scheme"": ""http"", ""http.method"": ""GET"", - ""http.host"": ""{host}:{port}"", + ""net.peer.name"": ""{host}"", + ""net.peer.port"": ""{port}"", ""http.status_code"": ""399"", ""http.flavor"": ""{flavor}"", ""http.url"": ""http://{host}:{port}/"" diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs index f03709ea7ad..42eabb95b8b 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs @@ -39,6 +39,8 @@ public class HttpWebRequestActivitySourceTests : IDisposable private readonly string testServerHost; private readonly int testServerPort; private readonly string hostNameAndPort; + private readonly string netPeerName; + private readonly int netPeerPort; static HttpWebRequestActivitySourceTests() { @@ -74,6 +76,8 @@ public HttpWebRequestActivitySourceTests() out this.testServerPort); this.hostNameAndPort = $"{this.testServerHost}:{this.testServerPort}"; + this.netPeerName = this.testServerHost; + this.netPeerPort = this.testServerPort; void ProcessServerRequest(HttpListenerContext context) { @@ -198,7 +202,7 @@ public async Task TestBasicReceiveAndResponseEvents(string method, string queryS // Check to make sure: The first record must be a request, the next record must be a response. Activity activity = AssertFirstEventWasStart(eventRecords); - VerifyActivityStartTags(this.hostNameAndPort, method, url, activity); + VerifyActivityStartTags(this.netPeerName, this.netPeerPort, method, url, activity); Assert.True(eventRecords.Records.TryDequeue(out var stopEvent)); Assert.Equal("Stop", stopEvent.Key); @@ -378,7 +382,7 @@ public async Task TestBasicReceiveAndResponseWebRequestEvents(string method, int // Check to make sure: The first record must be a request, the next record must be a response. Activity activity = AssertFirstEventWasStart(eventRecords); - VerifyActivityStartTags(this.hostNameAndPort, method, url, activity); + VerifyActivityStartTags(this.netPeerName, this.netPeerPort, method, url, activity); Assert.True(eventRecords.Records.TryDequeue(out var stopEvent)); Assert.Equal("Stop", stopEvent.Key); @@ -484,7 +488,7 @@ public async Task TestResponseWithoutContentEvents(string method) // Check to make sure: The first record must be a request, the next record must be a response. Activity activity = AssertFirstEventWasStart(eventRecords); - VerifyActivityStartTags(this.hostNameAndPort, method, url, activity); + VerifyActivityStartTags(this.netPeerName, this.netPeerPort, method, url, activity); Assert.True(eventRecords.Records.TryDequeue(out var stopEvent)); Assert.Equal("Stop", stopEvent.Key); @@ -523,9 +527,10 @@ public async Task TestRedirectedRequest(string method) [InlineData("POST")] public async Task TestRequestWithException(string method) { + string host = Guid.NewGuid().ToString() + ".com"; string url = method == "GET" - ? $"http://{Guid.NewGuid()}.com" - : $"http://{Guid.NewGuid()}.com"; + ? $"http://{host}" + : $"http://{host}"; using var eventRecords = new ActivitySourceRecorder(); @@ -548,7 +553,7 @@ public async Task TestRequestWithException(string method) // Check to make sure: The first record must be a request, the next record must be an exception. Activity activity = AssertFirstEventWasStart(eventRecords); - VerifyActivityStartTags(null, method, url, activity); + VerifyActivityStartTags(host, null, method, url, activity); Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair exceptionEvent)); Assert.Equal("Stop", exceptionEvent.Key); @@ -587,7 +592,7 @@ public async Task TestCanceledRequest(string method) Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Stop")); Activity activity = AssertFirstEventWasStart(eventRecords); - VerifyActivityStartTags(this.hostNameAndPort, method, url, activity); + VerifyActivityStartTags(this.netPeerName, this.netPeerPort, method, url, activity); Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair exceptionEvent)); Assert.Equal("Stop", exceptionEvent.Key); @@ -626,7 +631,7 @@ public async Task TestSecureTransportFailureRequest(string method) Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Stop")); Activity activity = AssertFirstEventWasStart(eventRecords); - VerifyActivityStartTags(null, method, url, activity); + VerifyActivityStartTags("expired.badssl.com", null, method, url, activity); Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair exceptionEvent)); Assert.Equal("Stop", exceptionEvent.Key); @@ -668,7 +673,7 @@ public async Task TestSecureTransportRetryFailureRequest(string method) Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Stop")); Activity activity = AssertFirstEventWasStart(eventRecords); - VerifyActivityStartTags(this.hostNameAndPort, method, url, activity); + VerifyActivityStartTags(this.netPeerName, this.netPeerPort, method, url, activity); Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair exceptionEvent)); Assert.Equal("Stop", exceptionEvent.Key); @@ -778,15 +783,17 @@ private static void VerifyHeaders(HttpWebRequest startRequest) Assert.Matches("^[0-9a-f]{2}-[0-9a-f]{32}-[0-9a-f]{16}-[0-9a-f]{2}$", traceparent); } - private static void VerifyActivityStartTags(string hostNameAndPort, string method, string url, Activity activity) + private static void VerifyActivityStartTags(string netPeerName, int? netPeerPort, string method, string url, Activity activity) { Assert.NotNull(activity.TagObjects); Assert.Equal(method, activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); - if (hostNameAndPort != null) + if (netPeerPort != null) { - Assert.Equal(hostNameAndPort, activity.GetTagValue(SemanticConventions.AttributeHttpHost)); + Assert.Equal(netPeerPort, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); } + Assert.Equal(netPeerName, activity.GetTagValue(SemanticConventions.AttributeNetPeerName)); + Assert.Equal(url, activity.GetTagValue(SemanticConventions.AttributeHttpUrl)); } diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs index 61f11102d7b..d447606a47e 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs @@ -182,7 +182,8 @@ public void DebugIndividualTest() ""spanAttributes"": { ""http.scheme"": ""http"", ""http.method"": ""GET"", - ""http.host"": ""{host}:{port}"", + ""net.peer.name"": ""{host}"", + ""net.peer.port"": ""{port}"", ""http.flavor"": ""1.1"", ""http.status_code"": ""200"", ""http.url"": ""http://{host}:{port}/"" diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json index f389d393422..c9f68f4698b 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json @@ -9,7 +9,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "200", "http.url": "http://{host}:{port}/" @@ -25,7 +26,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "POST", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "200", "http.url": "http://{host}:{port}/" @@ -42,7 +44,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "200", "http.url": "http://{host}:{port}/path/to/resource/" @@ -59,7 +62,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "200", "http.url": "http://{host}:{port}/path/to/resource#fragment" @@ -76,7 +80,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "200", "http.url": "http://{host}:{port}/path/to/resource#fragment" @@ -94,7 +99,7 @@ "spanAttributes": { "http.scheme": "https", "http.method": "GET", - "http.host": "sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com", + "net.peer.name": "sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com", "http.flavor": "{flavor}", "http.url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/" } @@ -111,7 +116,7 @@ "spanAttributes": { "http.scheme": "https", "http.method": "GET", - "http.host": "sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com", + "net.peer.name": "sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com", "http.flavor": "{flavor}", "http.url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/" } @@ -127,7 +132,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "200", "http.url": "http://{host}:{port}/" @@ -144,7 +150,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "200", "http.url": "http://{host}:{port}/" @@ -161,7 +168,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "399", "http.url": "http://{host}:{port}/" @@ -178,7 +186,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "400", "http.url": "http://{host}:{port}/" @@ -195,7 +204,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "401", "http.url": "http://{host}:{port}/" @@ -212,7 +222,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "403", "http.url": "http://{host}:{port}/" @@ -229,7 +240,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "404", "http.url": "http://{host}:{port}/" @@ -246,7 +258,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "429", "http.url": "http://{host}:{port}/" @@ -263,7 +276,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "501", "http.url": "http://{host}:{port}/" @@ -280,7 +294,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "503", "http.url": "http://{host}:{port}/" @@ -297,7 +312,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "504", "http.url": "http://{host}:{port}/" @@ -314,7 +330,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "600", "http.url": "http://{host}:{port}/" @@ -331,7 +348,8 @@ "spanAttributes": { "http.scheme": "http", "http.method": "GET", - "http.host": "{host}:{port}", + "net.peer.name": "{host}", + "net.peer.port": "{port}", "http.flavor": "{flavor}", "http.status_code": "200", "http.url": "http://{host}:{port}/"