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

Add flush interval for Jaeger exporter #340

Closed
bg451 opened this issue Sep 25, 2019 · 1 comment · Fixed by #341 · May be fixed by bg451/opentelemetry-node#11
Closed

Add flush interval for Jaeger exporter #340

bg451 opened this issue Sep 25, 2019 · 1 comment · Fixed by #341 · May be fixed by bg451/opentelemetry-node#11
Assignees

Comments

@bg451
Copy link
Member

bg451 commented Sep 25, 2019

During the SIG meeting today, @mayurkale22 mentioned a possible bug with the Jaeger exporter where Jaeger client only flushes when its span buffer reaches a size threshold, meaning that if you have a very low throughput system it might take a very long time for spans to get sent to Jaeger.

Looking at code, the exporter uses Jaeger's UDPSender, which is a fairly simple, low level piece of code that gets wrapped with more complex logic. In the jaeger client code base, there's another class that accepts a sender called a RemoteReporter which actually implements the interval flushing logic.

We can either add a flush interval to the Jaeger exporter or use RemoteReporter. I'm leaning towards adding a flush interval since it's a very small amount of code. Thoughts?

@bg451 bg451 self-assigned this Sep 25, 2019
bg451 pushed a commit to bg451/opentelemetry-node that referenced this issue Sep 25, 2019
bg451 pushed a commit to bg451/opentelemetry-node that referenced this issue Sep 25, 2019
bg451 pushed a commit to bg451/opentelemetry-node that referenced this issue Sep 25, 2019
bg451 pushed a commit to bg451/opentelemetry-node that referenced this issue Sep 25, 2019
@mayurkale22
Copy link
Member

+1 for flush interval option.

Another option could be, when we done with append logic just trigger the flush call https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-exporter-jaeger/src/jaeger.ts#L84-L92.

This should work nicely with both SpanProcessors (BatchSpanProcessor and SimpleSpanProcessor). In case of BatchSpanProcessor, append+flush will fire only when reached the max buffer size or timeout limit and in case of SimpleSpanProcessor, I expect to see immediately export (which won't work if we go with flush interval or may be we need to set small interval number).

Also, with this we can avoid multiple interval logic. This is the one in BatchSpanProcessor -> https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-basic-tracer/src/export/BatchSpanProcessor.ts#L47.

bg451 added a commit that referenced this issue Sep 26, 2019
* feat(jaeger-exporter): adds flushing on an interval

closes #340

* respond to comments

* yarn fix
SuperButterfly pushed a commit to SuperButterfly/opentelemetry-js that referenced this issue Sep 18, 2022
* feat(jaeger-exporter): adds flushing on an interval

closes open-telemetry/opentelemetry-js#340

* respond to comments

* yarn fix
viridius-1 pushed a commit to viridius-1/opentelemetry-js that referenced this issue Jan 14, 2023
* feat(jaeger-exporter): adds flushing on an interval

closes open-telemetry/opentelemetry-js#340

* respond to comments

* yarn fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants