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

feat: Add InfluxLoggingHandler to use the InfluxClient in python native logging #405

Merged
merged 10 commits into from
Feb 28, 2022

Conversation

strom-und-spiele
Copy link
Contributor

@strom-und-spiele strom-und-spiele commented Feb 15, 2022

Closes #364

Sample

import logging
from influxdb_client import InfluxLoggingHandler
handler = InfluxLoggingHandler(url="…", token="…", org="…", bucket="…")
handler.setLevel(logging.DEBUG)
influx_logger = logging.getLogger('…')
influx_logger.setLevel(logging.DEBUG)
influx_logger.addHandler(handler)
influx_logger.debug("mem,host=host1 used_percent=23.43234543")

Checklist / TODO

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • pytest tests completes successfully
  • Commit messages are in semantic format
  • Sign CLA (if not already signed)
  • sample usage in /examples/

Possible improvement

  • should the connection be checked on init?
  • should the logger behave on errors (e.g. buffer till a connection can be reestablished)?

@bednar
Copy link
Contributor

bednar commented Feb 16, 2022

@strom-und-spiele Thanks for the PR 👍 and I'm waiting to see what comes next.

Please convert your PR to draft state if you are not ready to review.

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2022

Codecov Report

Merging #405 (1e10cee) into master (62a0ba1) will increase coverage by 0.05%.
The diff coverage is 97.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #405      +/-   ##
==========================================
+ Coverage   93.26%   93.31%   +0.05%     
==========================================
  Files          28       29       +1     
  Lines        2509     2544      +35     
==========================================
+ Hits         2340     2374      +34     
- Misses        169      170       +1     
Impacted Files Coverage Δ
influxdb_client/client/write/point.py 96.73% <80.00%> (-0.59%) ⬇️
influxdb_client/__init__.py 100.00% <100.00%> (ø)
influxdb_client/client/loggingHandler.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62a0ba1...1e10cee. Read the comment docs.

@strom-und-spiele strom-und-spiele force-pushed the loggingHandler branch 2 times, most recently from db36282 to b284e6a Compare February 16, 2022 08:54
@strom-und-spiele strom-und-spiele changed the title [WIP] Logging handler feat: Logging handler Feb 16, 2022
@strom-und-spiele
Copy link
Contributor Author

class printablePoint(Point):
    def __str__(self):
        return f"self.get_line_protocol()"

Using a printablePoint makes logging with most other handlers (like logging to files and stdout) useful (the actual data is logged instead of the __repr__ of Point).
Is there anything that speaks agains adding an implementation of __str__ to the Point class and making this work out of the box?

@bednar
Copy link
Contributor

bednar commented Feb 17, 2022

Is there anything that speaks agains adding an implementation of str to the Point class and making this work out of the box?

It sounds reasonable, so feel free to add __str__ to the Point.

I'm a little worried about user confusion about this PR. Can you please mention in the PR title and documentation that it is about using client as a LoggingHandler and not implement native logging into client (it is already there).

@strom-und-spiele strom-und-spiele changed the title feat: Logging handler feat: Add InfluxLoggingHandler to use the InfluxClient in python native logging Feb 17, 2022
Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

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

Thanks again for your PR 👍

There are a few requirements that must be be satisfy before we accept the PR:

@strom-und-spiele strom-und-spiele force-pushed the loggingHandler branch 2 times, most recently from a5b388a to 9800c8a Compare February 25, 2022 19:03
Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, everything looks good 👍

LGTM

@bednar bednar added the enhancement New feature or request label Feb 28, 2022
@bednar bednar merged commit e66fa77 into influxdata:master Feb 28, 2022
@bednar bednar added this to the 1.27.0 milestone Feb 28, 2022
bednar added a commit that referenced this pull request Feb 28, 2022
@strom-und-spiele strom-und-spiele deleted the loggingHandler branch February 28, 2022 18:13
@strom-und-spiele
Copy link
Contributor Author

Thanks for your PR, everything looks good +1

LGTM

Thanks reviewing. 🥇

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

Successfully merging this pull request may close these issues.

Use influxdb-client as a python native logging handler that writes to InfluxDB
3 participants