Config refactor - fix struct/class T nullable ref issues, and allow…

… "raw" access to `ConfigurationResult` (#5715)

## Summary of changes

- Fixes bugs related to nullable reference types when used with generics
- Allow retrieving the "raw" `ConfigurationResult<>` directly from a
configuration builder.

## Reason for change

The first issue is the method `T? GetAs<T>(key)` doesn't behave as we
(previously) believed. If you call `GetAs<int>(somekey)` we previously
were expecting it to return `null`, but it doesn't - the best you can do
is return `default` i.e. 0.

There's no technical way around this limitation AFAIK other than having
separate methods, or having methods that are guaranteed non-null
returning. So in this PR we have both.

Additionally, there are some cases where it's useful to provide access
to the "raw" `ConfigurationResut<T>`. One example is shown in this PR
where the security settings want to know both whether the value was set
_and_ what the value is, while also providing a default. It will also be
useful in subsequent PRs for refactoring the OTel config.

## Implementation details

- Expose additional `Get*Result()` methods on `ConfigurationBuilder`,
e.g. `GetBoolResult()` returns a
`StructConfigurationResultWithKey<bool>`, `GetStringResult()` returns a
`ClassConfigurationResultWithKey<bool>`,
- You can use this to inspect the result to see if the key was present
(for example)
- You can then call `WithDefault(T default)` on this to get the correct
value (and record the default in telemetry correctly)

## Test coverage

Mostly a refactor, should be covered by existing tests

## Other details
<!-- Fixes #{issue} -->

This is part of a big stack of config refactoring PRs:

- #5713
- #5714
- #5715 (this PR)
- #5716
- #5717

<!-- ⚠️ 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. -->

tracer/src/Datadog.Trace/AppSec/SecuritySettings.cs

@@ -35,21 +35,13 @@ public SecuritySettings(IConfigurationSource? source, IConfigurationTelemetry te
                                  .WithKeys(ConfigurationKeys.AppSec.JsonBlockedTemplate)
                                  .AsString(SecurityConstants.BlockedJsonTemplate);
 
-            bool isEnabledSet = true;
-
-            bool GetEnabledDefaultValue()
-            {
-                isEnabledSet = false;
-                return false;
-            }
-
             // both should default to false
             var enabledEnvVar = config
                                .WithKeys(ConfigurationKeys.AppSec.Enabled)
-                               .AsBool(GetEnabledDefaultValue, null);
+                               .AsBoolResult();
 
-            Enabled = enabledEnvVar.Value;
-            CanBeToggled = !isEnabledSet;
+            CanBeToggled = !enabledEnvVar.ConfigurationResult.IsValid;
+            Enabled = enabledEnvVar.WithDefault(false);
 
             Rules = config.WithKeys(ConfigurationKeys.AppSec.Rules).AsString();
             CustomIpHeader = config.WithKeys(ConfigurationKeys.AppSec.CustomIpHeader).AsString();

tracer/src/Datadog.Trace/Configuration/ConfigurationSources/Telemetry/ConfigurationBuilder.cs

@@ -63,6 +63,59 @@ public ConfigurationBuilder(IConfigurationSource source, IConfigurationTelemetry
 
     public HasKeys WithKeys(string key, string fallbackKey1, string fallbackKey2, string fallbackKey3) => new(_source, _telemetry, key, fallbackKey1, fallbackKey2, fallbackKey3);
 
+    private static bool TryHandleResult<T>(
+        IConfigurationTelemetry telemetry,
+        string key,
+        ConfigurationResult<T> result,
+        bool recordValue,
+        Func<DefaultResult<T>>? getDefaultValue,
+        [NotNullIfNotNull(nameof(getDefaultValue))] out T? value)
+    {
+        if (result is { Result: { } ddResult, IsValid: true })
+        {
+            value = ddResult;
+            return true;
+        }
+
+        // don't have a default value, so caller (which knows what the <T> is needs
+        // to return the default value. Necessary because we can't create a generic
+        // method that works "correctly" for both value types and reference types.
+        if (getDefaultValue is null)
+        {
+            value = default;
+            return false;
+        }
+
+        var defaultValue = getDefaultValue();
+        RecordTelemetry(telemetry, key, recordValue, defaultValue);
+
+        // The compiler complains about this, because technically you _could_ call it as `TryHandleResult<int?>` (for example)
+        // in which case Func<DefaultResult<T>> _could_ return a `null` value, so the `[NotNullIfNotNull]` annotation
+        // would be wrong. In practice, we control the calls to this method, and we know that T is always non-null
+        // so it's safe to use the dammit here.
+        value = defaultValue.Result!;
+        return true;
+    }
+
+    private static void RecordTelemetry<T>(IConfigurationTelemetry telemetry, string key, bool recordValue, DefaultResult<T> defaultValue)
+    {
+        switch (defaultValue.Result)
+        {
+            case int intVal:
+                telemetry.Record(key, intVal, ConfigurationOrigins.Default);
+                break;
+            case double doubleVal:
+                telemetry.Record(key, doubleVal, ConfigurationOrigins.Default);
+                break;
+            case bool boolVal:
+                telemetry.Record(key, boolVal, ConfigurationOrigins.Default);
+                break;
+            default:
+                telemetry.Record(key, defaultValue.TelemetryValue, recordValue, ConfigurationOrigins.Default);
+                break;
+        }
+    }
+
     internal readonly struct HasKeys
     {
         public HasKeys(ITelemeteredConfigurationSource source, IConfigurationTelemetry telemetry, string key, string? fallbackKey1 = null, string? fallbackKey2 = null, string? fallbackKey3 = null)
@@ -196,26 +249,32 @@ public string AsString(string defaultValue, Func<string, bool>? validator)
             return defaultValue;
         }
 
+        // We have to use different methods for class/struct when we _don't_ have a null value, because NRTs don't work properly otherwise
         [return: NotNullIfNotNull(nameof(getDefaultValue))]
-        public T? GetAs<T>(Func<DefaultResult<T>>? getDefaultValue, Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
+        public T GetAs<T>(Func<DefaultResult<T>> getDefaultValue, Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
         {
             var result = GetAs(validator, converter);
+            return TryHandleResult(Telemetry, Key, result, recordValue: true, getDefaultValue, out var value)
+                       ? value
+                       : default!; // TryHandleResult always returns true as getDefaultValue != null
+        }
 
-            // We have a valid value
-            if (result is { Result: { } value, IsValid: true })
-            {
-                return value;
-            }
-
-            // don't have a valid value
-            if (getDefaultValue is null)
-            {
-                return default;
-            }
+        public T? GetAsClass<T>(Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
+            where T : class
+        {
+            var result = GetAs(validator, converter);
+            return TryHandleResult(Telemetry, Key, result, recordValue: true, getDefaultValue: null, out var value)
+                       ? value
+                       : null;
+        }
 
-            var defaultValue = getDefaultValue();
-            Telemetry.Record(Key, defaultValue.TelemetryValue, recordValue: true, ConfigurationOrigins.Default);
-            return defaultValue.Result!;
+        public T? GetAsStruct<T>(Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
+            where T : struct
+        {
+            var result = GetAs(validator, converter);
+            return TryHandleResult(Telemetry, Key, result, recordValue: true, getDefaultValue: null, out var value)
+                       ? value
+                       : null;
         }
 
         [return: NotNullIfNotNull(nameof(getDefaultValue))]
@@ -587,6 +646,90 @@ public bool AsBoolWithOpenTelemetryMapping(bool defaultValue, string openTelemet
             return null;
         }
 
+        // ****************
+        // Raw result accessors
+        // ****************
+        public ClassConfigurationResultWithKey<string> AsStringResult()
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetStringResult(validator: null, converter: null, recordValue: true));
+
+        public ClassConfigurationResultWithKey<string> AsStringResult(Func<string, ParsingResult<string>>? converter)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetStringResult(validator: null, converter, recordValue: true));
+
+        public ClassConfigurationResultWithKey<string> AsStringResult(Func<string, bool>? validator, Func<string, ParsingResult<string>>? converter)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetStringResult(validator, converter, recordValue: true));
+
+        public ClassConfigurationResultWithKey<string> AsRedactedStringResult()
+            => new(Telemetry, Key, recordValue: false, configurationResult: GetStringResult(validator: null, converter: null, recordValue: false));
+
+        public ClassConfigurationResultWithKey<string> AsRedactedStringResult(Func<string, ParsingResult<string>>? converter)
+            => new(Telemetry, Key, recordValue: false, configurationResult: GetStringResult(validator: null, converter, recordValue: false));
+
+        public ClassConfigurationResultWithKey<string> AsRedactedStringResult(Func<string, bool>? validator, Func<string, ParsingResult<string>>? converter)
+            => new(Telemetry, Key, recordValue: false, configurationResult: GetStringResult(validator, converter, recordValue: false));
+
+        public ClassConfigurationResultWithKey<string> AsStringResult(Func<string, bool>? validator, Func<string, ParsingResult<string>>? converter, bool recordValue)
+            => new(Telemetry, Key, recordValue, GetStringResult(validator, converter, recordValue));
+
+        // bool
+        public StructConfigurationResultWithKey<bool> AsBoolResult()
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetBoolResult(validator: null, converter: null));
+
+        public StructConfigurationResultWithKey<bool> AsBoolResult(Func<string, ParsingResult<bool>>? converter)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetBoolResult(validator: null, converter));
+
+        public StructConfigurationResultWithKey<bool> AsBoolResult(Func<bool, bool>? validator, Func<string, ParsingResult<bool>>? converter)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetBoolResult(validator, converter));
+
+        // T
+        public ClassConfigurationResultWithKey<T> GetAsClassResult<T>(Func<string, ParsingResult<T>> converter)
+            where T : class
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetAs(validator: null, converter));
+
+        public ClassConfigurationResultWithKey<T> GetAsClassResult<T>(Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
+            where T : class
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetAs(validator, converter));
+
+        public StructConfigurationResultWithKey<T> GetAsStructResult<T>(Func<string, ParsingResult<T>> converter)
+            where T : struct
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetAs(validator: null, converter));
+
+        public StructConfigurationResultWithKey<T> GetAsStructResult<T>(Func<T, bool>? validator, Func<string, ParsingResult<T>> converter)
+            where T : struct
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetAs(validator, converter));
+
+        // int
+        public StructConfigurationResultWithKey<int> AsInt32Result()
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetInt32Result(validator: null, converter: null));
+
+        public StructConfigurationResultWithKey<int> AsInt32Result(Func<string, ParsingResult<int>>? converter)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetInt32Result(validator: null, converter));
+
+        public StructConfigurationResultWithKey<int> AsInt32Result(Func<int, bool>? validator, Func<string, ParsingResult<int>>? converter)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetInt32Result(validator, converter));
+
+        // double
+        public StructConfigurationResultWithKey<double> AsDoubleResult()
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetDoubleResult(validator: null, converter: null));
+
+        public StructConfigurationResultWithKey<double> AsDoubleResult(Func<string, ParsingResult<double>>? converter)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetDoubleResult(validator: null, converter));
+
+        public StructConfigurationResultWithKey<double> AsDoubleResult(Func<double, bool>? validator, Func<string, ParsingResult<double>>? converter)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetDoubleResult(validator, converter));
+
+        // dictionary
+        public ClassConfigurationResultWithKey<IDictionary<string, string>> AsDictionaryResult()
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetDictionaryResult(allowOptionalMappings: false, separator: ':'));
+
+        public ClassConfigurationResultWithKey<IDictionary<string, string>> AsDictionaryResult(bool allowOptionalMappings)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetDictionaryResult(allowOptionalMappings, separator: ':'));
+
+        public ClassConfigurationResultWithKey<IDictionary<string, string>> AsDictionaryResult(char separator)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetDictionaryResult(allowOptionalMappings: false, separator));
+
+        public ClassConfigurationResultWithKey<IDictionary<string, string>> AsDictionaryResult(bool allowOptionalMappings, char separator)
+            => new(Telemetry, Key, recordValue: true, configurationResult: GetDictionaryResult(allowOptionalMappings, separator));
+
         private ConfigurationResult<string> GetStringResult(Func<string, bool>? validator, Func<string, ParsingResult<string>>? converter, bool recordValue)
             => converter is null
                    ? GetResult(AsStringSelector, validator, recordValue)
