jaegertracing / jaeger-client-python

🛑 This library is DEPRECATED!
https://jaegertracing.io/
Apache License 2.0
408 stars 155 forks source link

Feat request: Adding flexibility to configuration #344

Closed KazakovDenis closed 2 years ago

KazakovDenis commented 2 years ago

It's very sad, that Jaeger will no longer be supported, but I hope the next suggestions will help those who will continue using it. Jaeger is awesome and helps me a lot.

Requirement - what kind of business use case are you trying to solve?

Adding extensibility to the Jaeger client. It will be easier for users to customize the Jaeger client for their specific situations.

Problem - what in Jaeger blocks you from solving the requirement?

Some Config class methods need dependency injections because now they can only be completely overridden. I had a need to customize the Reporter a little bit, but its class is hardcoded:

    def new_tracer(self, io_loop=None):
        channel = self._create_local_agent_channel(io_loop=io_loop)
        sampler = self.sampler
        if not sampler:
            # getting sampler here
        logger.info('Using sampler %s', sampler)

        reporter = Reporter(
            channel=channel,
            queue_capacity=self.reporter_queue_size,
            batch_size=self.reporter_batch_size,
            flush_interval=self.reporter_flush_interval,
            logger=logger,
            metrics_factory=self._metrics_factory,
            error_reporter=self.error_reporter)

Therefore, I had to inherit Config and override new_tracer() method. And the same comes with Throttler and Tracer. Also, sampler property does not return a sampler, if sampler_type is None. Why not return RemoteControlledSampler here instead of initializing it in new_tracer()?

Proposal - what do you suggest to solve the problem or improve the existing situation?

So I had to do something like this:

# and more imports
from jaeger_client import Config as DefaultConfig, Tracer

class Config(DefaultConfig):
    # and other defaults
    reporter_cls = ModifiedReporter

    def new_tracer(self, io_loop=None) -> Tracer:
        channel = self._create_local_agent_channel(io_loop=io_loop)
        return self.create_tracer(
            reporter=self._get_reporter(channel),
            sampler=self._get_sampler(channel),
            throttler=self._get_throttler(channel),
        )

    def _get_reporter(self, channel: LocalAgentSender, **kwargs) -> BaseReporter:
        reporter = self.reporter_cls(
            # pass args
        )
        if self.logging:
            reporter = CompositeReporter(reporter, LoggingReporter(logger))
        return reporter

Any open questions to address

There are a few options to implement, such as:

# get an instance from the property as it did for a sampler
@property
def reporter(self) -> BaseReporter:

# or via the public or protected method
def get_reporter(self, channel: Any) -> BaseReporter:

or the way to provide defaults:

# via Config class attributes
class Config:
    tracer_cls: opentracing.Tracer = Tracer
    reporter_cls: BaseReporter = Reporter

# or directly passing to constructor
def __init__(..., tracer=Tracer, reporter=Reporter)

# or some another way, e.g. passing to config dict
tracer = Config(
    config={
        ...
        'reporter': {'type': ...},
        'tracer': {'type': ...},
        'throttler': {'type': ...},
    },
).initialize_tracer()

I would be glad to receive any feedback on these ideas and ready to make improvements quickly. Thanks!