-
Couldn't load subscription status.
- Fork 123
[FileBasedConfiguration] Propagators Configuration #4482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d54b417
6ae1132
2e21a16
bc654b2
9e47fc6
6409401
a19d41f
62af6ff
18e4ba5
f720f17
aa47f28
c35d39f
588a1ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| using Vendors.YamlDotNet.Serialization; | ||
|
|
||
| namespace OpenTelemetry.AutoInstrumentation.Configurations.FileBasedConfiguration; | ||
|
|
||
| internal class PropagatorConfiguration | ||
| { | ||
| /// <summary> | ||
| /// Gets or sets the composite propagator configuration. | ||
| /// </summary> | ||
| [YamlMember(Alias = "composite")] | ||
| public Dictionary<string, object>? Composite { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the composite list for the propagator. | ||
| /// </summary> | ||
| [YamlMember(Alias = "composite_list")] | ||
| public string? CompositeList { get; set; } | ||
|
|
||
| public IReadOnlyList<Propagator> GetEnabledPropagators() | ||
| { | ||
| if (Composite is null && string.IsNullOrEmpty(CompositeList)) | ||
| { | ||
| return []; | ||
| } | ||
|
|
||
| var propagatorNames = (Composite?.Keys ?? Enumerable.Empty<string>()) | ||
| .Concat(!string.IsNullOrEmpty(CompositeList) | ||
| ? CompositeList!.Split(Constants.ConfigurationValues.Separator) | ||
| : []) | ||
| .Distinct(); | ||
|
|
||
| var propagators = propagatorNames | ||
| .Select<string, Propagator?>(name => name switch | ||
| { | ||
| Constants.ConfigurationValues.Propagators.W3CTraceContext => Propagator.W3CTraceContext, | ||
| Constants.ConfigurationValues.Propagators.W3CBaggage => Propagator.W3CBaggage, | ||
| Constants.ConfigurationValues.Propagators.B3Multi => Propagator.B3Multi, | ||
| Constants.ConfigurationValues.Propagators.B3Single => Propagator.B3Single, | ||
| _ => null | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this legal? what is the result, NoOpPropagator etc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is legal. |
||
| }) | ||
| .OfType<Propagator>() | ||
| .ToList(); | ||
|
|
||
| return propagators; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,105 @@ | ||||||
| // Copyright The OpenTelemetry Authors | ||||||
| // SPDX-License-Identifier: Apache-2.0 | ||||||
|
|
||||||
| using OpenTelemetry.AutoInstrumentation.Configurations; | ||||||
| using OpenTelemetry.AutoInstrumentation.Configurations.FileBasedConfiguration; | ||||||
| using Xunit; | ||||||
|
|
||||||
| namespace OpenTelemetry.AutoInstrumentation.Tests.Configurations.FileBased; | ||||||
|
|
||||||
| public class FilebasedSdkSettingsTests | ||||||
| { | ||||||
| [Fact] | ||||||
| public void LoadFile_CompositeOnlyPropagators() | ||||||
| { | ||||||
| var propagator = new PropagatorConfiguration | ||||||
| { | ||||||
| Composite = new Dictionary<string, object> | ||||||
| { | ||||||
| { "tracecontext", new object() }, | ||||||
| { "baggage", new object() } | ||||||
| } | ||||||
| }; | ||||||
|
|
||||||
| var conf = new YamlConfiguration { Propagator = propagator }; | ||||||
| var settings = new SdkSettings(); | ||||||
|
|
||||||
| settings.LoadFile(conf); | ||||||
|
|
||||||
| Assert.Equal(2, settings.Propagators.Count); | ||||||
| Assert.Contains(Propagator.W3CTraceContext, settings.Propagators); | ||||||
| Assert.Contains(Propagator.W3CBaggage, settings.Propagators); | ||||||
| } | ||||||
|
|
||||||
| [Fact] | ||||||
| public void LoadFile_CompositeListOnlyPropagators() | ||||||
| { | ||||||
| var propagator = new PropagatorConfiguration | ||||||
| { | ||||||
| CompositeList = "b3multi,b3" | ||||||
| }; | ||||||
|
|
||||||
| var conf = new YamlConfiguration { Propagator = propagator }; | ||||||
| var settings = new SdkSettings(); | ||||||
|
|
||||||
| settings.LoadFile(conf); | ||||||
|
|
||||||
| Assert.Equal(2, settings.Propagators.Count); | ||||||
| Assert.Contains(Propagator.B3Multi, settings.Propagators); | ||||||
| Assert.Contains(Propagator.B3Single, settings.Propagators); | ||||||
| } | ||||||
|
|
||||||
| [Fact] | ||||||
| public void LoadFile_CompositeAndCompositeList_AreMergedWithoutDuplicates() | ||||||
| { | ||||||
| var propagator = new PropagatorConfiguration | ||||||
| { | ||||||
| Composite = new Dictionary<string, object> | ||||||
| { | ||||||
| { "tracecontext", new object() }, | ||||||
| { "baggage", new object() } | ||||||
| }, | ||||||
| CompositeList = "tracecontext,b3" | ||||||
| }; | ||||||
|
|
||||||
| var conf = new YamlConfiguration { Propagator = propagator }; | ||||||
| var settings = new SdkSettings(); | ||||||
|
|
||||||
| settings.LoadFile(conf); | ||||||
|
|
||||||
| Assert.Equal(3, settings.Propagators.Count); | ||||||
| Assert.Contains(Propagator.W3CTraceContext, settings.Propagators); | ||||||
| Assert.Contains(Propagator.W3CBaggage, settings.Propagators); | ||||||
| Assert.Contains(Propagator.B3Single, settings.Propagators); | ||||||
| } | ||||||
|
|
||||||
| [Fact] | ||||||
| public void LoadFile_GetEnabledPropagators_Empty() | ||||||
| { | ||||||
| var propagator = new PropagatorConfiguration(); | ||||||
|
|
||||||
| var conf = new YamlConfiguration { Propagator = propagator }; | ||||||
| var settings = new SdkSettings(); | ||||||
|
|
||||||
| settings.LoadFile(conf); | ||||||
|
|
||||||
| Assert.Empty(settings.Propagators); | ||||||
| } | ||||||
|
|
||||||
| [Fact] | ||||||
| public void LoadFile_GetEnabledPropagators_UnknownValues_AreIgnored() | ||||||
| { | ||||||
| var propagator = new PropagatorConfiguration | ||||||
| { | ||||||
| CompositeList = "invalid,b3multi" | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty test here?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactored test with load to settings 62af6ff |
||||||
| }; | ||||||
|
|
||||||
| var conf = new YamlConfiguration { Propagator = propagator }; | ||||||
| var settings = new SdkSettings(); | ||||||
|
|
||||||
| settings.LoadFile(conf); | ||||||
|
|
||||||
| Assert.Single(settings.Propagators); | ||||||
| Assert.Equal(Propagator.B3Multi, settings.Propagators[0]); | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| file_format: "1.0-rc.1" | ||
|
|
||
| propagator: | ||
| composite: | ||
| tracecontext: | ||
| baggage: | ||
| b3: | ||
| b3multi: | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| file_format: "1.0-rc.1" | ||
|
|
||
| propagator: | ||
| composite_list: ${OTEL_PROPAGATORS} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| using Xunit; | ||
| using YamlParser = OpenTelemetry.AutoInstrumentation.Configurations.FileBasedConfiguration.Parser.Parser; | ||
|
|
||
| namespace OpenTelemetry.AutoInstrumentation.Tests.Configurations.FileBased.Parser; | ||
|
|
||
| [Collection("Non-Parallel Collection")] | ||
| public class ParserPropagatorTests | ||
| { | ||
| [Fact] | ||
| public void Parse_FullConfigYaml_ShouldPopulateModelCorrectly() | ||
| { | ||
| var config = YamlParser.ParseYaml("Configurations/FileBased/Files/TestPropagatorFile.yaml"); | ||
|
|
||
| Assert.NotNull(config); | ||
| Assert.NotNull(config.Propagator); | ||
| Assert.NotNull(config.Propagator.Composite); | ||
| var composite = config.Propagator.Composite; | ||
| Assert.Null(config.Propagator.CompositeList); | ||
|
|
||
| string[] expectedPropagators = [ | ||
| "tracecontext", "baggage", "b3", "b3multi", | ||
| ]; | ||
|
|
||
| foreach (var expected in expectedPropagators) | ||
| { | ||
| Assert.Contains(expected, composite.Keys); | ||
| } | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Parse_EnvVarYaml_ShouldPopulateModelCompletely() | ||
| { | ||
| Environment.SetEnvironmentVariable("OTEL_PROPAGATORS", "tracecontext,baggage,b3,b3multi"); | ||
|
|
||
| var config = YamlParser.ParseYaml("Configurations/FileBased/Files/TestPropagatorFileEnvVars.yaml"); | ||
|
|
||
| Assert.NotNull(config); | ||
| Assert.NotNull(config.Propagator); | ||
| Assert.NotNull(config.Propagator.CompositeList); | ||
| Assert.Null(config.Propagator.Composite); | ||
| Assert.Equal("tracecontext,baggage,b3,b3multi", config.Propagator.CompositeList); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Distinct()already does the HashSet logic for you, no need to be explicit.return early >
ToLowerInvariant() adds extra string allocation. Maybe you can reuse
StringComparison.InvariantCultureIgnoreCasebased logic.Try to verify the list and not to copy it's contents over to another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in a19d41f
I made it case-sensitive, as it was done in
OTEL_PROPAGATORS.