Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[repo/AspNetCore] Prepare to .NET9 #2283

Merged
merged 26 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ namespace OpenTelemetry.Instrumentation.AspNetCore;
/// </summary>
internal sealed class AspNetCoreInstrumentation : IDisposable
{
private static readonly HashSet<string> DiagnosticSourceEvents = new()
{
private static readonly HashSet<string> DiagnosticSourceEvents =
[
"Microsoft.AspNetCore.Hosting.HttpRequestIn",
"Microsoft.AspNetCore.Hosting.HttpRequestIn.Start",
"Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop",
"Microsoft.AspNetCore.Diagnostics.UnhandledException",
"Microsoft.AspNetCore.Hosting.UnhandledException",
};
"Microsoft.AspNetCore.Hosting.UnhandledException"
];

private readonly Func<string, object?, object?, bool> isEnabled = (eventName, _, _)
=> DiagnosticSourceEvents.Contains(eventName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ namespace OpenTelemetry.Instrumentation.AspNetCore;
/// </summary>
internal sealed class AspNetCoreMetrics : IDisposable
{
private static readonly HashSet<string> DiagnosticSourceEvents = new()
{
private static readonly HashSet<string> DiagnosticSourceEvents =
[
"Microsoft.AspNetCore.Hosting.HttpRequestIn",
"Microsoft.AspNetCore.Hosting.HttpRequestIn.Start",
"Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop",
"Microsoft.AspNetCore.Diagnostics.UnhandledException",
"Microsoft.AspNetCore.Hosting.UnhandledException",
};
"Microsoft.AspNetCore.Hosting.UnhandledException"
];

private readonly Func<string, object?, object?, bool> isEnabled = (eventName, _, _)
=> DiagnosticSourceEvents.Contains(eventName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal class HttpInListener : ListenerHandler
return value;
}

return Enumerable.Empty<string>();
return [];
};

private static readonly PropertyFetcher<Exception> ExceptionPropertyFetcher = new("Exception");
Expand Down Expand Up @@ -82,6 +82,8 @@ public override void OnEventWritten(string name, object? payload)
this.OnException(activity, payload);
}

break;
default:
break;
}
}
Expand All @@ -98,8 +100,7 @@ public void OnStartActivity(Activity activity, object? payload)
// By this time, samplers have already run and
// activity.IsAllDataRequested populated accordingly.

var context = payload as HttpContext;
if (context == null)
if (payload is not HttpContext context)
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity), activity.OperationName);
return;
Expand Down Expand Up @@ -309,7 +310,7 @@ public void OnException(Activity activity, object? payload)
if (activity.IsAllDataRequested)
{
// We need to use reflection here as the payload type is not a defined public type.
if (!TryFetchException(payload, out Exception? exc))
if (!TryFetchException(payload, out var exc))
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnException), activity.OperationName);
return;
Expand Down Expand Up @@ -341,7 +342,9 @@ public void OnException(Activity activity, object? payload)
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top level properties are preserved")]
#endif
static bool TryFetchException(object? payload, [NotNullWhen(true)] out Exception? exc)
=> ExceptionPropertyFetcher.TryFetch(payload, out exc) && exc != null;
{
return ExceptionPropertyFetcher.TryFetch(payload, out exc) && exc != null;
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down Expand Up @@ -370,7 +373,7 @@ private static void AddGrpcAttributes(Activity activity, string grpcMethod, Http

activity.SetTag(SemanticConventions.AttributeClientPort, context.Connection.RemotePort);

bool validConversion = GrpcTagHelper.TryGetGrpcStatusCodeFromActivity(activity, out int status);
var validConversion = GrpcTagHelper.TryGetGrpcStatusCodeFromActivity(activity, out var status);
if (validConversion)
{
activity.SetStatus(GrpcTagHelper.ResolveSpanStatusForGrpcStatusCode(status));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal HttpInMetricsListener(string name)
public static void OnExceptionEventWritten(string name, object? payload)
{
// We need to use reflection here as the payload type is not a defined public type.
if (!TryFetchException(payload, out Exception? exc) || !TryFetchHttpContext(payload, out HttpContext? ctx))
if (!TryFetchException(payload, out var exc) || !TryFetchHttpContext(payload, out var ctx))
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(OnExceptionEventWritten), HttpServerRequestDurationMetricName);
return;
Expand All @@ -58,18 +58,21 @@ public static void OnExceptionEventWritten(string name, object? payload)
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The ASP.NET Core framework guarantees that top level properties are preserved")]
#endif
static bool TryFetchException(object? payload, [NotNullWhen(true)] out Exception? exc)
=> ExceptionPropertyFetcher.TryFetch(payload, out exc) && exc != null;
{
return ExceptionPropertyFetcher.TryFetch(payload, out exc) && exc != null;
}
#if NET
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The ASP.NET Core framework guarantees that top level properties are preserved")]
#endif
static bool TryFetchHttpContext(object? payload, [NotNullWhen(true)] out HttpContext? ctx)
=> HttpContextPropertyFetcher.TryFetch(payload, out ctx) && ctx != null;
{
return HttpContextPropertyFetcher.TryFetch(payload, out ctx) && ctx != null;
}
}

public static void OnStopEventWritten(string name, object? payload)
{
var context = payload as HttpContext;
if (context == null)
if (payload is not HttpContext context)
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(OnStopEventWritten), HttpServerRequestDurationMetricName);
return;
Expand Down Expand Up @@ -121,6 +124,8 @@ public override void OnEventWritten(string name, object? payload)
OnStopEventWritten(name, payload);
}

break;
default:
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@ internal static class TelemetryHelper

public static object GetBoxedStatusCode(int statusCode)
{
if (statusCode >= 100 && statusCode < 600)
{
return BoxedStatusCodes[statusCode - 100];
}

return statusCode;
return statusCode is >= 100 and < 600 ? BoxedStatusCodes[statusCode - 100] : statusCode;
}

private static object[] InitializeBoxedStatusCodes()
Expand Down
7 changes: 2 additions & 5 deletions src/Shared/DiagnosticSourceSubscriber.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public DiagnosticSourceSubscriber(
{
Guard.ThrowIfNull(handlerFactory);

this.listenerSubscriptions = new List<IDisposable>();
this.listenerSubscriptions = [];
this.handlerFactory = handlerFactory;
this.diagnosticSourceFilter = diagnosticSourceFilter;
this.isEnabledFilter = isEnabledFilter;
Expand All @@ -43,10 +43,7 @@ public DiagnosticSourceSubscriber(

public void Subscribe()
{
if (this.allSourcesSubscription == null)
{
this.allSourcesSubscription = DiagnosticListener.AllListeners.Subscribe(this);
}
this.allSourcesSubscription ??= DiagnosticListener.AllListeners.Subscribe(this);
}

public void OnNext(DiagnosticListener value)
Expand Down
4 changes: 2 additions & 2 deletions src/Shared/RedactionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ internal sealed class RedactionHelper

public static string? GetRedactedQueryString(string query)
{
int length = query.Length;
int index = 0;
var length = query.Length;
var index = 0;

// Preallocate some size to avoid re-sizing multiple times.
// Since the size will increase, allocating twice as much.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,19 @@

namespace OpenTelemetry.Instrumentation.AspNetCore.Benchmark.Instrumentation;

#pragma warning disable CA1515
public class AspNetCoreInstrumentationBenchmarks
#pragma warning restore CA1515
{
private HttpClient? httpClient;
private WebApplication? app;
private TracerProvider? tracerProvider;
private MeterProvider? meterProvider;

[Flags]
#pragma warning disable CA1515
public enum EnableInstrumentationOption
#pragma warning restore CA1515
{
/// <summary>
/// Instrumentation is not enabled for any signal.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,19 @@
*/
namespace OpenTelemetry.Instrumentation.AspNetCore.Benchmarks.Instrumentation;

#pragma warning disable CA1515
public class AspNetCoreInstrumentationNewBenchmarks
#pragma warning restore CA1515
{
private HttpClient? httpClient;
private WebApplication? app;
private TracerProvider? tracerProvider;
private MeterProvider? meterProvider;

[Flags]
#pragma warning disable CA1515
public enum EnableInstrumentationOption
#pragma warning restore CA1515
{
/// <summary>
/// Instrumentation is not enabled for any signal.
Expand All @@ -99,7 +103,7 @@ public enum EnableInstrumentationOption
[GlobalSetup(Target = nameof(GetRequestForAspNetCoreApp))]
public void GetRequestForAspNetCoreAppGlobalSetup()
{
KeyValuePair<string, string?>[] config = new KeyValuePair<string, string?>[] { new KeyValuePair<string, string?>("OTEL_SEMCONV_STABILITY_OPT_IN", "http") };
KeyValuePair<string, string?>[] config = [new("OTEL_SEMCONV_STABILITY_OPT_IN", "http")];
var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(config)
.Build();
Expand Down
66 changes: 26 additions & 40 deletions test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,7 @@ public async Task CustomPropagator(bool addSampler)
}
finally
{
Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
{
new TraceContextPropagator(),
new BaggagePropagator(),
}));
Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator([new TraceContextPropagator(), new BaggagePropagator()]));
}
}

Expand Down Expand Up @@ -299,14 +295,9 @@ void ConfigureTestServices(IServiceCollection services)
this.tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddAspNetCoreInstrumentation((opt) => opt.Filter = (ctx) =>
{
if (ctx.Request.Path == "/api/values/2")
{
throw new Exception("from InstrumentationFilter");
}
else
{
return true;
}
return ctx.Request.Path == "/api/values/2"
? throw new Exception("from InstrumentationFilter")
: true;
})
.AddInMemoryExporter(exportedItems)
.Build();
Expand Down Expand Up @@ -394,11 +385,7 @@ public async Task ExtractContextIrrespectiveOfSamplingDecision(SamplingDecision
}
finally
{
Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
{
new TraceContextPropagator(),
new BaggagePropagator(),
}));
Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator([new TraceContextPropagator(), new BaggagePropagator()]));
}
}

Expand All @@ -415,7 +402,7 @@ public async Task ExtractContextIrrespectiveOfTheFilterApplied()
Sdk.SetDefaultTextMapPropagator(new ExtractOnlyPropagator(activityContext, expectedBaggage));

// Arrange
bool isFilterCalled = false;
var isFilterCalled = false;
using var testFactory = this.factory
.WithWebHostBuilder(builder =>
{
Expand Down Expand Up @@ -466,11 +453,7 @@ public async Task ExtractContextIrrespectiveOfTheFilterApplied()
}
finally
{
Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
{
new TraceContextPropagator(),
new BaggagePropagator(),
}));
Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator([new TraceContextPropagator(), new BaggagePropagator()]));
}
}

