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

Add a new format, JSON_GCP #61

Merged
merged 3 commits into from
Jul 11, 2023
Merged

Add a new format, JSON_GCP #61

merged 3 commits into from
Jul 11, 2023

Conversation

jhchabran
Copy link
Member

When deploying a service that sends out logs to cloud logging, the severity was not parsed and because logs of our services are all sent out of STDERR, they all defaulted to severity: "ERROR", making it quite hard to read.

This PRs adds a new SRC_LOG_FORMAT=json_gcp, which outputs json logs in a structure that cloud logging understands.

See the results of a little test in Cloud run:

image

@codecov-commenter
Copy link

Codecov Report

Merging #61 (644425a) into main (ad2d71b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #61   +/-   ##
=======================================
  Coverage   61.62%   61.62%           
=======================================
  Files          16       16           
  Lines         641      641           
=======================================
  Hits          395      395           
  Misses        223      223           
  Partials       23       23           
Impacted Files Coverage Δ
init.go 0.00% <ø> (ø)

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

Copy link
Member

@bobheadxi bobheadxi left a comment

Choose a reason for hiding this comment

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

nice, clean change :) thank you!

output/output.go Outdated
Comment on lines 25 to 28
case string(FormatJSON), "logfmt":
// True 'logfmt' has significant limitations around certain field types:
// https://github.com/jsternberg/zap-logfmt#limitations so since it implies a
// desire for a somewhat structured format, we interpret it as OutputJSON.
"logfmt":
return FormatJSON
case string(FormatConsole),
case string(FormatConsole), "condensed":
// The previous 'condensed' format is optimized for local dev, so it serves the
// same purpose as OutputConsole
"condensed":
Copy link
Member

Choose a reason for hiding this comment

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

nit - keep the old formatting, or bring the docstring above the case

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh you're right, while it took me 2m to parse the original syntax with the line break and I like it much better that way, I missed that it messes how the comment reads now. Fixing this!

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

5 participants