Skip to content

Conversation

@anna-git
Copy link
Contributor

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

As part of Config inversion second 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 > #7565
Analyzers to guard platform and configurationbuilder > #7575

Summary of changes

  • Split platform-specific environment variables from Datadog configuration keys by creating a new PlatformKeys class
  • Added Nuke build step CheckConfigurationKeysAgainstJsonValidations to validate that all ConfigurationKeys constants are present in supported-configurations.json
  • Updated all references throughout the codebase to use PlatformKeys for external platform environment variables
  • Added missing DD_TRACE_ACTIVITY_LISTENER_ENABLED key to ConfigurationKeys
  • Fixed profiler-related key references to use appropriate classes

Reason for change

After adding supported-configurations.json in PR #7548, we need to ensure that:

  1. All configuration keys defined in ConfigurationKeys class are documented in the JSON file
  2. Platform-specific environment variables (from AWS, Azure, GCP, etc.) are separated from Datadog configuration keys
  3. The JSON file remains synchronized with the codebase as new keys are added

Without this validation, developers could:

  • Add new configuration keys to ConfigurationKeys but forget to add them to supported-configurations.json
  • Mix platform environment variables with Datadog configuration, making the JSON file confusing
  • Create inconsistencies between the code and documentation

Implementation details

PlatformKeys class creation:

  • Extracted platform-specific environment variables from ConfigurationKeys into new PlatformKeys class
  • Platform keys include:
    • AWS: AWS_LAMBDA_FUNCTION_NAME, AWS_REGION, etc.
    • Azure App Service: WEBSITE_SITE_NAME, WEBSITE_RESOURCE_GROUP, etc.
    • Azure Functions: FUNCTIONS_WORKER_RUNTIME, FUNCTIONS_EXTENSION_VERSION, etc.
    • GCP Functions: FUNCTION_NAME, FUNCTION_REGION, K_SERVICE, etc.
    • CLR Profiler: CORECLR_PROFILER, CORECLR_PROFILER_PATH, COR_PROFILER, etc.
    • Kubernetes: KUBERNETES_SERVICE_HOST, DD_ENTITY_ID, etc.

Nuke validation step:

Target CheckConfigurationKeysAgainstJsonValidations => _ => _
    .Description("Validates that all ConfigurationKeys constants are present in supported-configurations.json")
    .After(CompileManagedSrc)
    .Executes(() =>
    {
        // 1. Parse supported-configurations.json
        // 2. Use reflection to extract all const string fields from ConfigurationKeys
        // 3. Compare and report:
        //    - Keys in code but missing from JSON (ERROR)
        //    - Keys in JSON but missing from code (WARNING)
    });

Validation logic:

  • Loads the net6.0 assembly to analyze ConfigurationKeys class
  • Extracts all public const string fields from ConfigurationKeys and nested classes
  • Compares against supportedConfigurations, aliases, and deprecations sections in JSON
  • Reports errors if keys are defined in code but not documented in JSON
  • Reports warnings if keys are in JSON but not in code (potential cleanup needed)

Updated references:

  • TracerSettings: Changed Azure/AWS/GCP key references to use PlatformKeys
  • Test files: Updated to use PlatformKeys for platform environment variables
  • Profiler code: Fixed references to use appropriate key classes

Added missing key:

  • DD_TRACE_ACTIVITY_LISTENER_ENABLED was being used in code but not defined as a constant

Test coverage

  • Nuke step runs after CompileManagedSrc to validate synchronization
  • Uses reflection to analyze compiled assemblies, ensuring accuracy
  • Reports detailed error messages with field paths (e.g., ConfigurationKeys.AzureAppService.SiteNameKey)
  • All existing tests updated to use correct key classes

Other details

Benefits:

  • Prevents drift: Compile-time check ensures JSON stays synchronized with code
  • Clear separation: Platform keys vs Datadog configuration keys are now distinct
  • Better documentation: supported-configurations.json only contains Datadog-specific keys
  • Easier maintenance: Developers get immediate feedback if they forget to update JSON

