Skip to content

Commit b92c9c0

Browse files
gavinbarronbaywetgithub-code-quality[bot]CopilotCopilot
authored
feat: add http.route attribute to open telemetry on requests (#640)
* feat: add http.route attribute to open telemetry on requests * expanded tests to include some coverage of existing otel instrumentation * Potential fix for pull request finding 'Missing Dispose call on local IDisposable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update tests/http/httpClient/HttpClientRequestAdapterObservabilityTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix HttpResponseMessage disposal in Moq test setups (#645) * Initial plan * Fix HttpResponseMessage disposal issues in all observability tests Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * Wrap HttpResponseMessage instantiation with lambda in Moq ReturnsAsync (#644) * Initial plan * Fix HttpResponseMessage disposal in all ReturnsAsync calls Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * Fix HttpResponseMessage disposal in mock test setups (#642) * Initial plan * Fix HttpResponseMessage disposal in remaining test methods Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * Fix HttpResponseMessage disposal in mock test setups (#643) * Initial plan * Fix HttpResponseMessage disposal issues in observability tests Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * Fix mock response content-type in observability tests (#646) * Initial plan * Fix empty catch blocks by adding proper content-type to mock responses Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * chore: seals the test class because it implements IDisposable * Fix observability test setup to use correct content type (#647) * Initial plan * Fix empty catch blocks by adding explanatory comments Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * Fix test setup to avoid exceptions instead of silencing them Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * chore: removes try catch from tests * Deduplicate baseUrlPlaceholder constant in GetNormalizedHttpRoute (#648) * Initial plan * Deduplicate baseUrlPlaceholder constant definition Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * chore: fixes tests definitions Signed-off-by: Vincent Biret <vibiret@microsoft.com> * chore: formatting Signed-off-by: Vincent Biret <vibiret@microsoft.com> * Mark IDisposable test classes as sealed (#649) * Initial plan * refactor: mark IDisposable test classes as sealed Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com> * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * cleaned up tests to theories and fixed missing query string cases --------- Signed-off-by: Vincent Biret <vibiret@microsoft.com> Co-authored-by: Vincent Biret <vibiret@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
1 parent 40e97f6 commit b92c9c0

9 files changed

Lines changed: 515 additions & 6 deletions

src/http/httpClient/HttpClientRequestAdapter.cs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,101 @@ public string? BaseUrl
112112
{
113113
var decodedUriTemplate = ParametersNameDecodingHandler.DecodeUriEncodedString(requestInfo.UrlTemplate, charactersToDecodeForUriTemplate);
114114
var telemetryPathValue = string.IsNullOrEmpty(decodedUriTemplate) ? string.Empty : queryParametersCleanupRegex.Replace(decodedUriTemplate, string.Empty);
115+
// Also strip literal query strings (e.g., ?@id={@id})
116+
var questionMarkIndex = telemetryPathValue.IndexOf('?');
117+
if(questionMarkIndex >= 0)
118+
{
119+
telemetryPathValue = telemetryPathValue.Substring(0, questionMarkIndex);
120+
}
115121
var span = activitySource?.StartActivity($"{methodName} - {telemetryPathValue}");
116122
span?.SetTag("url.uri_template", decodedUriTemplate);
123+
if(!string.IsNullOrEmpty(telemetryPathValue))
124+
{
125+
var httpRoute = GetNormalizedHttpRoute(telemetryPathValue);
126+
span?.SetTag("http.route", httpRoute);
127+
}
117128
return span;
118129
}
130+
131+
/// <summary>
132+
/// Normalizes the URI template into an HTTP route for observability by removing
133+
/// the base URL placeholder and prefix, ensuring the route starts with "/".
134+
/// </summary>
135+
/// <param name="telemetryPathValue">The URI template path to normalize.</param>
136+
/// <returns>The normalized HTTP route.</returns>
137+
internal string GetNormalizedHttpRoute(string telemetryPathValue)
138+
{
139+
const string baseUrlPlaceholder = "{+baseurl}";
140+
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_0_OR_GREATER
141+
// Optimized path using spans to reduce string allocations
142+
var span = telemetryPathValue.AsSpan();
143+
144+
// Remove the base URL placeholder if present
145+
var placeholderIndex = span.IndexOf(baseUrlPlaceholder.AsSpan(), StringComparison.OrdinalIgnoreCase);
146+
if(placeholderIndex >= 0)
147+
{
148+
// Build the string without the placeholder in a single allocation
149+
var withoutPlaceholder = string.Create(
150+
telemetryPathValue.Length - baseUrlPlaceholder.Length,
151+
(telemetryPathValue, placeholderIndex, baseUrlPlaceholder.Length),
152+
static (destination, state) =>
153+
{
154+
var (source, index, length) = state;
155+
// Copy the part before the placeholder
156+
source.AsSpan(0, index).CopyTo(destination);
157+
// Copy the part after the placeholder
158+
source.AsSpan(index + length).CopyTo(destination[index..]);
159+
});
160+
span = withoutPlaceholder.AsSpan();
161+
}
162+
163+
// Remove the base URL prefix if present (zero-allocation slice)
164+
if(!string.IsNullOrEmpty(BaseUrl) && span.StartsWith(BaseUrl.AsSpan(), StringComparison.OrdinalIgnoreCase))
165+
{
166+
span = span[BaseUrl.Length..];
167+
}
168+
169+
// Trim whitespace (zero-allocation)
170+
span = span.Trim();
171+
172+
// If empty, return "/"
173+
if(span.IsEmpty)
174+
{
175+
return "/";
176+
}
177+
178+
// Trim leading slashes (zero-allocation slice)
179+
span = span.TrimStart('/');
180+
181+
// Final allocation - create string with "/" prefix
182+
return "/" + new string(span);
183+
#else
184+
// Fallback for older frameworks
185+
var httpRoute = telemetryPathValue;
186+
187+
// Remove the base URL placeholder if present
188+
var baseUrlPlaceholderIndex = httpRoute.IndexOf(baseUrlPlaceholder, StringComparison.OrdinalIgnoreCase);
189+
if(baseUrlPlaceholderIndex >= 0)
190+
{
191+
httpRoute = httpRoute.Remove(baseUrlPlaceholderIndex, baseUrlPlaceholder.Length);
192+
}
193+
194+
// Remove the base URL prefix if the route starts with it
195+
if(!string.IsNullOrEmpty(BaseUrl) && httpRoute.StartsWith(BaseUrl, StringComparison.OrdinalIgnoreCase))
196+
{
197+
httpRoute = httpRoute.Substring(BaseUrl!.Length);
198+
}
199+
200+
// Normalize whitespace and ensure the route starts with "/"
201+
httpRoute = httpRoute.Trim();
202+
if(string.IsNullOrEmpty(httpRoute))
203+
{
204+
return "/";
205+
}
206+
207+
return "/" + httpRoute.TrimStart('/');
208+
#endif
209+
}
119210
/// <summary>
120211
/// Send a <see cref="RequestInformation"/> instance with a collection instance of <typeparam name="ModelType"></typeparam>
121212
/// </summary>

0 commit comments

Comments
 (0)