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

suppress errors from Cobra #73

Merged
merged 1 commit into from
Dec 15, 2023
Merged

suppress errors from Cobra #73

merged 1 commit into from
Dec 15, 2023

Conversation

MikePresman
Copy link
Contributor

@MikePresman MikePresman commented Dec 14, 2023

In our observability stack we detected that we were emitting the logging twice for the same event and cause. After further investigation and recreating this locally
CleanShot 2023-12-14 at 15 56 01@2x

We can confirm we are double logging. After some investigation it became clear that cobra takes it upon itself to log errors coming out of CLI events it handles. Furthermore, cobra doesn't respect or even know about our log-encoding resulting in this problem. Luckily they provide a flag to suppress these logs.

As such this PR introduces this flag to be true so we don't have to see double logging anymore and can now just see the proper JSON logging we want from Zap

after the flag is set...
CleanShot 2023-12-14 at 16 08 25@2x

🎩

  1. Have the flag set to false or not set at all
  2. Change the client-gc-project command in the makefile to --keep 0 (this will error)
  3. run that command make client-gc-project and see the error being logged twice
  4. Now set the flag and run client-gc-project again and confirm you see just the JSON error

Copy link
Contributor

@angelini angelini left a comment

Choose a reason for hiding this comment

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

Those two screenshots look identical, but this sounds like the root of the problem

@MikePresman MikePresman merged commit 0b31505 into main Dec 15, 2023
4 checks passed
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.

3 participants