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

Improves logging #416

Merged
merged 4 commits into from
Jul 4, 2023
Merged

Improves logging #416

merged 4 commits into from
Jul 4, 2023

Conversation

nwaldispuehl
Copy link

by:

  • capitalizing log messages,
  • adding additional logging, e.g. for user logins (session creation), and
  • reducing the log level of these spammy grpc info lines to debug.

In our instance there are quite a lot of log lines like these:

INFO[0010]options.go:220 finished unary call with code OK              grpc.code=OK grpc.method=ListDevices grpc.service=proto.Devices grpc.start_time="2023-07-02T20:38:50+02:00" grpc.time_ms=0.258 span.kind=server system=grpc trace.id=a34950b0-b843-4844-b726-2b0abc543a70

By reducing their level to debug they are now filtered in the info setting.

What do you think?

- capitalizing log messages,
- adding additional logging for user logins (session creation), and
- reducing the log level of these spammy grpc `info` lines to `debug`.
@nwaldispuehl nwaldispuehl added the enhancement New feature or request label Jul 2, 2023
@nwaldispuehl nwaldispuehl added this to the 0.9.1 milestone Jul 2, 2023
@nwaldispuehl nwaldispuehl requested a review from a team as a code owner July 2, 2023 19:26
@nwaldispuehl nwaldispuehl self-assigned this Jul 2, 2023
@mergeable
Copy link

mergeable bot commented Jul 2, 2023

Thanks for creating a pull request! A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

PR fix: adapts test to new logging statement
PR fix: fixes typo check by excluding `go.mod`.
Copy link

@fbuetler fbuetler left a comment

Choose a reason for hiding this comment

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

Nice :)

cmd/serve/main.go Show resolved Hide resolved
main.go Show resolved Hide resolved
PR fix: reverts first letter of error messages to lower case.
@nwaldispuehl
Copy link
Author

nwaldispuehl commented Jul 3, 2023

Some unit tests are failing, but this appears to be an intermittent event outside of our control:

time="2023-07-03T10:49:02Z" level=warning msg="DNS lookup failed over TCP for upstream 185.150.99.255: read tcp 10.1.0.34:35558->185.150.99.255:53: i/o timeout"

-> Now working again 🎀

@awlx
Copy link
Member

awlx commented Jul 3, 2023

This was actually a problem on my side I updated DNSDist on those hosts and this caused a restart loop ;). PowerDNS/pdns#12975

@nwaldispuehl nwaldispuehl modified the milestones: 0.9.1, 0.10.0 Jul 4, 2023
@nwaldispuehl nwaldispuehl merged commit 2b40e6c into master Jul 4, 2023
@nwaldispuehl nwaldispuehl deleted the feature/improve-logging branch July 4, 2023 13:30
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.

3 participants