Skip to content

Conversation

@anna-git
Copy link
Contributor

@anna-git anna-git commented Sep 29, 2025

As part of Config inversion third step, this is part of stack:
Add gitlab step and json configuration file > #7548
Cleanup config keys constants, check them against local json file > #7556
Aliases handling via source generator > #7565
Analyzers to guard platform and configurationbuilder > #7575

Summary of changes

  • Added Roslyn analyzers to enforce proper usage of configuration keys in ConfigurationBuilder
  • Implemented three analyzers:
    • ConfigurationBuilderWithKeysAnalyzer: Ensures WithKeys/Or methods only use constants from PlatformKeys or ConfigurationKeys
    • PlatformKeysAnalyzer: Validates usage of platform keys
    • ConfigKeysNoPreprocessorDirsAnalyzer: Prevents preprocessor directives in configuration keys

Reason for change

  • Improve code quality and maintainability by enforcing consistent configuration key usage
  • Prevent runtime errors due to typos in configuration keys
  • Ensure all configuration keys are properly defined in central locations (PlatformKeys or ConfigurationKeys)
  • Part of the larger configuration inversion work to standardize configuration handling

Implementation details

  • Created Roslyn analyzers that run during compilation
  • Added appropriate error messages and code fixes
  • Updated existing code to comply with the new rules
  • Added comprehensive test coverage for all analyzers
  • Analyzers check for:
    • Hardcoded strings in WithKeys/Or method calls
    • Variables or expressions used instead of constants
    • Proper usage of platform keys
    • Prevention of preprocessor directives in configuration keys

Test coverage

  • Added unit tests for all analyzers
  • Verified analyzers work across different project types and target frameworks
  • Test cases cover:
    • Valid usage with constants from PlatformKeys/ConfigurationKeys
    • Invalid usage with hardcoded strings
    • Edge cases and various syntax patterns
    • Different method invocation patterns

Other details

  • Fixes issues with inconsistent configuration key usage
  • Part of a larger configuration inversion effort
  • Analyzers provide clear error messages to guide developers
  • No breaking changes to public APIs

@anna-git anna-git changed the title Anna/config inversion platformkeys check [Config Inversion] Platformkeys check Sep 29, 2025
@anna-git anna-git changed the title [Config Inversion] Platformkeys check [Config Inversion] Analyzers for ConfigurationBuilder Sep 29, 2025
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch from d273ab2 to 69d1378 Compare September 29, 2025 12:27
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch 4 times, most recently from 7186a16 to 4b7bede Compare September 29, 2025 14:07
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch 2 times, most recently from 0ac2639 to fa3c30d Compare September 29, 2025 14:15
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch 2 times, most recently from 77649c3 to 932c3c6 Compare September 29, 2025 14:52
@datadog-official

This comment has been minimized.

@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch from fa3c30d to 554b7d0 Compare September 29, 2025 15:07
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from 932c3c6 to 63f9685 Compare September 29, 2025 15:11
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch from 554b7d0 to dcd0751 Compare September 29, 2025 15:37
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from 63f9685 to b9187b9 Compare September 29, 2025 15:37
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch from dcd0751 to a70649e Compare September 29, 2025 15:52
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from b9187b9 to 7bae444 Compare September 29, 2025 15:52
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch from a70649e to c69d700 Compare September 29, 2025 17:34
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from 7bae444 to 9e24d0b Compare September 29, 2025 17:34
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch from c69d700 to afe38ff Compare October 2, 2025 09:49
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from 9e24d0b to 0dead4a Compare October 2, 2025 09:49
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch 2 times, most recently from 98c0725 to e83c851 Compare October 2, 2025 19:04
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from 0dead4a to da3bfea Compare October 2, 2025 19:04
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch from e83c851 to a24c630 Compare October 2, 2025 20:07
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from da3bfea to 450ef3f Compare October 2, 2025 20:07
@anna-git anna-git force-pushed the anna/config-inversion-aliases-handling branch from a24c630 to f82f250 Compare October 3, 2025 10:33
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from 450ef3f to d8a7e8f Compare October 3, 2025 10:33
@dd-trace-dotnet-ci-bot
Copy link

dd-trace-dotnet-ci-bot bot commented Oct 3, 2025

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-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 shown in red. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

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).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7575) - mean (74ms)  : 72, 75
     .   : milestone, 74,
    master - mean (74ms)  : 73, 76
     .   : milestone, 74,

    section Baseline
    This PR (7575) - mean (70ms)  : 68, 72
     .   : milestone, 70,
    master - mean (70ms)  : 68, 72
     .   : milestone, 70,

    section CallTarget+Inlining+NGEN
    This PR (7575) - mean (1,076ms)  : 1022, 1131
     .   : milestone, 1076,
    master - mean (1,065ms)  : 1005, 1125
     .   : milestone, 1065,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7575) - mean (110ms)  : 108, 112
     .   : milestone, 110,
    master - mean (109ms)  : 108, 111
     .   : milestone, 109,

    section Baseline
    This PR (7575) - mean (108ms)  : 105, 112
     .   : milestone, 108,
    master - mean (109ms)  : 106, 111
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (7575) - mean (759ms)  : 729, 789
     .   : milestone, 759,
    master - mean (754ms)  : 733, 775
     .   : milestone, 754,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7575) - mean (97ms)  : 95, 99
     .   : milestone, 97,
    master - mean (97ms)  : 95, 100
     .   : milestone, 97,

    section Baseline
    This PR (7575) - mean (96ms)  : 94, 99
     .   : milestone, 96,
    master - mean (97ms)  : 94, 100
     .   : milestone, 97,

    section CallTarget+Inlining+NGEN
    This PR (7575) - mean (727ms)  : 686, 767
     .   : milestone, 727,
    master - mean (722ms)  : 676, 767
     .   : milestone, 722,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7575) - mean (96ms)  : 94, 98
     .   : milestone, 96,
    master - mean (98ms)  : 95, 100
     .   : milestone, 98,

    section Baseline
    This PR (7575) - mean (96ms)  : 92, 99
     .   : milestone, 96,
    master - mean (98ms)  : 90, 105
     .   : milestone, 98,

    section CallTarget+Inlining+NGEN
    This PR (7575) - mean (674ms)  : 657, 691
     .   : milestone, 674,
    master - mean (673ms)  : 656, 691
     .   : milestone, 673,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7575) - mean (197ms)  : 194, 201
     .   : milestone, 197,
    master - mean (199ms)  : 193, 205
     .   : milestone, 199,

    section Baseline
    This PR (7575) - mean (193ms)  : 189, 197
     .   : milestone, 193,
    master - mean (195ms)  : 191, 199
     .   : milestone, 195,

    section CallTarget+Inlining+NGEN
    This PR (7575) - mean (1,192ms)  : 1119, 1265
     .   : milestone, 1192,
    master - mean (1,183ms)  : 1104, 1262
     .   : milestone, 1183,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7575) - mean (278ms)  : 271, 286
     .   : milestone, 278,
    master - mean (279ms)  : 273, 286
     .   : milestone, 279,

    section Baseline
    This PR (7575) - mean (276ms)  : 272, 281
     .   : milestone, 276,
    master - mean (279ms)  : 267, 292
     .   : milestone, 279,

    section CallTarget+Inlining+NGEN
    This PR (7575) - mean (955ms)  : 912, 998
     .   : milestone, 955,
    master - mean (948ms)  : 900, 996
     .   : milestone, 948,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7575) - mean (273ms)  : 266, 280
     .   : milestone, 273,
    master - mean (274ms)  : 266, 281
     .   : milestone, 274,

    section Baseline
    This PR (7575) - mean (270ms)  : 266, 274
     .   : milestone, 270,
    master - mean (270ms)  : 267, 274
     .   : milestone, 270,

    section CallTarget+Inlining+NGEN
    This PR (7575) - mean (940ms)  : 892, 989
     .   : milestone, 940,
    master - mean (943ms)  : 883, 1003
     .   : milestone, 943,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7575) - mean (274ms)  : 265, 283
     .   : milestone, 274,
    master - mean (271ms)  : 266, 277
     .   : milestone, 271,

    section Baseline
    This PR (7575) - mean (272ms)  : 265, 278
     .   : milestone, 272,
    master - mean (271ms)  : 264, 279
     .   : milestone, 271,

    section CallTarget+Inlining+NGEN
    This PR (7575) - mean (869ms)  : 838, 901
     .   : milestone, 869,
    master - mean (866ms)  : 838, 894
     .   : milestone, 866,

Loading

@anna-git anna-git requested review from link04 and removed request for a team October 13, 2025 14:44
@anna-git anna-git changed the base branch from anna/config-inversion-check-configurationkeys-are-in-json to anna/config-inversion-supported-config-file October 13, 2025 14:44
@anna-git anna-git requested review from a team as code owners October 13, 2025 14:44
@anna-git anna-git marked this pull request as draft October 13, 2025 22:37
@anna-git anna-git force-pushed the anna/config-inversion-platformkeys-check branch from 25076db to 23518f6 Compare October 21, 2025 16:12
@anna-git anna-git changed the base branch from anna/config-inversion-supported-config-file to master October 21, 2025 16:13
@anna-git anna-git closed this Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants