[FileBasedConfiguration] Sampler Configuration #4548
Conversation
Removed the line about file-based configuration for tracer sampler.
…configuration Add file-based sampler configuration
src/OpenTelemetry.AutoInstrumentation/Configurations/FileBasedConfiguration/SamplerFactory.cs
Show resolved
Hide resolved
docs/file-based-configuration.md
Outdated
| exporter: | ||
| console: | ||
|
|
||
| # Configure the sampler. If omitted, parent based sampler with a root of always_on is used. |
There was a problem hiding this comment.
Need to more examples how to configure other samplers, on only parent based.
There was a problem hiding this comment.
Added more examples to configuration in d1d1605
| [YamlMember(Alias = "root")] | ||
| public SamplerConfig? Root { get; set; } | ||
|
|
||
| [YamlMember(Alias = "remote_parent_sampled")] | ||
| public SamplerConfig? RemoteParentSampled { get; set; } | ||
|
|
||
| [YamlMember(Alias = "remote_parent_not_sampled")] | ||
| public SamplerConfig? RemoteParentNotSampled { get; set; } | ||
|
|
||
| [YamlMember(Alias = "local_parent_sampled")] | ||
| public SamplerConfig? LocalParentSampled { get; set; } | ||
|
|
||
| [YamlMember(Alias = "local_parent_not_sampled")] | ||
| public SamplerConfig? LocalParentNotSampled { get; set; } |
There was a problem hiding this comment.
Are you sure that we should support parent_based within ParentBased? With this structure it is possible. I think that you need separatte Config for all these subsections.
What is more, traceid ratio should be possible to use only on the root
There was a problem hiding this comment.
OTEL .NET SDK allows this, so I’d like to keep current solution and give users full freedom and responsibility for configuring the sampler
There was a problem hiding this comment.
Manual object structure building is something different than pre-configured options.
In such cases, IMO we should allow only valid options, especially in terms of the specification.
There was a problem hiding this comment.
Fixed in 0282066
| } | ||
|
|
||
| [Fact] | ||
| public void LoadFile_ConfiguresParentBasedSampler() |
There was a problem hiding this comment.
Added more test cases in 53fd6c9
There was a problem hiding this comment.
There should probably be a test for using the traceid ratio sampler under the root option, especially since that is one of the scenarios that we are documenting.
There was a problem hiding this comment.
Test Case Added in beab7af
3 participants
Why
Towards #4394
What
File Based Configuration for Sampler
Note
Original PR created by Ai Codex
ysolomchenko#1
Tests
Ci
Checklist
CHANGELOG.mdis updated.