Tired: fixing the issue cleanly with a nice detailed comment
Wired: leaving a cryptic block of ASCII art at the top of the file in order to push all the logging statements down into the 3-digit line number range

3-digit

Why stop there?

Seems slightly better to fall back to str(func) explicitly so we're always passing a string as the name? Unless you wanted to avoid the overhead?

Yeah, I was trying to avoid the overhead—changed to None-sentinel
check, which should be fast enough.

FWIW, this was a head-scratcher until I found this part of the contextlib docs:

contextmanager() uses ContextDecorator so the context managers it creates can be used as decorators as well as in with statements. When used as a decorator, a new generator instance is implicitly created on each function call (this allows the otherwise “one-shot” context managers created by contextmanager() to meet the requirement that context managers support multiple invocations in order to be used as decorators).

This was non-obvious to me (especially the nuance about how it manages to recreate the context manager via internal machinery) and would be helpful to at least have a comment - or maybe just write out the decorator by hand to make it obvious that the context manager is being regenerated on each function call. The fact that these context managers are also decorators feels mostly like a convenience when actually using them as decorators directly; we don't need that convenience (since we're using it in the context of our own decorator) so we could just be explicit about wrapping the decorated function in a with block.

Also, the fact that this is defined in an object that is also a context manager, although I see why it's there (to guard against incorrect usage of with log_latency(), feels extra confusing. Reintroducing the @log_latency form of the decorator might make sense just so that we can make the first parameter to log_latency() mandatory and avoid this class of error entirely, even if it makes the contract slightly more subtle?

Or alternatively, we could perhaps at least combine this context manager with the overall log_latency() and always go through this context-manager-decorator rather than taking the shortcut of returning _log_latency(...) as a context-manager-decorator sometimes and other times returning this object. E.g. the following:

class log_latency(object):
    def __init__(self, region_name=None, log_level=None, reusable=True):
        self._context_manager = _log_latency(region_name, log_level)

    def __enter__(self):
        if self._region_name is None:
            raise TypeError(...)
        self._context_manager.__enter__()
        return self

    def __exit__(self, *exc):
        return self._context_manager.__exit__(*exc)

    def __call__(self, func):
        qualname = getattr(func, "__qualname__", func)
        return _log_latency(qualname, self._log_level)(func)

Reintroducing the @log_latency form of the decorator might make
sense just so that we can make the first parameter to log_latency()
mandatory and avoid this class of error entirely, even if it makes the
contract slightly more subtle?

Okay, done. The analogous error that we no longer have a nice message
for is with log_latency:, which seems less likely (more obviously
wrong at call site) and would even more hackery to fix (something like
log_latency = _LogLatency() or a metaclass). This is fine enough.

Oh, meant to note—

The fact that these context managers are also decorators feels mostly
like a convenience when actually using them as decorators directly; we
don't need that convenience (since we're using it in the context of
our own decorator) so we could just be explicit about wrapping the
decorated function in a with block.

—in the revised version I’m still using this convenience, because it’s
simpler than doing the functools.wraps/*args, **kwargs dance
ourselves, but I’ve spelled it out a bit more to make it clearer. Agreed
that the contextlib magic is a bit magic.

Thanks! This version is vastly easier for me to wrap my head around :)

(I guess with log_latency: will just raise an AttributeError: __enter__ - but if Python 3.7 feels that's a good enough way to indicate "you tried to use something as a context manager and it wasn't one" then I think this is good enough for us.)

3-digit

Why stop there?

Yeah, I was trying to avoid the overhead—changed to None-sentinel
check, which should be fast enough.

Reintroducing the @log_latency form of the decorator might make
sense just so that we can make the first parameter to log_latency()
mandatory and avoid this class of error entirely, even if it makes the
contract slightly more subtle?

Okay, done. The analogous error that we no longer have a nice message
for is with log_latency:, which seems less likely (more obviously
wrong at call site) and would even more hackery to fix (something like
log_latency = _LogLatency() or a metaclass). This is fine enough.

It’s not super easy to get a handle to a function without qualname to
test this case, but with at least current versions of numpy the
following suffices as a manual test:

>>> import logging
>>> import numpy as np
>>> from tensorboard.util import timing
>>>
>>> np.__version__
'1.18.2'
>>> np.sin.__qualname__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'numpy.ufunc' object has no attribute '__qualname__'
>>> sin = timing.log_latency(np.sin, log_level=logging.WARNING)
>>> sin([1, 2, 3])
MainThread[7f81fc8d7740] ENTER <ufunc 'sin'>
MainThread[7f81fc8d7740] LEAVE <ufunc 'sin'> - 0.000302s elapsed
array([0.84147098, 0.90929743, 0.14112001])

(Don’t want to commit this in case the upstream bug is ever fixed.)

Thanks! This version is vastly easier for me to wrap my head around :)

(I guess with log_latency: will just raise an AttributeError: __enter__ - but if Python 3.7 feels that's a good enough way to indicate "you tried to use something as a context manager and it wasn't one" then I think this is good enough for us.)