Skip to content
Snippets Groups Projects
Forked from Maunium / synapse
Source project has a limited visibility.
  • Richard van der Hoff's avatar
    31b554c2
    Fixes for opentracing scopes (#11869) · 31b554c2
    Richard van der Hoff authored
    `start_active_span` was inconsistent as to whether it would activate the span
    immediately, or wait for `scope.__enter__` to happen (it depended on whether
    the current logcontext already had an associated scope). The inconsistency was
    rather confusing if you were hoping to set up a couple of separate spans before
    activating either.
    
    Looking at the other implementations of opentracing `ScopeManager`s, the
    intention is that it *should* be activated immediately, as the name
    implies. Indeed, the idea is that you don't have to use the scope as a
    contextmanager at all - you can just call `.close` on the result. Hence, our
    cleanup has to happen in `.close` rather than `.__exit__`.
    
    So, the main change here is to ensure that `start_active_span` does activate
    the span, and that `scope.close()` does close the scope.
    
    We also add some tests, which requires a `tracer` param so that we don't have
    to rely on the global variable in unit tests.
    Fixes for opentracing scopes (#11869)
    Richard van der Hoff authored
    `start_active_span` was inconsistent as to whether it would activate the span
    immediately, or wait for `scope.__enter__` to happen (it depended on whether
    the current logcontext already had an associated scope). The inconsistency was
    rather confusing if you were hoping to set up a couple of separate spans before
    activating either.
    
    Looking at the other implementations of opentracing `ScopeManager`s, the
    intention is that it *should* be activated immediately, as the name
    implies. Indeed, the idea is that you don't have to use the scope as a
    contextmanager at all - you can just call `.close` on the result. Hence, our
    cleanup has to happen in `.close` rather than `.__exit__`.
    
    So, the main change here is to ensure that `start_active_span` does activate
    the span, and that `scope.close()` does close the scope.
    
    We also add some tests, which requires a `tracer` param so that we don't have
    to rely on the global variable in unit tests.