Separate context propagation into separate layers (otep-66) #399
Conversation
| namespace OpenTelemetry.Api.Baggage | ||
| { | ||
| public abstract class BaggageContext | ||
| { } |
There was a problem hiding this comment.
- why this abstraction is needed? Is there an existing .NET primitive that could be used instead? IEnumerable or Dictionary?
- Does it have to be a class?
- Why it does not have GEt/Set/Remove/Clear and we need a manager for that? This does not seem like a .NET friendly API
| { | ||
| public abstract string GetEntry(BaggageContext context, string key); | ||
|
|
||
| public abstract BaggageContext SetEntry(BaggageContext context, string key, string value); |
There was a problem hiding this comment.
is it going to create a new BaggageContext or update one passed in the argument? If former, it seems inefficient. If latter, this should be changes to the method on BaggageContext
|
|
||
| namespace OpenTelemetry.Api.Baggage | ||
| { | ||
| public abstract class BaggageManagerBase |
| namespace OpenTelemetry.Api.Correlations | ||
| { | ||
| public class CorrelationContext | ||
| { } |
There was a problem hiding this comment.
same concerns as for the BaggageContext
|
|
||
| public abstract BaggageContext ClearEntries(BaggageContext context); | ||
|
|
||
| public abstract HttpInjectorBase GetHttpInjector(); |
There was a problem hiding this comment.
Is there a requirement to propagate extractors/injectors through the manager class? Could it be done as a constructor argument to adapters instead?
How extractor/injections are going to be set up by users? On the tracer/meter factory?
There was a problem hiding this comment.
Can you please provide some scenarios (in C# or pseudo-code) how this classes are going to be used and configured:
- who configures them
- where: Tracer/MeterFactory, somewhere else?
- who uses them: adapters, users.
- How they are going to be used
This would really help with API design/review
|
Thanks for the initial review @lmolkova. This is not complete yet and I just wanted to push something so show it's in progress - that's why I set the PR as draft. The work I've done so far in the above commits are trying to stitch together what's been done in Go & Java and will be refactored to better suit OTel .NET in the coming days. |
lmolkova
added
the
pkg:OpenTelemetry.Api
MikeGoldsmith
force-pushed
the
otel-66
branch
from
3151366 to
9c33a02
Compare
This is a draft PR to incorporate the changes in otep-66, specifically breaking context propagation into separate layers.