Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow formatter customizations #18

Merged
merged 8 commits into from
Jun 3, 2015

Conversation

EvaSDK
Copy link
Contributor

@EvaSDK EvaSDK commented Aug 1, 2014

I am currently pursuing the goal of using fluent just like any other logging Handler/Formatter via logging.config

In order to do that I had to solve a couple of problems that make default FluentRecordFormatter not easy to deal with. It is now able to take a format argument which is a dictionary that holds keys and values. Values are regular LogRecord standard Python %-style mapping keys. The defaults are unchanged.

I also added a couple of fixes I found in other pull requests as I found them as well during my tests, so this should cover Issue #7, PR #8 and PR #16.

Finally I added a sphinx ready docstring to FluentRecordFormatter._structuring although this function could probably be merged in format directly since I don't see it used anywhere else.

Once merged, the following sample program:

import logging
from fluent.handler import FluentHandler, FluentRecordFormatter

logger = logging.getLogger('toto')
logger.setLevel(logging.DEBUG)
handler = FluentHandler('hello.test')
handler.setLevel(logging.DEBUG)
formatter = FluentRecordFormatter(fmt={
    'msg': '%(message)s',
    'level': '%(levelname)s',
    'hostname': '%(hostname)s',
})
handler.setFormatter(formatter)
logger.addHandler(handler)

logger.info('hel""lo')

logs

2014-08-01 13:59:05 +0200 hello.test: {"msg":"hel\"\"lo","message":"hel\"\"lo","hostname":"seta.local","level":"INFO"}

to fluentd.

@kdeldycke
Copy link

👍

@EvaSDK
Copy link
Contributor Author

EvaSDK commented Aug 1, 2014

A sample configuration would be:

logging:
    version: 1

    formatters:
      brief:
        format: '%(message)s'
      default:
        format: '%(asctime)s %(levelname)-8s %(name)-15s %(message)s'
        datefmt: '%Y-%m-%d %H:%M:%S'
      fluent_fmt:
        '()': fluent.handler.FluentRecordFormatter
        format:
          message: '%(message)s'
          level: '%(levelname)s'
          hostname: '%(hostname)s'
          where: '%(module)s.%(funcName)s'

    handlers:
        console:
            class : logging.StreamHandler
            level: DEBUG
            formatter: default
            stream: ext://sys.stdout
        fluent:
            class: fluent.handler.FluentHandler
            host: localhost
            port: 24224
            tag: test.logging
            formatter: fluent_fmt
            level: DEBUG
        null:
            class: logging.NullHandler

    loggers:
        amqp:
            handlers: [null]
            propagate: False
        conf:
            handlers: [null]
            propagate: False
        '': # root logger
            handlers: [console, fluent]
            level: DEBUG
            propagate: False

loaded likeso:

import logging.config
import yaml

with open('logging.yaml') as fd:
    conf = yaml.load(fd)

logging.config.dictConfig(conf['logging'])

the way to customize formatter via logging.config.dictConfig drove me a bit crazy until I found the proper documentation. So I figured it would be useful to add it as an example here.

@EvaSDK EvaSDK mentioned this pull request Aug 5, 2014
@EvaSDK EvaSDK force-pushed the allow-formatter-customizations branch from 5082d68 to fbd05d8 Compare February 3, 2015 14:49
@EvaSDK
Copy link
Contributor Author

EvaSDK commented Feb 3, 2015

Branch updated against 0.3.5.

@kiyoto
Copy link
Contributor

kiyoto commented Feb 3, 2015

@EvaSDK

The idea looks good to me. Do you think you can add a test for the string input case? After that it's ready to be merged.

@EvaSDK
Copy link
Contributor Author

EvaSDK commented Feb 4, 2015

I can look into it yes.

@EvaSDK
Copy link
Contributor Author

EvaSDK commented Feb 25, 2015

Added some tests and fixed a few issues while doing so.

The most annoying thing to me is the drop of python 2.6 support due to class changes in python logging module and how it handles the asctime attribute. I think dropping 2.6 support is not too bad anyway due to it being unsupported on most distributions nowadays. If you still want to support it, I can elaborate an alternative solution.

@EvaSDK EvaSDK force-pushed the allow-formatter-customizations branch from 2768952 to 70d68cb Compare May 27, 2015 13:36
@EvaSDK
Copy link
Contributor Author

EvaSDK commented May 27, 2015

Added py26 compatibility back. It does not look pretty but if you want it for older distributions, you have it.
I included README update from PR #22.

This fixes: issue #7, PR #8, PR #22 / issue #23

With PR #31, this adds up to 3 pull requests to fix conformance to logging classes. Please tell me what to do with this PR, it has been almost a year and I did everything I could to satisfy your modification requests.

@repeatedly
Copy link
Member

@yyuu Could you check this PR?
I confirmed this change resolves #34 issue.
We should fix issue 34 to avoid logging trouble.

@yyuu
Copy link
Member

yyuu commented May 29, 2015

Looks good to me.

@repeatedly
Copy link
Member

@yyuu Feel free to merge this PR and release new version. Maybe update minor version is better?
After that, we will close the issue on fluentd repository.

@EvaSDK
Copy link
Contributor Author

EvaSDK commented May 30, 2015

The PR contains a version bump to 0.4.0 as I felt this was an important enough change.

@repeatedly
Copy link
Member

How about this PR? > #31

@yyuu
Copy link
Member

yyuu commented Jun 1, 2015

@repeatedly I don't have permission to upload fluent-logger to PyPI....

@EvaSDK
Copy link
Contributor Author

EvaSDK commented Jun 1, 2015

#31 squashed all my commits in one and adds a feature that is debatable on its own. Imho, it should be reworked as a single commit on top of my branch first.

@@ -1,6 +1,5 @@
language: python
python:
- "2.6"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EvaSDK Why did you remove 2.6 from .travis.yml? It's still in tox.ini and seems working fine at least for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I forgot to update .travis.yml when I re-added support for it. I'll fix that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it. I'll fix it by myself.

@yyuu yyuu merged commit 70d68cb into fluent:master Jun 3, 2015
yyuu added a commit that referenced this pull request Jun 3, 2015
@EvaSDK
Copy link
Contributor Author

EvaSDK commented Jun 3, 2015

w00t, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants