Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

custom parameters to BackgroundThreadTransport #249

Open
felippe-mendonca opened this issue Aug 3, 2018 · 2 comments
Open

custom parameters to BackgroundThreadTransport #249

felippe-mendonca opened this issue Aug 3, 2018 · 2 comments
Labels

Comments

@felippe-mendonca
Copy link
Contributor

When I started to use ZipkingExporter with the BackgroundThreadTransport, I'd noticed some delay to export (as well as the increase of queue of spans, driving an increase of used memory from service) in services that generate spans with a high rate. Then I realised that the BackgroundThreadTransport constructor has two parameters, but it's not possible to set these parameters since the ZipkinExporter constructor receives a type of transport instead of an instance. For me, it seems that isn't possible to change the constructor parameters of this transport type, but in some cases it could be interesting.

For now, to cover my issue I change the constructor of ZipkingExporter (see below) as well as implementation of BackgroundThreadExporter (now it has a method to set the exporter instead of receive this on constructor).

exporter = ZipkinExporter(
  service_name='MyService',
  host_name='localhost',
  port=9411,
  transport=BackgroundThreadTransport(max_batch_size=100)
)

I didn't open a pull request with this patch because I was in doubt if it's possible to change the transport parameters without the patch. Moreover, I just made the changes for ZipkinExporter. If this change makes sense, I can make the necessary modifications to cover all the exporter and then open a pull request.

Another thing is: for the BackgroundThreadTransport, beyond the max_batch_size, the _WAIT_PERIOD parameter may be interesting the possibility to change it.

@ocervell
Copy link
Contributor

ocervell commented Nov 1, 2018

+1 for this, biting me as well. I think the grace_period parameter and max_batch_size really depend on the use case, and we should be able to specify those.

@reyang
Copy link
Contributor

reyang commented May 3, 2019

#642 should address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants