Enhancement: Add default name to default seqlog logger created with log_to_seq()

[Vacant0mens]

• SeqLog version: 0.3.26
• Python version: 3.11+

Disclaimer

I'm not an expert with the seqlog module. I try to do my best research before posting issues on GitHub, but I'm only human. If any of the following is inaccurate, feel free to correct me.

Also, I am willing to do the coding work for this. I just wanted to (1) have a discussion about it first (if needed), (2) track my idea and its progress, and (3) make sure I wasn't misunderstanding the way that the seqlog module was put together.

Description

(This is in response to issues #40 and #41 regarding the Usage documentation page, as well as the seqlog.log_to_seq() module-level function)

For clarity and flexibility, a name argument should be added to log_to_seq() (with a default value). It can't currently be provided by adding it as a kwarg, because logging.basicConfig() doesn't accept a name argument. If it's not set somewhere it results in the logger's name property being set to '__main__', which isn't easily searchable. (However, It is possible to call logging.getLogger(name=__name__) to get the currently-configured Logger instance if the current logger wasn't given a name, but this is not very user-friendly.)

The stdlib logging documentation states:

[The logging.setLoggerClass(klass) function] is typically called before any loggers are instantiated by applications which need to use custom logger behavior. After this call, as at any other time, do not instantiate loggers directly using the subclass; continue using the logging.getLogger() API to get your loggers.

While this may be true for a lot of use cases for the seqlog module, the log_to_seq() function should instantiate (and return) the StructuredLogger class rather than creating a default instance of it and returning SeqLogHandler. (in my opinion, returning the logger would make more sense to the users.) That way it's not assumed that StructuredLogger is the only logger or logger class that any given user will be using.

Suggested Actions

1. Update the log_to_seq() function to accept a name argument and give it a default value (something like DefaultSeqLogger would be fine, or DefaultSeqRootLogger if override_root_logger=True).
2. Within the log_to_seq() function, instantiate the StructuredLogger class (with name as given or its default value), assign the class a new instance of SeqLogHandler with loggerClass.addHandler() (using the same arguments to instantiate it as are currently there).
3. Update the Usage documentation to reflect the above changes and/or expand the topic of instantiating the StructuredLogger class and its handler/s.

[tintoy]

That sounds like a good solution, thanks for taking the initiative!

I'm happy for you to make this change if you're comfortable doing so 🙂

[Vacant0mens]

Out of curiosity, was there a reason the original design returned the StructuredLogHandler class instead of the StructuredLogger class, using the logging.getLogger() call?

[tintoy]

Can't remember anymore I'm afraid 😂

I'm guessing it was a lack of familiarity with the API semantics for custom logging components, but it was a while ago now. I'm certainly not wedded to that implementation detail 🙂

[Vacant0mens]

I was looking over the documentation for the logging module. looks like basicConfig(), dictConfig(), and fileConfig() don't actually return anything when called. They just set up the logging config in the background and expect the user to call one of the logging functions to start using it.
i.e.:

>>> import logging
>>> logging.basicConfig(level=logging.INFO)  # <- no output from this call
>>> logging.info("this is a log")
INFO:root:this is a log
>>> 

I'd be okay refactoring the log_to_seq() and log_to_console() functions to set up the classes like I said (or maybe use dictConfig() for easier management in the future?) and just let them get configured in the background in the same way.

Also, this python-logstash module looks pretty solid. I bet we could use that as an example for this enhancement, as well as that other one for adding synchronous-vs-asynchronous logging in Issue #44

Thoughts on the two topics?

[tintoy]

Sure, that sounds like it could work - just as long as whatever solution we choose can still handle configuration of the root logger 🙂

[tintoy]

(happy for you to have a go at it; maybe create a PR if you're ok with that?)

[tintoy]

The main thing about log_to_seq is that it encapsulates "necessary" configuration for seq (formatter, former style, log handlers, etc). Originally when I wrote it, it was because Seq we originally from the .NET world and I wasn't sure how familiar consumers would be with Python logging (and associated configuration) so it was enough to get them going as a kind of "quick start".

[Vacant0mens]

Yeah, and as a basic quick-start thing, it works as it is for the majority of basic use cases, but using basicConfig() in the background like that isn't good for more advanced use cases (like if you have other Logger or Handler classes to set up aside from the ones imported from seqlog) because it says in the logging documentation that if you call basicConfig() after the root logger already has at least one handler, that function does nothing. ... which I just realized means that if you call log_to_seq() with override_root_logger=True, it doesn't currently set up any other loggers besides the StructuredRootLogger.

I'm just trying to think of the best way to keep the log_to_seq() and log_to_console() functions easy to use, but also make them more scalable.