Refactor TracerProvider to make configuration easy.#1027
Refactor TracerProvider to make configuration easy.#1027cijothomas merged 7 commits intoopen-telemetry:masterfrom cijothomas:cijothomas/refactortracerprovider
Conversation
| @@ -102,18 +101,6 @@ internal static object Run(ConsoleOptions options) | |||
|
|
|||
| internal class MyProcessor : ActivityProcessor | |||
| { | |||
There was a problem hiding this comment.
Writing own Processor is also greatly simplified now. No need of worrying about "next".
| ActivityProcessor = this.ActivityProcessor, | ||
| }; | ||
|
|
||
| var activitySource = new ActivitySourceAdapter(provider.Sampler, provider.ActivityProcessor, provider.Resource); |
There was a problem hiding this comment.
activitysourceadapter won't fech ActiivtyProcessors added after building the provider. its a TODO on my list.
| "MyCompany.MyProduct.MyLibrary", | ||
| }) | ||
| .AddMyExporter(); | ||
| .Build().AddMyExporter(); |
There was a problem hiding this comment.
I know you can add exporter after build, but hey 😺
There was a problem hiding this comment.
Yes, for builder, it makes more sense to add everything before Build().
| { | ||
| using var tracerProvider = Sdk.CreateTracerProvider( | ||
| using var tracerProvider = Sdk.CreateTracerProviderBuilder() | ||
| .AddActivitySources( |
There was a problem hiding this comment.
Trying to get more ideas - which one is more idiomatic? - AddActivitySource overload (singular form) or AddActivitySources (plural form).
There was a problem hiding this comment.
Should people know that what we did so far is based on Activity? What if in the future it changes to something else? Should we remove the name Activity from it?
There was a problem hiding this comment.
I feel like singular but with an overload for plural?
void Add[Activity]Source(this TraceProviderBuilder builder, string sourceName);
void Add[Activity]Source(this TraceProviderBuilder builder, params string[] sourceNames);
Codecov Report
@@ Coverage Diff @@
## master #1027 +/- ##
==========================================
+ Coverage 75.53% 77.03% +1.50%
==========================================
Files 221 221
Lines 6208 6166 -42
==========================================
+ Hits 4689 4750 +61
+ Misses 1519 1416 -103
|
| new MySampler()) | ||
| .AddProcessor(new SimpleActivityProcessor(new ConsoleExporter(new ConsoleExporterOptions()))); | ||
| }) | ||
| .SetSampler(new MySampler()) |
There was a problem hiding this comment.
"SetSampler" not "UseSampler"?
.
| // and use a custom MyProcessor, along with Console exporter. | ||
| using var tracerProvider = Sdk.CreateTracerProviderBuilder().AddActivitySource("MyCompany.MyProduct.MyWebServer") | ||
| .SetResource(Resources.CreateServiceResource("MyServiceName")) | ||
| .AddProcessor(new MyProcessor()) // This must be added before ConsoleExporter |
There was a problem hiding this comment.
What happens if it isn't added before ConsoleExporter?
There was a problem hiding this comment.
Any enrichments done by MyProcessor will not be visible to ConsoleExporter. Spec says processors are called in order they are added.
6 participants
Major refactoring in TracerProvider creation, incorporating feedback from #1008, #1008 (comment), and offline feedbacks.
The examples are modified to show the new usage.
Note: This is fairly big change affecting 50 files so want to submit separate PRs for major feedbacks.
For significant contributions please make sure you have completed the following items: