itamarst / eliot

Eliot: the logging system that tells you *why* it happened
https://eliot.readthedocs.io
Apache License 2.0
1.1k stars 66 forks source link

log_call could be more useful if it interoperated with the type system #374

Open exarkun opened 5 years ago

exarkun commented 5 years ago

log_call is appealing for the effort/lines it saves. But using it requires giving up on the Field system.

If I have an ActionType FOO defined, it would be nice to be able to say:

@log_call(action_type=FOO)
def bar(...):
    ...

And have this be equivalent to:

def bar(...):
    with FOO(...):
        ...
exarkun commented 5 years ago

Or possibly an API like:

@log_call(action_type=u"foo", x=SOME_FIELD, ...)
def foo(x, ...):
    ...

Since it's fields, not actions, that actually define the validators/serializers.

itamarst commented 5 years ago

Two use cases I can see here:

  1. You want type for documentation/type checking. In this case, Python's type hints and related toolchain seems better, otherwise you're basically duplicating information.
  2. Testing. For this use case, note that I'm pretty sure assertHasAction can now take action_type strings, not just ActionType objects. Would that address your requirements?
exarkun commented 5 years ago

You want type for documentation/type checking. In this case, Python's type hints and related toolchain seems better, otherwise you're basically duplicating information.

On reflection, I think my angle is really more about the serializers. I may be able to use some other tool to ensure arguments have the right type but for anything other than JSON-compatible types, Eliot is still unable to serialize them.

2. Testing. For this use case, note that I'm pretty sure assertHasAction can now take action_type strings, not just ActionType objects.

Hm. I haven't been writing a lot of tests for the logging I've been adding, yet. So I'm not sure about this one. Certainly it would be bad if you couldn't use assertHasAction to make assertions about an action emitted by log_call instead of one of the other ways. So it sounds like a good thing that it can work on action_type strings now. But beyond that, I dunno.

itamarst commented 5 years ago

Ah, serializers, I forgot about those. It's possible to have a custom JSON encoder class, so you can add custom serialization logic that way (and at work I added support for a __eliot__ on objects), but serializers are much less global, so I can see the benefit.

jtrakk commented 5 years ago

I like the @functools.singledispatch trick that Hynek discusses. The key is passing

json.dumps(obj, default=to_serializable)

Here's an implementation:

import functools
import json
import io
import sys

from typing import Any
from typing import Optional
from typing import Union
from typing import Callable

import attr
import eliot
import pendulum

@functools.singledispatch
def to_serializable(obj: Any) -> Union[str, list, dict, int, float]:
    try:
        return attr.asdict(obj)
    except attr.exceptions.NotAnAttrsClassError:
        return str(obj)

@to_serializable.register(pendulum.DateTime)
def _(obj):
    return obj.to_iso8601_string()

def json_to_file(file: Optional[io.TextIOWrapper] = None) -> Callable:

    if file is None:
        file = sys.stdout

    def _json_to_file(x):
        print(json.dumps(x, default=to_serializable), file=file)

    return _json_to_file

eliot.add_destination(json_to_file)

Considering switching to multipledispatch as it can use the type annotations on the function arguments instead of requiring register(T), and can have multiple registries.

itamarst commented 5 years ago

In some ways that's nicer, yeah. The real question to me is how global vs. specific serialization can work. And maybe the answer is "specific can be done a bit more manually" except that doesn't work for log_call...