@@ -694,4 +837,48 @@ private ConfigurationResult<IDictionary<string, string>> GetDictionaryResult(boo
             return result;
         }
     }
+
+    internal readonly struct StructConfigurationResultWithKey<T>(IConfigurationTelemetry telemetry, string key, bool recordValue, ConfigurationResult<T> configurationResult)
+        where T : struct
+    {
+        public readonly string Key = key;
+        public readonly IConfigurationTelemetry Telemetry = telemetry;
+        public readonly bool RecordValue = recordValue;
+        public readonly ConfigurationResult<T> ConfigurationResult = configurationResult;
+
+        public T WithDefault(T defaultValue)
+            => WithDefault(getDefaultValue: () => defaultValue);
+
+        public T WithDefault(Func<DefaultResult<T>> getDefaultValue)
+        {
+            if (TryHandleResult(Telemetry, Key, ConfigurationResult, RecordValue, getDefaultValue, out var value))
+            {
+                return value;
+            }
+
+            return default; // should never be invoked because we have a value for getDefaultValue
+        }
+    }
+
+    internal readonly struct ClassConfigurationResultWithKey<T>(IConfigurationTelemetry telemetry, string key, bool recordValue, ConfigurationResult<T> configurationResult)
+        where T : class
+    {
+        public readonly string Key = key;
+        public readonly IConfigurationTelemetry Telemetry = telemetry;
+        public readonly bool RecordValue = recordValue;
+        public readonly ConfigurationResult<T> ConfigurationResult = configurationResult;
+
+        public T WithDefault(T defaultValue)
+            => WithDefault(getDefaultValue: () => defaultValue);
+
+        public T WithDefault(Func<DefaultResult<T>> getDefaultValue)
+        {
+            if (TryHandleResult(Telemetry, Key, ConfigurationResult, RecordValue, getDefaultValue, out var value))
+            {
+                return value;
+            }
+
+            return default!; // should never be invoked because we have a value for getDefaultValue
+        }
+    }
 }

tracer/src/Datadog.Trace/Configuration/DynamicConfigurationManager.cs

@@ -77,7 +77,9 @@ private static void OnConfigurationChanged(ConfigurationBuilder settings)
                 TraceEnabled = settings.WithKeys(ConfigurationKeys.TraceEnabled).AsBool(),
                 // RuntimeMetricsEnabled = settings.WithKeys(ConfigurationKeys.RuntimeMetricsEnabled).AsBool(),
                 // DataStreamsMonitoringEnabled = settings.WithKeys(ConfigurationKeys.DataStreamsMonitoring.Enabled).AsBool(),
-                SamplingRules = settings.WithKeys(ConfigurationKeys.CustomSamplingRules).GetAs<string>(null, null, s => s),
+                // Note: Calling GetAsClass<string>() here instead of GetAsString() as we need to get the
+                // "serialized JToken", which in JsonConfigurationSource is different, as it allows for non-string tokens
+                SamplingRules = settings.WithKeys(ConfigurationKeys.CustomSamplingRules).GetAsClass<string>(validator: null, converter: s => s),
                 GlobalSamplingRate = settings.WithKeys(ConfigurationKeys.GlobalSamplingRate).AsDouble(),
                 // SpanSamplingRules = settings.WithKeys(ConfigurationKeys.SpanSamplingRules).AsString(),
                 LogsInjectionEnabled = settings.WithKeys(ConfigurationKeys.LogsInjectionEnabled).AsBool(),

tracer/test/Datadog.Trace.Tests/Configuration/Telemetry/ConfigurationBuilderTests.cs

@@ -627,10 +627,9 @@ public void GetAs_ReturnsNullWhenCantParseAndNoDefault(string value)
 
             var actual = new ConfigurationBuilder(source, _telemetry)
                         .WithKeys("key")
-                        .GetAs(
-                             getDefaultValue: null,
+                        .GetAsStruct(
                              validator: null,
-                             converter: _nullableConverter);
+                             converter: _converter);
 
             actual.Should().BeNull();
         }