bitpanda-labs / loggo2

Open source Python logging utilities
MIT License
4 stars 12 forks source link

New feature discussion: Adding inspectable parent info to manual calls to `Loggo.log` #66

Open interrogator opened 5 years ago

interrogator commented 5 years ago

When we do a manual log (Loggo.log('msg', level, extra)), the extra data is just whatever we pass in, plus the (probably pointless) 'loggo': True item.

Question: @hukkinj1 , @gcarq , would it be nice or not nice to inspecta little bit during these calls, and add a few things to extra data? Would be trivial to get script name, class name, func name, etc., though doing it in a nice consistent way might be a bit harder. Code could likely be shared from how we reconstruct build the call signature string during autologging, though of course it would be hackier, as we aren't passing the callable around, but need to get it through inspect stuff instead.

Helpful or not?

hukkin commented 5 years ago

I wouldn't mind if somebody does this, and the implementation is nice and bug-free. Could be useful.

Also another discussion for which I'm too lazy to start another thread/issue. Came to my mind again now looking at the signature Loggo.log('msg', level, extra). The current behavior for parameter level is that it is type string and extra['alert'] will be assigned to the value of level. I would like to make level strictly of type integer and have the same meaning as in the standard logging library. So setting it could look something like Loggo.log('msg', logging.DEBUG) or Loggo.log('msg', 10) (these two are equivalent).

If someone wants to assign a value to extra['alert'] they could do Loggo.log('msg', logging.DEBUG, {'alert': 'critical'}).

Overall this would IMO make the behavior more transparent, and the logic would not be based on a specific Graylog configuration.

Any thoughts?

interrogator commented 5 years ago

I'm going to turn the alert level stuff into a separate issue, it is completely unrelated. Regarding original issue, I'd like to know what extra info we would want to add before beginning, otherwise I can see myself just randomly saying, 'well, I guess we could also add the Python version. Oh, and the OS', etc etc.

hukkin commented 5 years ago

I don't feel like I personally ever had a need for any of this data, since if the log message doesn't completely suck, then you can always find the module, class, callable name etc. by searching the codebase for the message. So I don't consider this especially important and dont think we should populate the extra data with all sorts of random data for no reason.

But maybe the ones you already mentioned in the first post (class name, callable name, maybe module) could we justifiable as these we also get when using the Loggo decorators.

hukkin commented 5 years ago

When it comes to my unrelated discussion opener, if you agree with the content, then maybe make an issue for implementation already. If you want to discuss further, make an issue for discussion. Sounds good?

interrogator commented 5 years ago

I have added your idea under section the wrong way in the following issue: https://github.com/bitpanda-labs/loggo/issues/68

interrogator commented 5 years ago

Regarding the issue mentioned in the title, OK, I think if there is a way that I can get the same behaviour as the decorated logs (having class name etc in the log data) I will add these fields. If I can't get it the same, I'd rather do nothing than have it not match up. Agree?

hukkin commented 5 years ago

Completely agree.

interrogator commented 5 years ago

One other area of thought is for some log fields that aren't likely needed at bitpanda, but which might help (smaller) projects with different development environments. Just brainstorming about stuff others might like in an open-source logging util:

@gcarq you've made quite a few little bits and pieces in your time. Can you think of any values like this you might like in your logs?

Obviously bitpanda devs must approve new features, bitpanda shouldn't pay for their development, and some might be expensive, so many could be off by default. If we want to pursue this, we could easily control new fields, or batches of them, by config:

extras = dict(memory=True, http=True, pandas=TRue)
config = dict(facility='little-app',
              extra_fields=extras)
detailed = Loggo(config)
# ...
interrogator commented 5 years ago

BTW we so far have this, almost unused:

    def add_custom_log_data(self):
        """
        An overwritable method useful for adding custom log data
        """
        return dict()

which can be overwritten to grab some new stuff out of the loggo instance or I guess anything about the session in general.

I wonder how y'all might like passing in callables at config time:

import psutil
psutil.cpu_percent()
# 1.7
psutil.cpu_stats()
# scpustats(ctx_switches=20455687, interrupts=6598984, soft_interrupts=2134212, syscalls=0)
config = dict(facility='little-app',
              extra_fields=[psutil.cpu_percent, psutil.cpu_stats])

The field names could be automatically obtained with __qualname__ of the callable. Or, the user could pass in a dict. Kinda cool eh?

interrogator commented 5 years ago

Wouldn't mind writing some little helpers inside loggo, accessing in some fancy way like:

import loggo
helpers = [loggo.helpers.memory, loggo.helpers.cpu, loggo.helpers.pandas]
config = dict(facility='little-app',
              helpers=helpers)
Loggo = loggo.Loggo(config)

I'd like to do this just because it's so fucking beautiful.