zipkin-reporter-java icon indicating copy to clipboard operation
zipkin-reporter-java copied to clipboard

Zipkin AsyncReporter is not blocking on close

Open marcingrzejszczak opened this issue 4 years ago • 2 comments

Feature

Allow the AsyncReporter.close() to be properly used in shutdown hooks.

Rationale

Currently when a e.g. batch job finishes the AsyncReporter, even if registered as a shutdown hook, does not drain the span queue in a blocking way. The current solution is to do sth more or less as presented here [1] . So first, call flush, then wait for the timeout period + some time required to send out spans and only then call close. It would be great to have close() do all of that in a blocking way.

Example Scenario

Each application using an AsyncReporter.

Prior Art

  • [1] https://github.com/spring-cloud/spring-cloud-sleuth/pull/2036/files#diff-36ecfac59e4fa2cf4943436b9402d46508965320fb013f97da12a5d7817392c1R138-R152

marcingrzejszczak avatar Oct 08 '21 08:10 marcingrzejszczak

Looking at the code, it looks like it does wait up to the closeTimeout for spans to be flushed when closing:

https://github.com/openzipkin/zipkin-reporter-java/blob/ab669b823ab2ccbb753c4005247090c0645c6d5b/core/src/main/java/zipkin2/reporter/AsyncReporter.java#L340-L343

Is that not the behavior you're seeing?

shakuzen avatar Oct 11 '21 02:10 shakuzen

Yup, that's the behavior that I don't see. I need to wait more than that time for the spans to be sent.

marcingrzejszczak avatar Oct 11 '21 10:10 marcingrzejszczak

thanks for finding the setting @shakuzen. So, this is already configurable (and also tested as such in spring beans). someone who wants to set this to a higher value than 1 second can.

aside: my guess is there's a slow server or 100pct sampling is going on, as a second is a fair amount of time and why it is the default.

codefromthecrypt avatar Dec 11 '23 01:12 codefromthecrypt