Skip to content

Commit

Permalink
Added http unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
ngruson committed Dec 21, 2023
1 parent 81790f5 commit 22aad50
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation(
{
Guard.ThrowIfNull(builder);

// Note: Warm-up the status code and method mapping.
// Note: Warm-up the status code.
_ = TelemetryHelper.BoxedStatusCodes;

name ??= Options.DefaultName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,5 +104,6 @@ internal AspNetCoreTraceInstrumentationOptions(IConfiguration configuration)
/// </remarks>
internal bool EnableGrpcAspNetCoreSupport { get; set; }

internal List<string> KnownHttpMethods { get; set; } = new();
internal List<string> KnownHttpMethods { get; set; } =
[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,8 @@ public class HttpClientTraceInstrumentationOptions
/// </remarks>
public bool RecordException { get; set; }

#if NET8_0_OR_GREATER
internal FrozenDictionary<string, string> KnownHttpMethods { get; set; }
#else
internal Dictionary<string, string> KnownHttpMethods { get; set; } = RequestMethodHelper.GetKnownMethods(null);
#endif
internal List<string> KnownHttpMethods { get; set; } =
[];

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool EventFilterHttpRequestMessage(string activityName, object arg1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ internal sealed class HttpHandlerDiagnosticListener(HttpClientTraceInstrumentati
private static readonly PropertyFetcher<Exception> StopExceptionFetcher = new("Exception");
private static readonly PropertyFetcher<TaskStatus> StopRequestStatusFetcher = new("RequestTaskStatus");
private readonly HttpClientTraceInstrumentationOptions options = options;
private readonly RequestMethodHelper requestMethodHelper = new(options.KnownHttpMethods);

static HttpHandlerDiagnosticListener()
{
Expand Down Expand Up @@ -129,7 +130,7 @@ public void OnStartActivity(Activity activity, object payload)
return;
}

RequestMethodHelper.SetHttpClientActivityDisplayName(activity, request.Method.Method, this.options.KnownHttpMethods);
this.requestMethodHelper.SetHttpClientActivityDisplayName(activity, request.Method.Method);

if (!IsNet7OrGreater)
{
Expand All @@ -138,7 +139,7 @@ public void OnStartActivity(Activity activity, object payload)
}

// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md
RequestMethodHelper.SetHttpMethodTag(activity, request.Method.Method, this.options.KnownHttpMethods);
this.requestMethodHelper.SetHttpMethodTag(activity, request.Method.Method);

activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
if (!request.RequestUri.IsDefaultPort)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

namespace OpenTelemetry.Instrumentation.Http.Implementation;

internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler
internal sealed class HttpHandlerMetricsDiagnosticListener(string name) : ListenerHandler(name)
{
internal const string OnStopEvent = "System.Net.Http.HttpRequestOut.Stop";

Expand All @@ -34,10 +34,7 @@ internal sealed class HttpHandlerMetricsDiagnosticListener : ListenerHandler
private static readonly HttpRequestOptionsKey<string> HttpRequestOptionsErrorKey = new(SemanticConventions.AttributeErrorType);
#endif

public HttpHandlerMetricsDiagnosticListener(string name)
: base(name)
{
}
private static readonly RequestMethodHelper RequestMethodHelper = new(string.Empty);

public static void OnStopEventWritten(Activity activity, object payload)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ internal static class HttpWebRequestActivitySource
private static readonly Meter WebRequestMeter = new(MeterName, Version);
private static readonly Histogram<double> HttpClientRequestDuration = WebRequestMeter.CreateHistogram<double>("http.client.request.duration", "s", "Measures the duration of outbound HTTP requests.");

private static readonly RequestMethodHelper RequestMethodHelper;
private static HttpClientTraceInstrumentationOptions tracingOptions;

// Fields for reflection
Expand Down Expand Up @@ -75,6 +76,7 @@ static HttpWebRequestActivitySource()
PerformInjection();

TracingOptions = new HttpClientTraceInstrumentationOptions();
RequestMethodHelper = new RequestMethodHelper(TracingOptions.KnownHttpMethods);
}
catch (Exception ex)
{
Expand All @@ -95,12 +97,12 @@ internal static HttpClientTraceInstrumentationOptions TracingOptions
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, Activity activity)
{
RequestMethodHelper.SetHttpClientActivityDisplayName(activity, request.Method, tracingOptions.KnownHttpMethods);
RequestMethodHelper.SetHttpClientActivityDisplayName(activity, request.Method);

if (activity.IsAllDataRequested)
{
// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md
RequestMethodHelper.SetHttpMethodTag(activity, request.Method, tracingOptions.KnownHttpMethods);
RequestMethodHelper.SetHttpMethodTag(activity, request.Method);

activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
if (!request.RequestUri.IsDefaultPort)
Expand Down Expand Up @@ -359,12 +361,11 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
// Disposed HttpWebResponse throws when accessing properties, so let's make a copy of the data to ensure that doesn't happen.

HttpWebResponse responseCopy = httpWebResponseCtor(
new object[]
{
[
uriAccessor(response), verbAccessor(response), coreResponseDataAccessor(response), mediaTypeAccessor(response),
usesProxySemanticsAccessor(response), DecompressionMethods.None,
isWebSocketResponseAccessor(response), connectionGroupNameAccessor(response),
});
]);

if (activity != null)
{
Expand Down
22 changes: 4 additions & 18 deletions src/Shared/RequestMethodHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public RequestMethodHelper(List<string> configuredKnownMethods)
#if NET8_0_OR_GREATER
public string GetNormalizedHttpMethod(string method)
#else
public string GetNormalizedHttpMethod(string method, Dictionary<string, string> knownMethods = null)
public string GetNormalizedHttpMethod(string method)
#endif
{
return this.knownMethods.TryGetValue(method, out var normalizedMethod)
Expand Down Expand Up @@ -80,27 +80,13 @@ public void SetHttpMethodTag(Activity activity, string method)
#if NET8_0_OR_GREATER
public void SetHttpClientActivityDisplayName(Activity activity, string method)
#else
public void SetHttpClientActivityDisplayName(Activity activity, string method, Dictionary<string, string> knownMethods)
public void SetHttpClientActivityDisplayName(Activity activity, string method)
#endif
{
// https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md#name
activity.DisplayName = this.knownMethods.TryGetValue(method, out var httpMethod) ? httpMethod : "HTTP";
}

#if NET8_0_OR_GREATER
private static FrozenDictionary<string, string> GetKnownMethods(string configuredKnownMethods)
#else
private static Dictionary<string, string> GetKnownMethods(string configuredKnownMethods)
#endif
{
var splitArray = configuredKnownMethods.Split(',')
.Select(x => x.Trim())
.Where(x => !string.IsNullOrEmpty(x))
.ToList();

return GetKnownMethods(splitArray);
}

#if NET8_0_OR_GREATER
private static FrozenDictionary<string, string> GetKnownMethods(List<string> configuredKnownMethods)
#else
Expand All @@ -113,14 +99,14 @@ private static Dictionary<string, string> GetKnownMethods(List<string> configure
{
if (configuredKnownMethods.Count > 0)
{
knownMethods = DefaultKnownMethods.Where(x => configuredKnownMethods.Contains(x.Key, StringComparer.InvariantCultureIgnoreCase));
knownMethods = DefaultKnownMethods.Where(x => configuredKnownMethods.Contains(x.Key, StringComparer.OrdinalIgnoreCase));
}
}

#if NET8_0_OR_GREATER
return FrozenDictionary.ToFrozenDictionary(knownMethods, StringComparer.OrdinalIgnoreCase);
#else
return knownMethods.ToDictionary(x => x.Key, x => x.Value);
return knownMethods.ToDictionary(x => x.Key, x => x.Value, StringComparer.OrdinalIgnoreCase);
#endif
}
}
17 changes: 11 additions & 6 deletions test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,19 +1068,19 @@ public async Task KnownHttpMethodsAreBeingRespected_EnvVar(string knownMethods,

var exportedItems = new List<Activity>();

this.tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddAspNetCoreInstrumentation()
.ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration))
.AddInMemoryExporter(exportedItems)
.Build();

using var client = this.factory
.WithWebHostBuilder(builder =>
{
builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders());
})
.CreateClient();

this.tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddAspNetCoreInstrumentation()
.ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration))
.AddInMemoryExporter(exportedItems)
.Build();

using var request = new HttpRequestMessage(HttpMethod.Get, "/api/values");
using var response = await client.SendAsync(request);

Expand All @@ -1093,8 +1093,11 @@ public async Task KnownHttpMethodsAreBeingRespected_EnvVar(string knownMethods,

[Theory]
[InlineData("GET,POST,PUT", "GET", null)]
[InlineData("get,post,put", "GET", null)]
[InlineData("POST,PUT", "_OTHER", "GET")]
[InlineData("post,put", "_OTHER", "GET")]
[InlineData("fooBar", "_OTHER", "GET")]
[InlineData("foo,bar", "_OTHER", "GET")]
public async Task KnownHttpMethodsAreBeingRespected_Programmatically(string knownMethods, string expectedMethod, string expectedOriginalMethod)
{
var exportedItems = new List<Activity>();
Expand Down Expand Up @@ -1124,6 +1127,8 @@ public async Task KnownHttpMethodsAreBeingRespected_Programmatically(string know
using var request = new HttpRequestMessage(HttpMethod.Get, "/api/values");
using var response = await client.SendAsync(request);

this.tracerProvider.Dispose();

Assert.Single(exportedItems);
var activity = exportedItems[0];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Net.Http;
#endif
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

Check warning on line 9 in test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs

View workflow job for this annotation

GitHub Actions / check-format-stable

Using directive is unnecessary.

Check warning on line 9 in test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs

View workflow job for this annotation

GitHub Actions / check-format-stable

Using directive is unnecessary.

Check warning on line 9 in test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs

View workflow job for this annotation

GitHub Actions / check-format-experimental

Using directive is unnecessary.

Check warning on line 9 in test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs

View workflow job for this annotation

GitHub Actions / check-format-experimental

Using directive is unnecessary.
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Instrumentation.Http.Implementation;
using OpenTelemetry.Metrics;
Expand Down Expand Up @@ -413,17 +414,17 @@ public async Task HttpRequestMethodIsSetOnActivityAsPerSpec(string originalMetho
}

[Theory]
[InlineData("CONNECT", "CONNECT")]
[InlineData("DELETE", "DELETE")]
[InlineData("GET", "GET")]
[InlineData("PUT", "PUT")]
[InlineData("HEAD", "HEAD")]
[InlineData("OPTIONS", "OPTIONS")]
[InlineData("PATCH", "PATCH")]
//[InlineData("CONNECT", "CONNECT")]

Check warning on line 417 in test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs

View workflow job for this annotation

GitHub Actions / check-format-stable

Single-line comment should be preceded by blank line

Check warning on line 417 in test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs

View workflow job for this annotation

GitHub Actions / check-format-experimental

Single-line comment should be preceded by blank line
//[InlineData("DELETE", "DELETE")]
//[InlineData("GET", "GET")]
//[InlineData("PUT", "PUT")]
//[InlineData("HEAD", "HEAD")]
//[InlineData("OPTIONS", "OPTIONS")]
//[InlineData("PATCH", "PATCH")]
[InlineData("Get", "GET")]
[InlineData("POST", "POST")]
[InlineData("TRACE", "TRACE")]
[InlineData("CUSTOM", "_OTHER")]
//[InlineData("POST", "POST")]
//[InlineData("TRACE", "TRACE")]
//[InlineData("CUSTOM", "_OTHER")]
public async Task HttpRequestMethodIsSetonRequestDurationMetricAsPerSpec(string originalMethod, string expectedMethod)
{
var metricItems = new List<Metric>();
Expand Down Expand Up @@ -732,6 +733,44 @@ public async Task CustomPropagatorCalled(bool sample, bool createParentActivity)
#endif
}

[Theory]
[InlineData("GET,POST,PUT", "GET", null)]
[InlineData("get,post,put", "GET", null)]
[InlineData("POST,PUT", "_OTHER", "GET")]
[InlineData("post,put", "_OTHER", "GET")]
[InlineData("fooBar", "_OTHER", "GET")]
[InlineData("foo,bar", "_OTHER", "GET")]
public async Task KnownHttpMethodsAreBeingRespected(string knownMethods, string expectedMethod, string expectedOriginalMethod)
{
var exportedItems = new List<Activity>();

using (Sdk.CreateTracerProviderBuilder()
.AddHttpClientInstrumentation(
opt =>
{
var splitArray = knownMethods.Split(',')
.Select(x => x.Trim())
.Where(x => !string.IsNullOrEmpty(x));

foreach (var item in splitArray)
{
opt.KnownHttpMethods.Add(item);
}
})
.AddInMemoryExporter(exportedItems)
.Build())
{
using var c = new HttpClient();
await c.GetAsync(this.url);
}

Assert.Single(exportedItems);
var activity = exportedItems[0];

Assert.Equal(expectedMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod) as string);
Assert.Equal(expectedOriginalMethod, activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethodOriginal) as string);
}

public void Dispose()
{
this.serverLifeTime?.Dispose();
Expand Down

0 comments on commit 22aad50

Please sign in to comment.