Skip to content

Commit 74d5e0d

Browse files
andrewlockveerbia
authored andcommitted
Simplify duplication in IConfigurationSource (#6386)
## Summary of changes - Delete legacy `IConfigurationSource` - Rename `ITelmeteredConfigurationSource` => `IConfigurationSource` - Remove some "public" API workarounds - Make some APIs `public` instead of `internal` (just to satisfy compiler) - Fix tests ## Reason for change `ITelmeteredConfigurationSource` was introduced when `IConfigurationSource` was public, and so could not be changed. We have since removed that interface from the public API (it's not in `Datadog.trace.Manual`) so now it simply adds confusion and complexity (as it should _not_ be used internally in Datadog.Trace). ## Implementation details This PR looks more complex than it is, because in order to replace all the usages of the old `IConfigurationSource` with the new one, I had to mark some other APIs public (and document them to satisfy the analyzers). It is mostly - Delete legacy `IConfigurationSource` - Rename `ITelmeteredConfigurationSource` => `IConfigurationSource` - Fix any errors I took the opportunity to also remove some of the `*Internal` methods which were used to avoid calling the "public" APIs; seeing as these aren't _actually_ public any more, that's just unnecessary duplication. ## Test coverage The testing was a pain. The `ConfigurationBuilder` tests were all designed to check that `ITelmeteredConfigurationSource` gave the same results as `IConfigurationSource` (while never _explicitly_ specifying what the "expected" behaviour was. I took some time to enumerate the _actual_ expected values for the `NameValueCollection` source, but the `Json` source has very different behaviour that is more of a pain to test, so I chose to simplify a lot there. We could do a _lot_ to clean up those tests, but I didn't want to add even more complexity in this PR. ## Other details To help "fix" the ASM tests I introduced a `DictionaryObjectConfigurationSource`, in which you pass in a `Dictionary<string, object>`. This is useful for testing (as it was previously used) but is actually going to be useful for a future clean-up PR too, so I kept it in Datadog.Trace instead of in the test project. Part of stack - #6370 - #6376 - #6385 - #6386 👈 This PR - #6397 - #6399 - #6400 - #6405 - #6408
1 parent 57c5efb commit 74d5e0d

File tree

37 files changed

+839
-1207
lines changed

37 files changed

+839
-1207
lines changed

tracer/src/Datadog.Trace.Tools.Runner/Utils.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -402,9 +402,9 @@ public static string GetEnvironmentVariable(string key, string defaultValue = nu
402402
env[ConfigurationKeys.AgentUri] = agentUrl;
403403
}
404404

405-
var configurationSource = new CompositeConfigurationSourceInternal();
406-
configurationSource.AddInternal(new NameValueConfigurationSource(env, ConfigurationOrigins.EnvVars));
407-
configurationSource.AddInternal(GlobalConfigurationSource.Instance);
405+
var configurationSource = new CompositeConfigurationSource();
406+
configurationSource.Add(new NameValueConfigurationSource(env, ConfigurationOrigins.EnvVars));
407+
configurationSource.Add(GlobalConfigurationSource.Instance);
408408

409409
var tracerSettings = new TracerSettings(configurationSource, new ConfigurationTelemetry(), new OverrideErrorLog());
410410
var settings = new ImmutableTracerSettings(tracerSettings, unusedParamNotToUsePublicApi: true);

tracer/src/Datadog.Trace/Ci/Configuration/CIVisibilitySettings.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,13 @@ private TracerSettings InitializeTracerSettings()
301301
{
302302
var source = GlobalConfigurationSource.CreateDefaultConfigurationSource();
303303
var defaultExcludedUrlSubstrings = string.Empty;
304-
var configResult = ((ITelemeteredConfigurationSource)source).GetString(ConfigurationKeys.HttpClientExcludedUrlSubstrings, NullConfigurationTelemetry.Instance, validator: null, recordValue: false);
304+
var configResult = source.GetString(ConfigurationKeys.HttpClientExcludedUrlSubstrings, NullConfigurationTelemetry.Instance, validator: null, recordValue: false);
305305
if (configResult is { IsValid: true, Result: { } substrings } && !string.IsNullOrWhiteSpace(substrings))
306306
{
307307
defaultExcludedUrlSubstrings = substrings + ", ";
308308
}
309309

310-
source.InsertInternal(0, new NameValueConfigurationSource(
310+
source.Insert(0, new NameValueConfigurationSource(
311311
new NameValueCollection
312312
{
313313
[ConfigurationKeys.HttpClientExcludedUrlSubstrings] = defaultExcludedUrlSubstrings + "/session/FakeSessionIdForPollingPurposes",

tracer/src/Datadog.Trace/Configuration/ConfigurationSources/CompositeConfigurationSource.cs

+15-144
Original file line numberDiff line numberDiff line change
@@ -21,211 +21,82 @@ namespace Datadog.Trace.Configuration
2121
/// <summary>
2222
/// Represents one or more configuration sources.
2323
/// </summary>
24-
public class CompositeConfigurationSource : IConfigurationSource, IEnumerable<IConfigurationSource>, ITelemeteredConfigurationSource
24+
internal class CompositeConfigurationSource : IConfigurationSource, IEnumerable<IConfigurationSource>
2525
{
26-
private readonly List<ITelemeteredConfigurationSource> _sources = new();
27-
28-
/// <summary>
29-
/// Initializes a new instance of the <see cref="CompositeConfigurationSource"/> class.
30-
/// </summary>
31-
[PublicApi]
32-
public CompositeConfigurationSource()
33-
{
34-
TelemetryFactory.Metrics.Record(PublicApiUsage.CompositeConfigurationSource_Ctor);
35-
}
36-
37-
private protected CompositeConfigurationSource(bool unusedParamNotToUsePublicApi)
38-
{
39-
// unused parameter is to give us a non-public API we can use
40-
}
26+
private readonly List<IConfigurationSource> _sources = new();
4127

4228
/// <summary>
4329
/// Adds a new configuration source to this instance.
4430
/// </summary>
4531
/// <param name="source">The configuration source to add.</param>
46-
[PublicApi]
4732
public void Add(IConfigurationSource source)
4833
{
49-
TelemetryFactory.Metrics.Record(PublicApiUsage.CompositeConfigurationSource_Add);
5034
if (source == null) { ThrowHelper.ThrowArgumentNullException(nameof(source)); }
5135

52-
AddInternal(source);
36+
_sources.Add(source);
5337
}
5438

5539
/// <summary>
56-
/// Inserts an element into the <see cref="CompositeConfigurationSource"/> at the specified index.
40+
/// Inserts an element into the <see cref="Datadog.Trace.Configuration.CompositeConfigurationSource"/> at the specified index.
5741
/// </summary>
5842
/// <param name="index">The zero-based index at which <paramref name="item"/> should be inserted.</param>
5943
/// <param name="item">The configuration source to insert.</param>
60-
[PublicApi]
6144
public void Insert(int index, IConfigurationSource item)
6245
{
63-
TelemetryFactory.Metrics.Record(PublicApiUsage.CompositeConfigurationSource_Insert);
6446
if (item == null) { ThrowHelper.ThrowArgumentNullException(nameof(item)); }
6547

66-
InsertInternal(index, item);
67-
}
68-
69-
/// <summary>
70-
/// Gets the <see cref="string"/> value of the first setting found with
71-
/// the specified key from the current list of configuration sources.
72-
/// Sources are queried in the order in which they were added.
73-
/// </summary>
74-
/// <param name="key">The key that identifies the setting.</param>
75-
/// <returns>The value of the setting, or <c>null</c> if not found.</returns>
76-
[PublicApi]
77-
public string? GetString(string key)
78-
{
79-
var value = _sources
80-
.Select(source => source.GetString(key, NullConfigurationTelemetry.Instance, validator: null, recordValue: true))
81-
.FirstOrDefault(value => value.IsValid, ConfigurationResult<string>.NotFound());
82-
return value.IsValid ? value.Result : null;
83-
}
84-
85-
/// <summary>
86-
/// Gets the <see cref="int"/> value of the first setting found with
87-
/// the specified key from the current list of configuration sources.
88-
/// Sources are queried in the order in which they were added.
89-
/// </summary>
90-
/// <param name="key">The key that identifies the setting.</param>
91-
/// <returns>The value of the setting, or <c>null</c> if not found.</returns>
92-
[PublicApi]
93-
public int? GetInt32(string key)
94-
{
95-
var value = _sources
96-
.Select(source => source.GetInt32(key, NullConfigurationTelemetry.Instance, validator: null))
97-
.FirstOrDefault(value => value.IsValid, ConfigurationResult<int>.NotFound());
98-
return value.IsValid ? value.Result : null;
99-
}
100-
101-
/// <summary>
102-
/// Gets the <see cref="double"/> value of the first setting found with
103-
/// the specified key from the current list of configuration sources.
104-
/// Sources are queried in the order in which they were added.
105-
/// </summary>
106-
/// <param name="key">The key that identifies the setting.</param>
107-
/// <returns>The value of the setting, or <c>null</c> if not found.</returns>
108-
[PublicApi]
109-
public double? GetDouble(string key)
110-
{
111-
var value = _sources
112-
.Select(source => source.GetDouble(key, NullConfigurationTelemetry.Instance, validator: null))
113-
.FirstOrDefault(value => value.IsValid, ConfigurationResult<double>.NotFound());
114-
return value.IsValid ? value.Result : null;
115-
}
116-
117-
/// <summary>
118-
/// Gets the <see cref="bool"/> value of the first setting found with
119-
/// the specified key from the current list of configuration sources.
120-
/// Sources are queried in the order in which they were added.
121-
/// </summary>
122-
/// <param name="key">The key that identifies the setting.</param>
123-
/// <returns>The value of the setting, or <c>null</c> if not found.</returns>
124-
[PublicApi]
125-
public bool? GetBool(string key)
126-
{
127-
var value = _sources
128-
.Select(source => source.GetBool(key, NullConfigurationTelemetry.Instance, validator: null))
129-
.FirstOrDefault(value => value.IsValid, ConfigurationResult<bool>.NotFound());
130-
return value.IsValid ? value.Result : null;
131-
}
132-
133-
internal void AddInternal(IConfigurationSource source)
134-
{
135-
var telemeteredSource = source as ITelemeteredConfigurationSource ?? new CustomTelemeteredConfigurationSource(source);
136-
_sources.Add(telemeteredSource);
137-
}
138-
139-
internal void InsertInternal(int index, IConfigurationSource source)
140-
{
141-
var telemeteredSource = source as ITelemeteredConfigurationSource ?? new CustomTelemeteredConfigurationSource(source);
142-
_sources.Insert(index, telemeteredSource);
143-
}
144-
145-
/// <inheritdoc />
146-
[PublicApi]
147-
IEnumerator<IConfigurationSource> IEnumerable<IConfigurationSource>.GetEnumerator()
148-
{
149-
return _sources
150-
.Select(
151-
x => x as IConfigurationSource
152-
?? ((CustomTelemeteredConfigurationSource)x).Source)
153-
.GetEnumerator();
154-
}
155-
156-
/// <inheritdoc />
157-
[PublicApi]
158-
IEnumerator IEnumerable.GetEnumerator()
159-
{
160-
return _sources
161-
.Select(
162-
x => x as IConfigurationSource
163-
?? ((CustomTelemeteredConfigurationSource)x).Source)
164-
.GetEnumerator();
48+
_sources.Insert(index, item);
16549
}
16650

16751
/// <inheritdoc />
168-
[PublicApi]
169-
public IDictionary<string, string>? GetDictionary(string key)
170-
{
171-
var value = _sources
172-
.Select(source => source.GetDictionary(key, NullConfigurationTelemetry.Instance, validator: null))
173-
.FirstOrDefault(value => value.IsValid, ConfigurationResult<IDictionary<string, string>>.NotFound());
174-
return value.IsValid ? value.Result : null;
175-
}
52+
IEnumerator<IConfigurationSource> IEnumerable<IConfigurationSource>.GetEnumerator() => _sources.GetEnumerator();
17653

17754
/// <inheritdoc />
17855
[PublicApi]
179-
public IDictionary<string, string>? GetDictionary(string key, bool allowOptionalMappings)
180-
{
181-
var value = _sources
182-
.Select(source => source.GetDictionary(key, NullConfigurationTelemetry.Instance, validator: null, allowOptionalMappings, separator: ':'))
183-
.FirstOrDefault(value => value.IsValid, ConfigurationResult<IDictionary<string, string>>.NotFound());
184-
return value.IsValid ? value.Result : null;
185-
}
56+
IEnumerator IEnumerable.GetEnumerator() => _sources.GetEnumerator();
18657

18758
/// <inheritdoc />
188-
bool ITelemeteredConfigurationSource.IsPresent(string key)
59+
public bool IsPresent(string key)
18960
=> _sources.Select(source => source.IsPresent(key)).FirstOrDefault(value => value);
19061

19162
/// <inheritdoc />
192-
ConfigurationResult<string> ITelemeteredConfigurationSource.GetString(string key, IConfigurationTelemetry telemetry, Func<string, bool>? validator, bool recordValue)
63+
public ConfigurationResult<string> GetString(string key, IConfigurationTelemetry telemetry, Func<string, bool>? validator, bool recordValue)
19364
=> _sources
19465
.Select(source => source.GetString(key, telemetry, validator, recordValue))
19566
.FirstOrDefault(value => value.IsValid, ConfigurationResult<string>.NotFound());
19667

19768
/// <inheritdoc />
198-
ConfigurationResult<int> ITelemeteredConfigurationSource.GetInt32(string key, IConfigurationTelemetry telemetry, Func<int, bool>? validator)
69+
public ConfigurationResult<int> GetInt32(string key, IConfigurationTelemetry telemetry, Func<int, bool>? validator)
19970
=> _sources
20071
.Select(source => source.GetInt32(key, telemetry, validator))
20172
.FirstOrDefault(value => value.IsValid, ConfigurationResult<int>.NotFound());
20273

20374
/// <inheritdoc />
204-
ConfigurationResult<double> ITelemeteredConfigurationSource.GetDouble(string key, IConfigurationTelemetry telemetry, Func<double, bool>? validator)
75+
public ConfigurationResult<double> GetDouble(string key, IConfigurationTelemetry telemetry, Func<double, bool>? validator)
20576
=> _sources
20677
.Select(source => source.GetDouble(key, telemetry, validator))
20778
.FirstOrDefault(value => value.IsValid, ConfigurationResult<double>.NotFound());
20879

20980
/// <inheritdoc />
210-
ConfigurationResult<bool> ITelemeteredConfigurationSource.GetBool(string key, IConfigurationTelemetry telemetry, Func<bool, bool>? validator)
81+
public ConfigurationResult<bool> GetBool(string key, IConfigurationTelemetry telemetry, Func<bool, bool>? validator)
21182
=> _sources
21283
.Select(source => source.GetBool(key, telemetry, validator))
21384
.FirstOrDefault(value => value.IsValid, ConfigurationResult<bool>.NotFound());
21485

21586
/// <inheritdoc />
216-
ConfigurationResult<IDictionary<string, string>> ITelemeteredConfigurationSource.GetDictionary(string key, IConfigurationTelemetry telemetry, Func<IDictionary<string, string>, bool>? validator)
87+
public ConfigurationResult<IDictionary<string, string>> GetDictionary(string key, IConfigurationTelemetry telemetry, Func<IDictionary<string, string>, bool>? validator)
21788
=> _sources
21889
.Select(source => source.GetDictionary(key, telemetry, validator))
21990
.FirstOrDefault(value => value.IsValid, ConfigurationResult<IDictionary<string, string>>.NotFound());
22091

22192
/// <inheritdoc />
222-
ConfigurationResult<IDictionary<string, string>> ITelemeteredConfigurationSource.GetDictionary(string key, IConfigurationTelemetry telemetry, Func<IDictionary<string, string>, bool>? validator, bool allowOptionalMappings, char separator)
93+
public ConfigurationResult<IDictionary<string, string>> GetDictionary(string key, IConfigurationTelemetry telemetry, Func<IDictionary<string, string>, bool>? validator, bool allowOptionalMappings, char separator)
22394
=> _sources
22495
.Select(source => source.GetDictionary(key, telemetry, validator, allowOptionalMappings, separator))
22596
.FirstOrDefault(value => value.IsValid, ConfigurationResult<IDictionary<string, string>>.NotFound());
22697

22798
/// <inheritdoc />
228-
ConfigurationResult<T> ITelemeteredConfigurationSource.GetAs<T>(string key, IConfigurationTelemetry telemetry, Func<string, ParsingResult<T>> converter, Func<T, bool>? validator, bool recordValue)
99+
public ConfigurationResult<T> GetAs<T>(string key, IConfigurationTelemetry telemetry, Func<string, ParsingResult<T>> converter, Func<T, bool>? validator, bool recordValue)
229100
=> _sources
230101
.Select(source => source.GetAs<T>(key, telemetry, converter, validator, recordValue))
231102
.FirstOrDefault(value => value.IsValid, ConfigurationResult<T>.NotFound());

tracer/src/Datadog.Trace/Configuration/ConfigurationSources/CompositeConfigurationSourceInternal.cs

-15
This file was deleted.

tracer/src/Datadog.Trace/Configuration/ConfigurationSources/DictionaryConfigurationSource.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public DictionaryConfigurationSource(IReadOnlyDictionary<string, string> diction
2121

2222
internal override ConfigurationOrigins Origin => ConfigurationOrigins.Code;
2323

24-
public override string? GetString(string key)
24+
protected override string? GetString(string key)
2525
{
2626
_dictionary.TryGetValue(key, out var value);
2727
return value;

0 commit comments

Comments
 (0)