Skip to content
This repository has been archived by the owner on Jul 11, 2022. It is now read-only.

Tracer.close() is not fully synchronous #50

Closed
yurishkuro opened this issue May 25, 2017 · 9 comments
Closed

Tracer.close() is not fully synchronous #50

yurishkuro opened this issue May 25, 2017 · 9 comments

Comments

@yurishkuro
Copy link
Member

It was meant to be fully synchronous to guarantee that all spans are flushed, however it only blocks until the spans are handed off to the sender, which itself is async, so if close() is used from a command line tool or a hello-world app that exist immediately, the spans may not have time to flush to UDP port.

@kgrvamsi
Copy link

kgrvamsi commented Jun 6, 2018

@yurishkuro do we need to append this time.sleep(2) when we write a production level application ?
Consider the scenario of distributed system where Agent/Collector/Query runs on Centralized host and we have code running on containers(roughly 30+) will the trace close needed based on the network lag for all these containers communicating to the centralized server?

@yurishkuro
Copy link
Member Author

If your services support graceful shutdown they yes, you might need to add this sleep after calling tracer.close(). But most long-running server applications are simply killed when they are upgraded or relocated, so in practice this rarely matters since close() won't be called anyway. You might lose some spans not flushed yet.

@spsotirop
Copy link

Hello,
I am hijacking this thread since I am facing an issue with the reporting of the spans. I would like to integrate the library in a StackStorm project (the underlying stack includes the usage of gunicorn and eventlet). My approach is the instantiation of a jaeger client (as a member of a custom singleton class) and the reporting of the spans is done inside a decorator used in the functions I would like to measure. The problem is that I must use time.sleep(2) in order for the spans to reach the collector. Is there a different approach to overcome this?

def tracing_decorator(func):
    @wraps(func)
    def wrapper_func(self, *args, **kwargs):
        name = func.__name__
        class_name = self.__class__.__name__
        with Tracer.get_instance().tracer_client.start_span(name) as span:
            span.log_kv({'function': name, 'class': class_name})
            s = func(self, *args, **kwargs)
        time.sleep(2)
        return s
    return wrapper_func

@yurishkuro
Copy link
Member Author

There is no different approach, I think, we just need to fix this specific issue.

@spsotirop
Copy link

Ok no problem, thanks for the prompt response

@obataku
Copy link
Contributor

obataku commented Nov 20, 2020

@yurishkuro is there a reason await-ing the Future from Tracer.close() (in the context of an event loop of course) would be insufficient to flush everything? waiting for the queue to drain (as Reporter._flush does with yield self.queue.join()) sounds like it would work but your comment

it only blocks until the spans are handed off to the sender

... as well as @nziebart's here

we could wait until the length of the queue is zero, but that does not necessarily mean that the background thread has finished sending all the data

... make me suspect I'm missing something

@yurishkuro
Copy link
Member Author

@obataku I don't know the underlying cause, it doesn't seem to work.

@AudriusButkevicius
Copy link

Is this still relevant given the merged PR?

@kasium
Copy link
Contributor

kasium commented Sep 15, 2021

@obataku so if I understand your PR right, simply doing yield trace.stop() should help?

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

No branches or pull requests

6 participants