-
Notifications
You must be signed in to change notification settings - Fork 155
In Jaeger Config, enable to add directly a sampler #324
In Jaeger Config, enable to add directly a sampler #324
Conversation
It can be useful when using a specific sampler that is not included in this library Signed-off-by: Etienne CARRIERE <[email protected]>
Signed-off-by: Etienne CARRIERE <[email protected]>
Signed-off-by: Etienne CARRIERE <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #324 +/- ##
=======================================
Coverage 95.43% 95.43%
=======================================
Files 25 25
Lines 1971 1973 +2
Branches 273 274 +1
=======================================
+ Hits 1881 1883 +2
Misses 56 56
Partials 34 34
Continue to review full report at Codecov.
|
jaeger_client/config.py
Outdated
sampler_config = self.config.get('sampler', {}) | ||
sampler_type = sampler_config.get('type', None) | ||
sampler_param = sampler_config.get('param', None) | ||
sampler_param = self.config.get('sampler', {}) |
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.
this is overloading sampler_param
variable. I would keep it as sampler_config
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.
I choose to change from sampler_config
to sampler_param
as now it can be both a sampler or a config so I choose a more generic name but if you want I can come back to sampler_config
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.
I'd be fine with sampler_config
or sampler
(prefer config as it's more common use case). But param
is confusing since it's the inner field.
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.
you are right . I come back to sampler_config .
Signed-off-by: Etienne Carriere <[email protected]>
It can be useful when using a specific sampler that is not included in
this library (for example private sampler)
Which problem is this PR solving?
#323
Short description of the changes
Accept on Jaeger Config parameter sampler not only a dict but also a Sampler object