Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ea4c914
feat: add http.route attribute to open telemetry on requests
gavinbarron Feb 3, 2026
50b1e7a
expanded tests to include some coverage of existing otel instrumentation
gavinbarron Feb 4, 2026
03f0e6e
Potential fix for pull request finding 'Missing Dispose call on local…
baywet Feb 4, 2026
f0171bd
Apply suggestions from code review
baywet Feb 4, 2026
ba384ac
Update tests/http/httpClient/HttpClientRequestAdapterObservabilityTes…
baywet Feb 4, 2026
0ea0d30
Fix HttpResponseMessage disposal in Moq test setups (#645)
Copilot Feb 4, 2026
02a02fe
Wrap HttpResponseMessage instantiation with lambda in Moq ReturnsAsyn…
Copilot Feb 4, 2026
2a13ceb
Fix HttpResponseMessage disposal in mock test setups (#642)
Copilot Feb 4, 2026
4acdcbe
Fix HttpResponseMessage disposal in mock test setups (#643)
Copilot Feb 4, 2026
bc39434
Fix mock response content-type in observability tests (#646)
Copilot Feb 4, 2026
7f91f9b
chore: seals the test class because it implements IDisposable
baywet Feb 4, 2026
b7decc5
Fix observability test setup to use correct content type (#647)
Copilot Feb 4, 2026
059cdca
chore: removes try catch from tests
baywet Feb 4, 2026
3c9ecd6
Deduplicate baseUrlPlaceholder constant in GetNormalizedHttpRoute (#648)
Copilot Feb 4, 2026
b89ed9b
chore: fixes tests definitions
baywet Feb 4, 2026
a71f036
chore: formatting
baywet Feb 4, 2026
c0dde22
Mark IDisposable test classes as sealed (#649)
Copilot Feb 4, 2026
0635ea9
Apply suggestion from @Copilot
baywet Feb 4, 2026
22e4ffe
Merge branch 'main' into feat/add-otel-http.route
gavinbarron Feb 5, 2026
0a74985
cleaned up tests to theories and fixed missing query string cases
gavinbarron Feb 6, 2026
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
79 changes: 79 additions & 0 deletions src/http/httpClient/HttpClientRequestAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,87 @@
var telemetryPathValue = string.IsNullOrEmpty(decodedUriTemplate) ? string.Empty : queryParametersCleanupRegex.Replace(decodedUriTemplate, string.Empty);
var span = activitySource?.StartActivity($"{methodName} - {telemetryPathValue}");
span?.SetTag("url.uri_template", decodedUriTemplate);
if(!string.IsNullOrEmpty(telemetryPathValue))
{
var httpRoute = GetNormalizedHttpRoute(telemetryPathValue);
span?.SetTag("http.route", httpRoute);
}
return span;
}

/// <summary>
/// Normalizes the URI template into an HTTP route for observability by removing
/// the base URL placeholder and prefix, ensuring the route starts with "/".
/// </summary>
/// <param name="telemetryPathValue">The URI template path to normalize.</param>
/// <returns>The normalized HTTP route.</returns>
internal string GetNormalizedHttpRoute(string telemetryPathValue)
{
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_0_OR_GREATER
// Optimized path using spans to reduce string allocations
const string baseUrlPlaceholder = "{+baseurl}";
Comment thread
baywet marked this conversation as resolved.
Outdated
var span = telemetryPathValue.AsSpan();

// Remove the base URL placeholder if present
var placeholderIndex = span.IndexOf(baseUrlPlaceholder.AsSpan(), StringComparison.OrdinalIgnoreCase);
if(placeholderIndex >= 0)
{
// Concatenate parts around the placeholder - reduces allocations by using spans
var withoutPlaceholder = string.Concat(
new string(span[..placeholderIndex]),
new string(span[(placeholderIndex + baseUrlPlaceholder.Length)..])
);
Comment thread
baywet marked this conversation as resolved.
Outdated
span = withoutPlaceholder.AsSpan();
}

// Remove the base URL prefix if present (zero-allocation slice)
if(!string.IsNullOrEmpty(BaseUrl) && span.StartsWith(BaseUrl.AsSpan(), StringComparison.OrdinalIgnoreCase))
{
span = span[BaseUrl.Length..];
}

// Trim whitespace (zero-allocation)
span = span.Trim();

// If empty, return "/"
if(span.IsEmpty)
{
return "/";
}

// Trim leading slashes (zero-allocation slice)
span = span.TrimStart('/');

// Final allocation - create string with "/" prefix
return "/" + new string(span);
#else
// Fallback for older frameworks
var httpRoute = telemetryPathValue;
const string baseUrlPlaceholder = "{+baseurl}";

// Remove the base URL placeholder if present
var baseUrlPlaceholderIndex = httpRoute.IndexOf(baseUrlPlaceholder, StringComparison.OrdinalIgnoreCase);
if(baseUrlPlaceholderIndex >= 0)
{
httpRoute = httpRoute.Remove(baseUrlPlaceholderIndex, baseUrlPlaceholder.Length);
}

// Remove the base URL prefix if the route starts with it
if(!string.IsNullOrEmpty(BaseUrl) && httpRoute.StartsWith(BaseUrl, StringComparison.OrdinalIgnoreCase))
{
httpRoute = httpRoute.Substring(BaseUrl!.Length);
}

// Normalize whitespace and ensure the route starts with "/"
httpRoute = httpRoute.Trim();
if(string.IsNullOrEmpty(httpRoute))
{
return "/";
}

return "/" + httpRoute.TrimStart('/');
#endif
}
/// <summary>
/// Send a <see cref="RequestInformation"/> instance with a collection instance of <typeparam name="ModelType"></typeparam>
/// </summary>
Expand Down Expand Up @@ -262,7 +341,7 @@
#if NET5_0_OR_GREATER
public async Task<ModelType?> SendPrimitiveAsync<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] ModelType>(RequestInformation requestInfo, Dictionary<string, ParsableFactory<IParsable>>? errorMapping = default, CancellationToken cancellationToken = default)
#else
public async Task<ModelType?> SendPrimitiveAsync<ModelType>(RequestInformation requestInfo, Dictionary<string, ParsableFactory<IParsable>>? errorMapping = default, CancellationToken cancellationToken = default)

Check warning on line 344 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 32 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 344 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 31 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
#endif
{
using var span = startTracingSpan(requestInfo, nameof(SendPrimitiveAsync));
Expand Down Expand Up @@ -427,7 +506,7 @@
activity?.SetTag("com.microsoft.kiota.response.type", result.GetType().FullName);
}
}
private static async Task DrainAsync(HttpResponseMessage response, CancellationToken cancellationToken)

Check warning on line 509 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this unused method parameter 'cancellationToken'. (https://rules.sonarsource.com/csharp/RSPEC-1172)

Check warning on line 509 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this unused method parameter 'cancellationToken'. (https://rules.sonarsource.com/csharp/RSPEC-1172)
{
if(response.Content != null)
{
Expand Down Expand Up @@ -522,13 +601,13 @@
if(contentStream == Stream.Null || (contentStream.CanSeek && contentStream.Length == 0))
return null;// ensure a useful stream is passed to the factory
#pragma warning disable CS0618 // Type or member is obsolete
//TODO remove with v2

Check warning on line 604 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)

Check warning on line 604 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var rootNode = pNodeFactory is IAsyncParseNodeFactory asyncParseNodeFactory ? await asyncParseNodeFactory.GetRootParseNodeAsync(responseContentType!, contentStream, cancellationToken).ConfigureAwait(false) : pNodeFactory.GetRootParseNode(responseContentType!, contentStream);
#pragma warning restore CS0618 // Type or member is obsolete
return rootNode;
}
private const string ClaimsKey = "claims";
private const string BearerAuthenticationScheme = "Bearer";

Check warning on line 610 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field 'BearerAuthenticationScheme'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 610 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field 'BearerAuthenticationScheme'. (https://rules.sonarsource.com/csharp/RSPEC-1144)
private async Task<HttpResponseMessage> GetHttpResponseMessageAsync(RequestInformation requestInfo, CancellationToken cancellationToken, Activity? activityForAttributes, string? claims = default, bool isStreamResponse = false)
{
using var span = activitySource?.StartActivity(nameof(GetHttpResponseMessageAsync));
Expand Down Expand Up @@ -612,7 +691,7 @@
else throw new InvalidOperationException($"Could not convert the request information to a {typeof(T).Name}");
}

private HttpRequestMessage GetRequestMessageFromRequestInformation(RequestInformation requestInfo, Activity? activityForAttributes)

Check warning on line 694 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 25 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 694 in src/http/httpClient/HttpClientRequestAdapter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 25 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
using var span = activitySource?.StartActivity(nameof(GetRequestMessageFromRequestInformation));
SetBaseUrlForRequestInformation(requestInfo);// this method can also be called from a different context so ensure the baseUrl is added.
Expand Down
Loading
Loading