Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

Add logger name to log format #608

Closed
wants to merge 1 commit into from
Closed

Conversation

ncdc
Copy link
Contributor

@ncdc ncdc commented Oct 6, 2014

Add %(name)s to log format so log messages are prefixed with the
logger's name.

Signed-off-by: Andy Goldstein agoldste@redhat.com

Add %(name)s to log format so log messages are prefixed with the
logger's name.

Signed-off-by: Andy Goldstein <agoldste@redhat.com>
@ncdc ncdc mentioned this pull request Oct 6, 2014
@wking
Copy link
Contributor

wking commented Oct 6, 2014

On Mon, Oct 06, 2014 at 06:49:31AM -0700, Andy Goldstein wrote:

  • Add logger name to log format

Looks fine to me, although I would have probably gone with:

'%(asctime)s %(name)s %(levelname)s: %(message)s'

@ncdc
Copy link
Contributor Author

ncdc commented Oct 6, 2014

@wking I can do that. Do you want me to switch it now or wait for others to chime in?

@wking
Copy link
Contributor

wking commented Oct 6, 2014

On Mon, Oct 06, 2014 at 08:39:49AM -0700, Andy Goldstein wrote:

Do you want me to switch it now or wait for others to chime in?

It's not my call, so you should probably wait ;)

@dmp42
Copy link
Contributor

dmp42 commented Oct 6, 2014

I don't have hard feelings on this.

A couple comments though:

@bacongobbler
Copy link
Contributor

FYI django's recommended logger formats look like so:

'formatters': {
        'verbose': {
            'format': '%(levelname)s %(asctime)s %(module)s %(process)d %(thread)d %(message)s'
        },
        'simple': {
            'format': '%(levelname)s %(message)s'
        }
}

Take what you will from it :)

From https://docs.djangoproject.com/en/1.7/topics/logging/

@wking
Copy link
Contributor

wking commented Oct 6, 2014

On Mon, Oct 06, 2014 at 10:22:33AM -0700, Matthew Fisher wrote:

        'format': '%(levelname)s %(asctime)s %(module)s %(process)d %(thread)d %(message)s'

I'm happy to reuse an existing standard. My suggestion was just
because I'd rather sort first by name (i.e. module) and then by level.
I've also not seen brackets in log messages, except for marking out
process ids in syslog messages 1.

@dmp42
Copy link
Contributor

dmp42 commented Oct 6, 2014

@wking @bacongobbler @ncdc please go ahead:

  • review other frameworks/well known python libraries position on this (Django is a good start indeed)
  • champion a choice / reach an agreement
  • amend this PR with it

@noxiouz
Copy link
Contributor

noxiouz commented Oct 6, 2014

May be Flask or Tornado formats?

@dmp42
Copy link
Contributor

dmp42 commented Oct 29, 2014

@ncdc willing to own this and get it through?

@ncdc
Copy link
Contributor Author

ncdc commented Oct 30, 2014

@dmp42 sure, I'll update the PR.

@wking the reason I put the logger's name in [brackets] is that I'm used to JBoss log formatting, which typically does the class name that way.

I've found that if we specify --log-config when running gunicorn, it will control the log format for everything, and the format set in app.py is ignored. I am thinking this is a good thing, as it allows end users to control the log format instead of having it hardcoded. We can add an example/default logging.conf (see e.g. https://github.com/benoitc/gunicorn/blob/master/examples/logging.conf) and allow users to override it with a GUNICORN_LOG_CONFIG environment variable.

Thoughts?

@wking
Copy link
Contributor

wking commented Oct 30, 2014

On Thu, Oct 30, 2014 at 08:07:08AM -0700, Andy Goldstein wrote:

I've found that if we specify --log-config when running gunicorn,
it will control the log format for everything, and the format set in
app.py is ignored.

Ah, nice. In that case, pick whatever default you like ;).

@ncdc
Copy link
Contributor Author

ncdc commented Oct 30, 2014

Exactly :-) So we just need a reasonable default logging.conf.

@dmp42
Copy link
Contributor

dmp42 commented Dec 2, 2014

friendly bump :)

@ncdc
Copy link
Contributor Author

ncdc commented Dec 4, 2014

@dmp42 we're ok closing in favor of waiting for v2

@dmp42
Copy link
Contributor

dmp42 commented Dec 4, 2014

Ok, thanks @ncdc !

@dmp42 dmp42 closed this Dec 4, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants