[AspNetCore] Do not add built-in tags for v11+#2
[AspNetCore] Do not add built-in tags for v11+#2martincostello merged 26 commits intodotnet-vnextfrom
Conversation
a44cb2a to
02103d5
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…0.49 (open-telemetry#4325) Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…metry#4328) Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…dentifiers in sanitization (open-telemetry#4317)
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: martincostello <martin@martincostello.com>
Co-authored-by: Matthew Hensley <130569+matt-hensley@users.noreply.github.com>
Co-authored-by: Piotr Kiełkowicz <pkiekowicz@splunk.com>
Do not add tags to activities for ASP.NET Core 11 that are already added by the framework. Relates to open-telemetry#3808.
There was a problem hiding this comment.
Pull request overview
This PR updates several contrib components and tests, most notably optimizing ASP.NET Core tracing instrumentation to avoid duplicating tags when newer ASP.NET Core versions emit OpenTelemetry tags natively.
Changes:
- ASP.NET Core instrumentation: detect native OpenTelemetry tag emission in newer runtimes and avoid re-adding framework-provided tags; add an end-to-end test endpoint (
/ping) and new e2e coverage. - SQL sanitization: harden parsing for malformed bracketed identifiers and preserve CRLF behavior; expand unit + fuzz test coverage.
- Broader maintenance: AWS X-Ray remote sampler polling/disposal improvements, add scope version/schema URL assertions for Redis/Process, update Docker image pins and dependency/workflow versions, and add new banned API rules.
Reviewed changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/TestApp.AspNetCore/Program.cs | Adds /ping endpoint used by new ASP.NET Core end-to-end test. |
| test/Shared/TestHttpServer.cs | Treats additional HttpListener error code as “already closed”. |
| test/OpenTelemetry.Sampler.AWS.Tests/TestAWSXRayRemoteSampler.cs | Refactors sampler reflection helper usage and adds non-blocking poll test. |
| test/OpenTelemetry.Instrumentation.Wcf.Tests/TelemetryEndpointBehaviorTests.cs | Adds NETFRAMEWORK regression test for null Action dispatch operation. |
| test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/redis.Dockerfile | Updates pinned Redis image digest. |
| test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/StackExchangeRedisCallsInstrumentationTests.cs | Verifies activity source name/version and schema URL behavior. |
| test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs | Updates expected metric set and asserts meter version/schema URL. |
| test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/postgres.Dockerfile | Updates pinned Postgres image digest. |
| test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/mysql.Dockerfile | Updates MySQL version and digest. |
| test/OpenTelemetry.Instrumentation.ConfluentKafka.Tests/kafka.Dockerfile | Updates Kafka container image version/digest. |
| test/OpenTelemetry.Instrumentation.ConfluentKafka.Tests/KafkaFixture.cs | Configures Kafka listener + KRaft mode for updated container. |
| test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/TestApplicationFactory.cs | Removes conditional compilation around MapGroup route setup. |
| test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplication/RouteInfo.cs | Removes conditional compilation; minor method simplification. |
| test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs | Removes conditional compilation for NET-only metrics test/imports. |
| test/OpenTelemetry.Instrumentation.AspNetCore.Tests/EndToEndTests.cs | New end-to-end test verifying activity tags under feature switch. |
| test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs | Removes conditional compilation and increases export wait timeout. |
| test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs | Adds SQL sanitization regression tests (CRLF and malformed bracketed identifier). |
| test/OpenTelemetry.Contrib.Shared.FuzzTests/SqlProcessorTests.cs | Adds fuzz property test for malformed bracketed identifiers; uses invariant formatting. |
| src/Shared/StopwatchExtensions.cs | Adds non-NET polyfill for elapsed time computation. |
| src/Shared/SqlProcessor.cs | Improves SQL tokenization/sanitization, comment handling, and malformed identifier handling. |
| src/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs | Replaces instance Equals with static string.Equals to satisfy new banned APIs. |
| src/OpenTelemetry.Sampler.AWS/CHANGELOG.md | Clarifies security fix entry with GHSA reference. |
| src/OpenTelemetry.Sampler.AWS/AWSXRayRemoteSampler.cs | Adds cancellation/locking around polling, safer timer disposal, and cancellation-aware fetch logic. |
| src/OpenTelemetry.Resources.Azure/CHANGELOG.md | Adds GHSA reference and clarifies response-size limit entry. |
| src/OpenTelemetry.Resources.AWS/CHANGELOG.md | Adds GHSA reference and clarifies response-size limit entry. |
| src/OpenTelemetry.OpAmp.Client/CHANGELOG.md | Adds GHSA reference and clarifies response-size limit entry. |
| src/OpenTelemetry.Instrumentation.StackExchangeRedis/TracerProviderBuilderExtensions.cs | Uses ActivitySource.Name instead of a separate constant name. |
| src/OpenTelemetry.Instrumentation.StackExchangeRedis/StackExchangeRedisConnectionInstrumentation.cs | Switches to ActivitySourceFactory; adds schema version variants. |
| src/OpenTelemetry.Instrumentation.StackExchangeRedis/OpenTelemetry.Instrumentation.StackExchangeRedis.csproj | Links shared ActivitySourceFactory.cs. |
| src/OpenTelemetry.Instrumentation.StackExchangeRedis/Implementation/RedisProfilerEntryToActivityConverter.cs | Chooses ActivitySource based on old/new attribute emission options. |
| src/OpenTelemetry.Instrumentation.StackExchangeRedis/CHANGELOG.md | Notes addition of scope version and schema URL to traces. |
| src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj | Links shared StopwatchExtensions.cs. |
| src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlTelemetryHelper.cs | Uses Stopwatch.GetElapsedTime path for duration calculation. |
| src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md | Documents SQL sanitization fix. |
| src/OpenTelemetry.Instrumentation.Process/README.md | Removes documentation for process.cpu.count metric. |
| src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs | Switches to MeterFactory and removes process.cpu.count instrument. |
| src/OpenTelemetry.Instrumentation.Process/OpenTelemetry.Instrumentation.Process.csproj | Links shared MeterFactory.cs. |
| src/OpenTelemetry.Instrumentation.Process/MeterProviderBuilderExtensions.cs | Registers meter using MeterInstance.Name. |
| src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md | Notes scope version/schema URL for metrics and removal of process.cpu.count. |
| src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md | Adds GHSA resolution note for query redaction entry. |
| src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md | Documents SQL sanitization fix. |
| src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs | Adds native-tag detection and avoids duplicating framework-provided tags on newer runtimes. |
| src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md | Documents behavior change and guidance for ASP.NET Core 10 switch. |
| src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs | Minor formatting and comment wording updates. |
| src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs | Makes isEnabled delegate static; expression-bodied Dispose. |
| src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md | Adds GHSA resolution note for query redaction entry. |
| src/OpenTelemetry.Instrumentation.AWS/Implementation/Utils.cs | Uses static string.Equals; minor expression-bodied refactor. |
| src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSServiceType.cs | Uses static string.Equals for case-insensitive comparisons. |
| src/OpenTelemetry.Exporter.OneCollector/CHANGELOG.md | Adds GHSA reference for response-size limit entry. |
| src/OpenTelemetry.Exporter.Geneva/Metrics/TlvMetricExporter.cs | Uses static string.Equals for reserved dimension checks. |
| src/OpenTelemetry.Exporter.Geneva/Metrics/GenevaMetricExporterOptions.cs | Uses static string.Equals for reserved dimension validation. |
| src/OpenTelemetry.Exporter.Geneva/Internal/ConnectionStringBuilder.cs | Uses static string.Equals for boolean parsing. |
| opentelemetry-dotnet-contrib.slnx | Adds src/Shared/StopwatchExtensions.cs to solution items. |
| build/BannedSymbols.txt | Bans instance string.Equals overloads to avoid NRE risk. |
| Directory.Packages.props | Updates package versions and conditions ReferenceAssemblies/StyleCop refs. |
| .github/workflows/sanitycheck.yml | Bumps crate-ci/typos action version. |
| .github/workflows/publish-packages.yml | Updates validation tool version and NuGet/login action version. |
| .github/workflows/markdownlint.yml | Bumps markdownlint action version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (request.Host.HasValue) | ||
| { | ||
| activity.SetTag(SemanticConventions.AttributeServerAddress, request.Host.Value); | ||
|
|
||
| if (request.Host.Port.HasValue) | ||
| if (request.Host.Port is { } port) | ||
| { | ||
| activity.SetTag(SemanticConventions.AttributeServerPort, port); | ||
| } |
| bool? originalValue = null; | ||
|
|
||
| if (AppContext.TryGetSwitch("Microsoft.AspNetCore.Hosting.SuppressActivityOpenTelemetryData", out var existingValue)) | ||
| { | ||
| originalValue = existingValue; | ||
| } | ||
|
|
||
| AppContext.SetSwitch("Microsoft.AspNetCore.Hosting.SuppressActivityOpenTelemetryData", isEnabled); | ||
|
|
||
| try | ||
| { | ||
| var exportedItems = new List<Activity>(); | ||
|
|
||
| void ConfigureTestServices(IServiceCollection services) | ||
| { | ||
| this.tracerProvider = Sdk.CreateTracerProviderBuilder() | ||
| .AddAspNetCoreInstrumentation() | ||
| .AddInMemoryExporter(exportedItems) | ||
| .Build(); | ||
| } | ||
|
|
||
| // Arrange | ||
| using var client = this.factory | ||
| .WithWebHostBuilder(builder => | ||
| { | ||
| builder.ConfigureTestServices(ConfigureTestServices); | ||
| builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); | ||
| }) | ||
| .CreateClient(); | ||
|
|
||
| client.DefaultRequestHeaders.UserAgent.Add(new("OpenTelemetry.Instrumentation.AspNetCore.Tests", "1.0")); | ||
|
|
||
| _ = await client.GetStringAsync(new Uri("/ping", UriKind.Relative)); | ||
|
|
||
| WaitForActivityExport(exportedItems, 1); | ||
|
|
||
| var activity = Assert.Single(exportedItems); | ||
|
|
||
| ValidateAspNetCoreActivity(activity, "/ping"); | ||
|
|
||
| Assert.Equal("GET /ping", activity.DisplayName); | ||
| Assert.Equal("GET", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod)); | ||
| Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeServerAddress)); | ||
| Assert.Equal("OpenTelemetry.Instrumentation.AspNetCore.Tests/1.0", activity.GetTagValue(SemanticConventions.AttributeUserAgentOriginal)); | ||
| Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeUrlScheme)); | ||
| Assert.Equal("/ping", activity.GetTagValue(SemanticConventions.AttributeUrlPath)); | ||
| } | ||
| finally | ||
| { | ||
| if (originalValue is { } previousValue) | ||
| { | ||
| AppContext.SetSwitch("Microsoft.AspNetCore.Hosting.SuppressActivityOpenTelemetryData", previousValue); | ||
| } | ||
| } |
| #if NET10_0 | ||
| // In ASP.NET Core 10 OpenTelemetry tags are suppressed by default, | ||
| // see https://github.com/dotnet/aspnetcore/blob/7387de91234d3ef751fa50b3d1bfede4130213ff/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L59-L67. | ||
| return false; | ||
| #elif NET11_0_OR_GREATER | ||
| // In ASP.NET Core 11+ OpenTelemetry tags are emitted by default, | ||
| // see https://github.com/dotnet/aspnetcore/blob/655f41d52f2fc75992eac41496b8e9cc119e1b54/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L59-L67. | ||
| return true; | ||
| #else | ||
| // In ASP.NET Core 8 and 9 the feature switch does not exist and there are no native OpenTelemetry tags | ||
| return false; | ||
| #endif |
Relates to open-telemetry#3808.
Changes
Do not add tags to activities for ASP.NET Core 11 that are already added by the framework.
Merge requirement checklist
Unit tests added/updatedAppropriateCHANGELOG.mdfiles updated for non-trivial changesChanges in public API reviewed (if applicable)