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

Read logging config from logging.yml if available #2659

Conversation

lunkwill42
Copy link
Member

This adds the ability to override most of NAV's logging config with the more flexible dictConfig format, as read from an optional logging.yml file (which can also be overriden by environment variable, just like logging.conf)

This could be used to customize NAV logging in your deployment to a much larger degree. It should also be possible to have NAV keep logging to its normal log files by configuring a default StreamHandler, as most NAV daemons will redirect stderr to its log file on daemonization.

@lunkwill42 lunkwill42 requested a review from hmpf August 10, 2023 14:13
@lunkwill42
Copy link
Member Author

I was able to log both to Humio and to local files by adding this to my /etc/nav/logging.yml:

version: 1
loggers:
  nav:
    level: DEBUG
  root:
    handlers: [humio, console]

handlers:
  humio:
    class: humiologging.handlers.HumioJSONHandler
    level: DEBUG
    humio_host: https://your-humio-ingest-addr-here
    ingest_token: SECRET_TOKEN_THINGY
  console:
    class: logging.StreamHandler
    formatter: default


formatters:
  default:
    format: '%(asctime)s [%(levelname)s] [%(name)s] %(message)s'

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #2659 (9bbc066) into master (2075d6b) will increase coverage by 0.63%.
Report is 125 commits behind head on master.
The diff coverage is 95.00%.

❗ Current head 9bbc066 differs from pull request most recent head 43cabfa. Consider uploading reports for the commit 43cabfa to get more accurate results

@@            Coverage Diff             @@
##           master    #2659      +/-   ##
==========================================
+ Coverage   54.61%   55.25%   +0.63%     
==========================================
  Files         560      561       +1     
  Lines       40712    41546     +834     
==========================================
+ Hits        22235    22955     +720     
- Misses      18477    18591     +114     
Files Changed Coverage Δ
python/nav/logs.py 81.67% <95.00%> (+2.40%) ⬆️

... and 17 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Aug 10, 2023

Test results

     12 files       12 suites   14m 44s ⏱️
3 204 tests 3 204 ✔️ 0 💤 0
9 087 runs  9 087 ✔️ 0 💤 0

Results for commit 9bbc066.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Looks fine but codecov complains about missing coverage, and reports wrong lines even.

Also, docs needed.

@lunkwill42 lunkwill42 force-pushed the feature/support-standard-logging-fileconfig branch from 9bea213 to aa3d873 Compare August 11, 2023 12:23
@lunkwill42
Copy link
Member Author

Looks fine but codecov complains about missing coverage, and reports wrong lines even.

Also, docs needed.

Added tests and docs, but now this depends on #2660 being merged first.

I also see we have still more implicitly covered error handling code in ipdevpoll that keeps falling in and out of coverage due to timing randomness. I've added that to my worklist.

@lunkwill42 lunkwill42 requested a review from hmpf August 11, 2023 12:25
This adds the ability to override most of NAV's logging config with
the more flexible dictConfig format, as read from an optional
`logging.yml` file (which can also be overriden by environment variable,
just like `logging.conf`)
@lunkwill42 lunkwill42 marked this pull request as ready for review August 11, 2023 12:33
@lunkwill42 lunkwill42 self-assigned this Aug 11, 2023
@lunkwill42 lunkwill42 force-pushed the feature/support-standard-logging-fileconfig branch from aa3d873 to 6a9828c Compare August 11, 2023 12:34
NAV has by default installed on it, so if you want NAV to also keep logging to
files in addition to Humio, you need to add an extra handler that logs to a
stream (``stderr`` by default), and you need to specify a format for it. This
example just redefines the log line format NAV uses by default.
Copy link
Contributor

Choose a reason for hiding this comment

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

You should perhaps actually show here what NAV's default is, without talking about humiologging. Then you can show how to add humiologging afterwards so that that it is easy to see the state before and after.

@lunkwill42 lunkwill42 requested a review from hmpf September 7, 2023 09:07
As requested in review, this begins by documenting how to replicate
NAV's default logging config, and proceeds to document Humio only after
that.
@lunkwill42 lunkwill42 force-pushed the feature/support-standard-logging-fileconfig branch from 9bbc066 to 43cabfa Compare September 7, 2023 09:53
@lunkwill42 lunkwill42 merged commit 1caa681 into Uninett:master Sep 7, 2023
7 of 8 checks passed
@lunkwill42 lunkwill42 deleted the feature/support-standard-logging-fileconfig branch September 7, 2023 09:54
@sonarcloud
Copy link

sonarcloud bot commented Sep 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

None yet

2 participants