Expand All @@ -479,7 +462,7 @@ public async Task BaggageIsNotClearedWhenActivityStopped()
{
int? baggageCountAfterStart = null;
int? baggageCountAfterStop = null;
using EventWaitHandle stopSignal = new EventWaitHandle(false, EventResetMode.ManualReset);
using var stopSignal = new EventWaitHandle(false, EventResetMode.ManualReset);

void ConfigureTestServices(IServiceCollection services)
{
Expand All @@ -503,6 +486,8 @@ void ConfigureTestServices(IServiceCollection services)
stopSignal.Set();
}

break;
default:
break;
}
},
Expand Down Expand Up @@ -542,9 +527,9 @@ void ConfigureTestServices(IServiceCollection services)
[InlineData(SamplingDecision.RecordAndSample, true, true)]
public async Task FilterAndEnrichAreOnlyCalledWhenSampled(SamplingDecision samplingDecision, bool shouldFilterBeCalled, bool shouldEnrichBeCalled)
{
bool filterCalled = false;
bool enrichWithHttpRequestCalled = false;
bool enrichWithHttpResponseCalled = false;
var filterCalled = false;
var enrichWithHttpRequestCalled = false;
var enrichWithHttpResponseCalled = false;
void ConfigureTestServices(IServiceCollection services)
{
this.tracerProvider = Sdk.CreateTracerProviderBuilder()
Expand Down Expand Up @@ -667,9 +652,10 @@ void ConfigureTestServices(IServiceCollection services)
})
.CreateClient();

var message = new HttpRequestMessage();

message.Method = new HttpMethod(originalMethod);
var message = new HttpRequestMessage
{
Method = new HttpMethod(originalMethod),
};

try
{
Expand Down Expand Up @@ -811,8 +797,8 @@ public async Task ShouldExportActivityWithOneOrMoreExceptionFilters(int mode)
[Fact]
public async Task DiagnosticSourceCallbacksAreReceivedOnlyForSubscribedEvents()
{
int numberOfUnSubscribedEvents = 0;
int numberofSubscribedEvents = 0;
var numberOfUnSubscribedEvents = 0;
var numberofSubscribedEvents = 0;

this.tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddAspNetCoreInstrumentation(
Expand Down Expand Up @@ -866,9 +852,9 @@ public async Task DiagnosticSourceCallbacksAreReceivedOnlyForSubscribedEvents()
[Fact]
public async Task DiagnosticSourceExceptionCallbackIsReceivedForUnHandledException()
{
int numberOfUnSubscribedEvents = 0;
int numberofSubscribedEvents = 0;
int numberOfExceptionCallbacks = 0;
var numberOfUnSubscribedEvents = 0;
var numberofSubscribedEvents = 0;
var numberOfExceptionCallbacks = 0;

this.tracerProvider = Sdk.CreateTracerProviderBuilder()
.AddAspNetCoreInstrumentation(
Expand Down Expand Up @@ -941,10 +927,10 @@ public async Task DiagnosticSourceExceptionCallbackIsReceivedForUnHandledExcepti
[Fact]
public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHandledInMiddleware()
{
int numberOfUnSubscribedEvents = 0;
int numberOfSubscribedEvents = 0;
int numberOfExceptionCallbacks = 0;
bool exceptionHandled = false;
var numberOfUnSubscribedEvents = 0;
var numberOfSubscribedEvents = 0;
var numberOfExceptionCallbacks = 0;
var exceptionHandled = false;

// configure SDK
this.tracerProvider = Sdk.CreateTracerProviderBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public void TestTracingOptionsDIConfig(string? name)
{
name ??= Options.DefaultName;

bool optionsPickedFromDI = false;
var optionsPickedFromDI = false;
void ConfigureTestServices(IServiceCollection services)
{
services.AddOpenTelemetry()
Expand Down
Loading
Loading