Conversation
95b0301 to
39a7db5
Compare
346d13a to
1440fdf
Compare
39a7db5 to
7b7e9bf
Compare
1440fdf to
dad2367
Compare
anna-git
added a commit
that referenced
this pull request
Dec 9, 2025
…any string key (#7689) ## Context Part of **Configuration Inversion (Step 4)** - Stack progress: 1. [#7548](#7548) - Add GitLab step and JSON configuration file 2. [#7688](#7688) - Cleanup configuration / platform keys + analyzers 3. [#7698](#7698) - Source generator for ConfigurationKeys 4. **→ [#7689](#7689) - Aliases handling and analyzers (this PR)** 5. [#7931](#7931) - Replace manual ConfigurationKeys with generated version 6. #[7932](#7932 Forbid use of System.Environment methods and adapt everywhere ## Summary Adds source generator for configuration key aliases, integrates alias resolution into `ConfigurationBuilder`, and adds Roslyn analyzers to enforce proper configuration key usage. ## Changes **Alias Source Generator:** - `ConfigKeyAliasesSwitcherGenerator` reads aliases from `supported-configurations.json` - Auto-generates switch statements to resolve primary keys from aliases - Generated for all target frameworks (net461, netstandard2.0, netcoreapp3.1, net6.0) **ConfigurationBuilder Improvements:** - Integrated alias resolution directly into `ConfigurationBuilder.WithKeys()` - Removed manual fallback overloads throughout codebase - Added `GetKeyWithAlias()` method to `IConfigurationSource` interface **IntegrationSettings Special Handling:** - Updated to use pattern-based key construction (e.g., `DD_TRACE_{INTEGRATION}_ENABLED`) - Simplified configuration reading by leveraging alias system **Roslyn Analyzers:** - **DD0007**: `PlatformKeysAnalyzer` - Enforces use of `PlatformKeys` for external platform environment variables - **DD0008**: `ConfigurationBuilderWithKeysAnalyzer` - Enforces use of [ConfigurationKeys](cci:2://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.Logging.cs:9:0-58:1)/`PlatformKeys` constants in `ConfigurationBuilder.WithKeys()` calls **Special Cases:** - Fixed `DatadogLoggingFactory` to handle deprecated `DD_TRACE_LOG_PATH` with proper analyzer suppression - Updated configuration tests to work with alias resolution ## Motivation Completes configuration inversion by: - Auto-generating alias resolution from `supported-configurations.json` - Eliminating manual fallback chains - Enforcing compile-time validation of configuration keys - Preventing hardcoded strings and typos ## Test Coverage - Added `ConfigKeyAliasesSwitcherGeneratorTests` with comprehensive test coverage - Added `PlatformKeysAnalyzerTests` with 146 lines of tests - Added `ConfigurationBuilderWithKeysAnalyzerTests` with 538 lines of tests - Updated existing configuration tests for alias support - All tests pass with new alias system ## Related Work Builds on #7548 (configuration registry), #7688 (PlatformKeys separation), and #7698 (source generator for ConfigurationKeys).
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (7932) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7932) - mean (77ms) : 74, 79
master - mean (77ms) : 74, 80
section Bailout
This PR (7932) - mean (81ms) : 78, 84
master - mean (81ms) : 78, 84
section CallTarget+Inlining+NGEN
This PR (7932) - mean (1,067ms) : 1018, 1117
master - mean (1,070ms) : 1007, 1133
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7932) - mean (120ms) : 115, 126
master - mean (120ms) : 115, 124
section Bailout
This PR (7932) - mean (122ms) : 120, 124
master - mean (122ms) : 118, 125
section CallTarget+Inlining+NGEN
This PR (7932) - mean (789ms) : 726, 852
master - mean (792ms) : 725, 859
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7932) - mean (106ms) : 103, 110
master - mean (108ms) : 103, 112
section Bailout
This PR (7932) - mean (107ms) : 105, 110
master - mean (108ms) : 106, 110
section CallTarget+Inlining+NGEN
This PR (7932) - mean (752ms) : 671, 833
master - mean (758ms) : 673, 843
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7932) - mean (104ms) : 101, 108
master - mean (105ms) : 102, 108
section Bailout
This PR (7932) - mean (106ms) : 102, 110
master - mean (106ms) : 103, 109
section CallTarget+Inlining+NGEN
This PR (7932) - mean (697ms) : 671, 722
master - mean (701ms) : 679, 723
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7932) - mean (192ms) : 187, 198
master - mean (193ms) : 189, 196
section Bailout
This PR (7932) - mean (196ms) : 193, 198
master - mean (196ms) : 192, 200
section CallTarget+Inlining+NGEN
This PR (7932) - mean (1,129ms) : 1057, 1202
master - mean (1,119ms) : 1066, 1171
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7932) - mean (275ms) : 271, 279
master - mean (276ms) : 269, 283
section Bailout
This PR (7932) - mean (276ms) : 272, 280
master - mean (276ms) : 272, 279
section CallTarget+Inlining+NGEN
This PR (7932) - mean (922ms) : 875, 968
master - mean (931ms) : 890, 972
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7932) - mean (269ms) : 263, 275
master - mean (270ms) : 262, 278
section Bailout
This PR (7932) - mean (269ms) : 266, 273
master - mean (268ms) : 265, 272
section CallTarget+Inlining+NGEN
This PR (7932) - mean (926ms) : 885, 967
master - mean (927ms) : 889, 964
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (7932) - mean (269ms) : 263, 274
master - mean (269ms) : 264, 273
section Bailout
This PR (7932) - mean (268ms) : 264, 271
master - mean (269ms) : 265, 273
section CallTarget+Inlining+NGEN
This PR (7932) - mean (829ms) : 805, 853
master - mean (828ms) : 810, 846
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
7b7e9bf to
8ba62d5
Compare
dad2367 to
f18f8e4
Compare
303b602 to
70df7c9
Compare
This comment has been minimized.
This comment has been minimized.
anna-git
added a commit
that referenced
this pull request
Dec 10, 2025
…nes in the whole solution (#7931) (This PR is actually #7697 that accidentally got closed) ## Context Part of **Configuration Inversion (Step 5)** - Stack progress: 1. [#7548](#7548) - Add GitLab step and JSON configuration file 2. [#7688](#7688) - Cleanup configuration / platform keys + source generator 3. [#7698](#7698) - Aliases handling via source generator 4. [#7689](#7689) - Analyzers for platform and ConfigurationBuilder 5. **→ [#7931](#7697) - Replace manual ConfigurationKeys by generated ones in the whole solution (this PR)** 6. [#7932](#7932) - Forbid use of System.Environment methods and adapt everywhere 7. [#7937](#7937) - Integration names to generated keys ## Summary of changes Fixed the [ConfigurationKeysGenerator](cci:2://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace.SourceGenerators/Configuration/ConfigurationKeysGenerator.cs:21:0-958:1) to properly read and apply the [configuration_keys_mapping.json](cci:7://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Configuration/configuration_keys_mapping.json:0:0-0:0) file, and extracted common file header comments to a reusable constant. **Key changes:** - Fixed JSON array extraction in [ParseMappingFile](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace.SourceGenerators/Configuration/ConfigurationKeysGenerator.cs:350:4-524:5) method - the generator was incorrectly trying to extract the `"mappings"` field as an object instead of an array - Extracted configuration generator comments to `Constants.ConfigurationGeneratorComment` for reuse across multiple generators - Updated both [ConfigurationKeysGenerator](cci:2://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace.SourceGenerators/Configuration/ConfigurationKeysGenerator.cs:21:0-958:1) and [ConfigKeyAliasesSwitcherGenerator](cci:2://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace.SourceGenerators/Configuration/ConfigKeyAliasesSwitcherGenerator.cs:24:0-340:1) to use the shared constant - Added documentation as to how to add a key now ## Reason for change The [configuration_keys_mapping.json](cci:7://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Configuration/configuration_keys_mapping.json:0:0-0:0) file was being ignored during code generation, causing the generator to produce incorrect constant names. This meant that any manual edits to constant names in the mapping file were not being respected, and the generated code would use auto-generated names instead of the explicitly mapped ones. Additionally, the file header comments explaining that files are auto-generated were duplicated across generators, violating DRY principles. ## Implementation details 1. **Fixed array extraction logic**: Replaced the call to [JsonReader.ExtractJsonObjectSection()](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace.SourceGenerators/Configuration/JsonReader.cs:16:4-91:5) with custom array extraction code that: - Finds the `"mappings":` key in the JSON - Locates the opening `[` bracket - Tracks bracket nesting to find the matching closing `]` - Extracts the complete array content 2. **Resolved variable scope issue**: Reused existing `inString` and `escapeNext` variables from the outer scope instead of redeclaring them, fixing compilation errors. 3. **Centralized header comments**: Created `Constants.ConfigurationGeneratorComment` containing the standardized auto-generation notice and updated both generators to use it. ## Test coverage - Verified the generator builds successfully without errors - The mapping file is now properly parsed and applied during code generation - Generated constant names now match the mappings defined in [configuration_keys_mapping.json](cci:7://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Configuration/configuration_keys_mapping.json:0:0-0:0) ## Other details This fix ensures that the explicit naming conventions defined in [configuration_keys_mapping.json](cci:7://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Configuration/configuration_keys_mapping.json:0:0-0:0) are respected, maintaining consistency with the existing codebase and preventing future confusion when constant names don't match their expected values. --------- Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
Base automatically changed from
anna/config-inversion-use-generate-config-keys-5
to
master
December 10, 2025 14:56
c89c676 to
41c88a5
Compare
…ent are called with ConfigurationKeys/PlatformKeys
…er` per Andrew's comments
b481629 to
d8e691c
Compare
d8e691c to
cafe31e
Compare
Merged
Closed
NachoEchevarria
added a commit
that referenced
this pull request
Jan 21, 2026
## Summary of changes In [this PR](#7932), benchmark tests were broken because of a compilation error. We did not detect that until merged into master since the task run-benchmarks is not required for merging into master. ` [ERR] RunBenchmarks: C:\dd-trace-dotnet\tracer\test\benchmarks\Benchmarks.Trace\Asm\AppSecBenchmarkUtils.cs(79,47): error CS7036: There is no argument given that corresponds to the required parameter 'ddDotnetTracerHome' of 'WafLibraryInvoker.Initialize(string, string, string)' [C:\dd-trace-dotnet\tracer\test\benchmarks\Benchmarks.Trace\Benchmarks.Trace.csproj::TargetFramework=net472] ` ## Reason for change ## Implementation details ## Test coverage ## Other details <!-- Fixes #{issue} --> <!--⚠️ Note: Where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. MergeQueue is NOT enabled in this repository. If you have write access to the repo, the PR has 1-2 approvals (see above), and all of the required checks have passed, you can use the Squash and Merge button to merge the PR. If you don't have write access, or you need help, reach out in the #apm-dotnet channel in Slack. -->
anna-git
added a commit
that referenced
this pull request
Jan 27, 2026
## Context Part of **Configuration Inversion (Step 6)** - Stack progress: 1. [#7548](#7548) - Add GitLab step and JSON configuration file 2. [#7688](#7688) - Cleanup configuration / platform keys + source generator 3. [#7698](#7698) - Aliases handling via source generator 4. [#7689](#7689) - Analyzers for platform and ConfigurationBuilder 5. [#7931](#7931) - Replace manual ConfigurationKeys by generated ones in the whole solution 6. [#7932](#7932) - Forbid use of System.Environment methods and adapt everywhere 7. **→ [#7937](#7937) - Integration names to generated keys (this PR)** ## Summary of changes Extends the `EnumExtensionsGenerator` to generate [IntegrationNameToKeys](cci:2://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Generated/net6.0/Datadog.Trace.SourceGenerators/EnumExtensionsGenerator/IntegrationNameToKeys.g.cs:14:4-392:5) class for the `IntegrationId` enum, providing centralized configuration key mapping for integrations including enabled, analytics enabled, and analytics sample rate keys. Also fixes AdoNet instrumentation compilation error. ## Reason for change - Eliminates code duplication in configuration key generation across the codebase - Provides compile-time safety for integration configuration key access - Centralizes key pattern definitions with proper documentation - Ensures consistency across all integration key formats (canonical, mixed case, short aliases) ## Implementation details - **Source Generator**: Extended `EnumExtensionsGenerator` with [GenerateIntegrationNameToKeys](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace.SourceGenerators/EnumExtensions/Sources.cs:82:4-178:5) method that generates: - `AllIntegrationEnabledKeys` array containing all enabled configuration keys - [GetIntegrationEnabledKeys(string)](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Generated/net6.0/Datadog.Trace.SourceGenerators/EnumExtensionsGenerator/IntegrationNameToKeys.g.cs:102:8-197:9) - returns enabled keys for an integration - [GetIntegrationAnalyticsEnabledKeys(string)](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Generated/net6.0/Datadog.Trace.SourceGenerators/EnumExtensionsGenerator/IntegrationNameToKeys.g.cs:198:8-294:9) - returns analytics enabled keys (marked obsolete) - [GetIntegrationAnalyticsSampleRateKeys(string)](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Generated/net6.0/Datadog.Trace.SourceGenerators/EnumExtensionsGenerator/IntegrationNameToKeys.g.cs:295:8-391:9) - returns analytics sample rate keys (marked obsolete) - Key pattern constants (`IntegrationEnabledKeyPattern`, `AnalyticsEnabledKeyPattern`, `AnalyticsSampleRateKeyPattern`) - **Configuration**: Updated [ConfigurationBuilder.WithIntegrationAnalyticsKey](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Configuration/ConfigurationSources/Telemetry/ConfigurationBuilder.cs:32:4-42:5) and [WithIntegrationAnalyticsSampleRateKey](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/src/Datadog.Trace/Configuration/ConfigurationSources/Telemetry/ConfigurationBuilder.cs:44:4-54:5) to use generated methods ## Test coverage - Added [CanGenerateIntegrationNameToKeysForIntegrationId](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/test/Datadog.Trace.SourceGenerators.Tests/EnumExtensionsGeneratorTests.cs:411:4-527:5) - verifies complete generated output with all three methods - Added [DoesNotGenerateIntegrationNameToKeysForNonIntegrationIdEnum](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/test/Datadog.Trace.SourceGenerators.Tests/EnumExtensionsGeneratorTests.cs:529:4-552:5) - ensures generation only for `IntegrationId` enum - Added [IntegrationNameToKeysGeneratesCorrectKeyFormats](cci:1://file:///Users/anna.yafi/go/src/github.com/DataDog/dd-trace-dotnet3/tracer/test/Datadog.Trace.SourceGenerators.Tests/EnumExtensionsGeneratorTests.cs:554:4-670:5) - validates key formats for various integration names - All tests verify obsolete attributes, key patterns, and switch expression logic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Part of Configuration Inversion (Step 6) - Stack progress:
I'll update the PR summary to mention the YAML documentation file:
Summary of changes
Banned direct
System.Environment.GetEnvironmentVariable()usage and migrated all environment variable access to useEnvironmentHelperswith strongly-typed ConfigurationKeys andPlatformKeysconstants.Key changes:
System.Environment.GetEnvironmentVariable()viaBannedApiAnalyzersEnvironmentGetEnvironmentVariableAnalyzer(DD0009) to enforce ConfigurationKeys/PlatformKeysusage onlyPlatformKeysby category (Ci, Aws, AzureAppService, ServiceFabric, DotNet)Reason for change
Direct
System.Environment.GetEnvironmentVariable()calls with string literals are error-prone. Centralizing throughEnvironmentHelperswith strongly-typed constants provides compile-time validation, discoverability, and refactoring safety.Implementation details
BannedSymbols.txtand configured.editorconfigto treat RS0030 as error (vendored code excluded)EnvironmentHelperscalls accept only ConfigurationKeys/PlatformKeysconstants, rejecting hardcoded stringsTest coverage
System.EnvironmentusageOther details
+2,600/-940 lines. No breaking changes, negligible performance impact.