Improve GetUri (#2947)

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -5,6 +5,8 @@
 * Fix: drop direct reference of the `Microsoft.AspNetCore.Http.Features` from
   net5 & net6 targets (already part of the FrameworkReference since the net5).
   ([#2860](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2860))
+* Reduce allocations calculating the http.url tag.
+  ([#2947](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2947))
 
 ## Unreleased
 

src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs

@@ -22,7 +22,6 @@ using System.Reflection;
 #if !NETSTANDARD2_0
 using System.Runtime.CompilerServices;
 #endif
-using System.Text;
 using Microsoft.AspNetCore.Http;
 using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Internal;
@@ -300,37 +299,46 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
 
         private static string GetUri(HttpRequest request)
         {
-            var builder = new StringBuilder();
+            // this follows the suggestions from https://github.com/dotnet/aspnetcore/issues/28906
+            var scheme = request.Scheme ?? string.Empty;
 
-            builder.Append(request.Scheme).Append("://");
-
-            if (request.Host.HasValue)
+            // HTTP 1.0 request with NO host header would result in empty Host.
+            // Use placeholder to avoid incorrect URL like "http:///"
+            var host = request.Host.Value ?? UnknownHostName;
+            var pathBase = request.PathBase.Value ?? string.Empty;
+            var path = request.Path.Value ?? string.Empty;
+            var queryString = request.QueryString.Value ?? string.Empty;
+            var length = scheme.Length + Uri.SchemeDelimiter.Length + host.Length + pathBase.Length
+                         + path.Length + queryString.Length;
+#if NETSTANDARD2_1
+            return string.Create(length, (scheme, host, pathBase, path, queryString), (span, parts) =>
             {
-                builder.Append(request.Host.Value);
-            }
-            else
-            {
-                // HTTP 1.0 request with NO host header would result in empty Host.
-                // Use placeholder to avoid incorrect URL like "http:///"
-                builder.Append(UnknownHostName);
-            }
+                CopyTo(ref span, parts.scheme);
+                CopyTo(ref span, Uri.SchemeDelimiter);
+                CopyTo(ref span, parts.host);
+                CopyTo(ref span, parts.pathBase);
+                CopyTo(ref span, parts.path);
+                CopyTo(ref span, parts.queryString);
 
-            if (request.PathBase.HasValue)
-            {
-                builder.Append(request.PathBase.Value);
-            }
-
-            if (request.Path.HasValue)
-            {
-                builder.Append(request.Path.Value);
-            }
-
-            if (request.QueryString.HasValue)
-            {
-                builder.Append(request.QueryString);
-            }
-
-            return builder.ToString();
+                static void CopyTo(ref Span<char> buffer, ReadOnlySpan<char> text)
+                {
+                    if (!text.IsEmpty)
+                    {
+                        text.CopyTo(buffer);
+                        buffer = buffer.Slice(text.Length);
+                    }
+                }
+            });
+#else
+            return new System.Text.StringBuilder(length)
+                .Append(scheme)
+                .Append(Uri.SchemeDelimiter)
+                .Append(host)
+                .Append(pathBase)
+                .Append(path)
+                .Append(queryString)
+                .ToString();
+#endif
         }
 
 #if !NETSTANDARD2_0

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs

@@ -49,12 +49,13 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Tests
         }
 
         [Theory]
-        [InlineData("/api/values", "user-agent", 503, "503")]
-        [InlineData("/api/values", null, 503, null)]
-        [InlineData("/api/exception", null, 503, null)]
-        [InlineData("/api/exception", null, 503, null, true)]
+        [InlineData("/api/values", null, "user-agent", 503, "503")]
+        [InlineData("/api/values", "?query=1", null, 503, null)]
+        [InlineData("/api/exception", null, null, 503, null)]
+        [InlineData("/api/exception", null, null, 503, null, true)]
         public async Task SuccessfulTemplateControllerCallGeneratesASpan(
             string urlPath,
+            string query,
             string userAgent,
             int statusCode,
             string reasonPhrase,
@@ -81,7 +82,13 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Tests
                     }
 
                     // Act
-                    var response = await client.GetAsync(urlPath);
+                    var path = urlPath;
+                    if (query != null)
+                    {
+                        path += query;
+                    }
+
+                    var response = await client.GetAsync(path);
                 }
                 catch (Exception)
                 {
@@ -109,7 +116,7 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Tests
             Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeHttpHost));
             Assert.Equal("GET", activity.GetTagValue(SemanticConventions.AttributeHttpMethod));
             Assert.Equal(urlPath, activity.GetTagValue(SemanticConventions.AttributeHttpTarget));
-            Assert.Equal($"http://localhost{urlPath}", activity.GetTagValue(SemanticConventions.AttributeHttpUrl));
+            Assert.Equal($"http://localhost{urlPath}{query}", activity.GetTagValue(SemanticConventions.AttributeHttpUrl));
             Assert.Equal(statusCode, activity.GetTagValue(SemanticConventions.AttributeHttpStatusCode));
 
             if (statusCode == 503)