create text map propagators from registry #885
Conversation
removing undocumented dependency between sdk and extension by adding text map propagators to a registry as part of composer autoloading, similar to how exporters, transports etc are treated.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #885 +/- ##
============================================
- Coverage 81.56% 81.39% -0.18%
- Complexity 2008 2011 +3
============================================
Files 264 264
Lines 5219 5229 +10
============================================
- Hits 4257 4256 -1
- Misses 962 973 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
|
||
| \OpenTelemetry\SDK\FactoryRegistry::registerTextMapPropagatorFactory( | ||
| \OpenTelemetry\SDK\Common\Configuration\KnownValues::VALUE_B3, | ||
| \OpenTelemetry\Extension\Propagator\B3\B3SinglePropagatorFactory::class |
There was a problem hiding this comment.
I would prefer if we add a ClosureTextMapPropagatorFactory (or allow registering closure factories directly) to avoid adding a factory class for every implementation.
There was a problem hiding this comment.
I've just removed the factories altogether, and registered the propagator instances instead. It's simpler, but also adding non-factories to a FactoryRegistry which is a little shady.
There was a problem hiding this comment.
Agree, that it's simpler, but as you written it breaks rule of storing only factories there. I don't have strong opinions for or against. Maybe, we should consider some renaming as FactoryRegistry will no longer store factories only?
There was a problem hiding this comment.
I've renamed it to Registry, which isn't very imaginative but seems valid.
removing undocumented dependency between sdk and extension by adding text map propagators to a registry as part of composer autoloading, similar to how exporters, transports etc are treated.