Example validation output:

Configuration keys defined in ConfigurationKeys but missing from supported-configurations.json:
  - DD_TRACE_ENABLED (defined as ConfigurationKeys.TraceEnabled)
  - DD_SERVICE (defined as ConfigurationKeys.ServiceName)

Configuration keys in supported-configurations.json but not defined in ConfigurationKeys:
  - DD_DEPRECATED_KEY

Future improvements:

  • Could add a code fix/analyzer to automatically suggest adding keys to JSON
  • Could generate JSON file automatically from ConfigurationKeys class
  • Could add validation to CI pipeline to block PRs that break synchronization

This PR builds on #7548 (add supported-configurations.json) and is a prerequisite for future PRs that will add Roslyn analyzers to enforce usage of these centralized constants.

@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch 2 times, most recently from 7dbf345 to 8d4256d Compare September 24, 2025 13:31
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from 65fbbec to 3bb22a8 Compare September 24, 2025 13:43
@dd-trace-dotnet-ci-bot
Copy link

dd-trace-dotnet-ci-bot bot commented Sep 24, 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 (7556) - mean (72ms)  : 71, 74
     .   : milestone, 72,
    master - mean (72ms)  : 71, 73
     .   : milestone, 72,

    section Baseline
    This PR (7556) - mean (68ms)  : 66, 71
     .   : milestone, 68,
    master - mean (68ms)  : 66, 70
     .   : milestone, 68,

    section CallTarget+Inlining+NGEN
    This PR (7556) - mean (1,052ms)  : 999, 1105
     .   : milestone, 1052,
    master - mean (1,056ms)  : 985, 1127
     .   : milestone, 1056,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7556) - mean (107ms)  : 105, 108
     .   : milestone, 107,
    master - mean (106ms)  : 105, 108
     .   : milestone, 106,

    section Baseline
    This PR (7556) - mean (106ms)  : 103, 108
     .   : milestone, 106,
    master - mean (106ms)  : 103, 108
     .   : milestone, 106,

    section CallTarget+Inlining+NGEN
    This PR (7556) - mean (742ms)  : 721, 763
     .   : milestone, 742,
    master - mean (750ms)  : 700, 799
     .   : milestone, 750,

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

    section Baseline
    This PR (7556) - mean (101ms)  : 98, 103
     .   : milestone, 101,
    master - mean (100ms)  : 98, 102
     .   : milestone, 100,

    section CallTarget+Inlining+NGEN
    This PR (7556) - mean (774ms)  : 731, 816
     .   : milestone, 774,
    master - mean (779ms)  : 738, 820
     .   : milestone, 779,

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

    section Baseline
    This PR (7556) - mean (92ms)  : 90, 94
     .   : milestone, 92,
    master - mean (92ms)  : 90, 95
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (7556) - mean (661ms)  : 646, 675
     .   : milestone, 661,
    master - mean (663ms)  : 648, 679
     .   : milestone, 663,

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

    section Baseline
    This PR (7556) - mean (194ms)  : 190, 198
     .   : milestone, 194,
    master - mean (194ms)  : 189, 199
     .   : milestone, 194,

    section CallTarget+Inlining+NGEN
    This PR (7556) - mean (1,168ms)  : 1111, 1226
     .   : milestone, 1168,
    master - mean (1,168ms)  : 1113, 1223
     .   : milestone, 1168,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7556) - mean (278ms)  : 274, 282
     .   : milestone, 278,
    master - mean (278ms)  : 275, 282
     .   : milestone, 278,

    section Baseline
    This PR (7556) - mean (278ms)  : 272, 284
     .   : milestone, 278,
    master - mean (278ms)  : 273, 283
     .   : milestone, 278,

    section CallTarget+Inlining+NGEN
    This PR (7556) - mean (940ms)  : 902, 979
     .   : milestone, 940,
    master - mean (944ms)  : 882, 1007
     .   : milestone, 944,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Bailout
    This PR (7556) - mean (282ms)  : 278, 286
     .   : milestone, 282,
    master - mean (282ms)  : 277, 287
     .   : milestone, 282,

    section Baseline
    This PR (7556) - mean (283ms)  : 278, 288
     .   : milestone, 283,
    master - mean (283ms)  : 276, 290
     .   : milestone, 283,

    section CallTarget+Inlining+NGEN
    This PR (7556) - mean (998ms)  : 964, 1032
     .   : milestone, 998,
    master - mean (996ms)  : 957, 1035
     .   : milestone, 996,

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

    section Baseline
    This PR (7556) - mean (271ms)  : 264, 278
     .   : milestone, 271,
    master - mean (270ms)  : 266, 274
     .   : milestone, 270,

    section CallTarget+Inlining+NGEN
    This PR (7556) - mean (853ms)  : 833, 873
     .   : milestone, 853,
    master - mean (857ms)  : 837, 877
     .   : milestone, 857,

Loading

@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from 3bb22a8 to cbb3593 Compare September 24, 2025 16:41
@anna-git anna-git changed the title Anna/config inversion check configurationkeys are in json [Config inversion] inversion check configurationkeys are in json Sep 25, 2025
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch 2 times, most recently from d340651 to aef9602 Compare September 25, 2025 12:24
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from 8d4256d to afa84af Compare September 25, 2025 12:24
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch 2 times, most recently from 57a0761 to 437db7e Compare September 25, 2025 15:01
@datadog-datadog-prod-us1

This comment has been minimized.

@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from afa84af to 05247c7 Compare September 29, 2025 11:55
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from 437db7e to e4fb133 Compare September 29, 2025 11:55
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from 05247c7 to c50a9c1 Compare September 29, 2025 12:26
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from e4fb133 to bc2ad02 Compare September 29, 2025 12:27
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from c50a9c1 to 7dbfceb Compare September 29, 2025 14:07
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch 2 times, most recently from 5a9a712 to 6d51e76 Compare September 29, 2025 14:12
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from 7dbfceb to 56b17eb Compare September 29, 2025 15:37
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from 6d51e76 to f9bb5ba Compare September 29, 2025 15:37
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from 56b17eb to 7fb125e Compare September 29, 2025 17:34
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from f9bb5ba to 1bc1b38 Compare September 29, 2025 17:34
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from 7fb125e to 25fa9a0 Compare October 2, 2025 09:49
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch 2 times, most recently from 3ab9889 to e31264e Compare October 3, 2025 10:33
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from 25fa9a0 to b7cefbd Compare October 6, 2025 14:23
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from e31264e to 416672b Compare October 6, 2025 14:23
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from b7cefbd to 6dfb9a9 Compare October 7, 2025 09:53
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from 416672b to be5f635 Compare October 7, 2025 09:53
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch from 1cfe73f to 096952f Compare October 10, 2025 11:32
@anna-git anna-git force-pushed the anna/config-inversion-check-configurationkeys-are-in-json branch from b202c12 to 1f81c8c Compare October 10, 2025 11:32
@anna-git anna-git changed the title [Config inversion] Cleanup configuration keys constants, and check them against local json file [Config registry] Cleanup configuration keys constants, and check them against local json file Oct 10, 2025
@anna-git anna-git marked this pull request as ready for review October 10, 2025 14:02
@anna-git anna-git requested review from a team as code owners October 10, 2025 14:02
@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-supported-config-file branch from 096952f to 869594a Compare October 21, 2025 16:14
@anna-git anna-git force-pushed the anna/config-inversion-supported-config-file branch 9 times, most recently from bb9d5d8 to f819c41 Compare October 28, 2025 14:33
Base automatically changed from anna/config-inversion-supported-config-file to master October 28, 2025 17:37
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