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

Make API for omitting keys more approachable. #725

Merged
merged 2 commits into from
Jul 9, 2019

Conversation

juicemia
Copy link
Contributor

The way to omit keys from log lines can be more obvious to package consumers.

This addresses #724.

Hugo Torres added 2 commits June 24, 2019 12:18
Currently encoders will omit fields when encoding log entries if the key
configured for that field is set to the empty string. However, this
behavior isn't documented or specified explicitly as being part of the
interface for encoding.

This adds documentation to the `Encoder` interface that explicitly
outlines the behavior that should be implemented when omitting fields.
The empty string can be used to omit a key from a log line. However,
there's no explicitly exposed API for using this behavior.

This cleans up the API for users by allowing them to use a constant for
omitting the key instead of throwing around `""` everywhere, or making
their own constant and hoping that `""` will always omit the key.
@CLAassistant
Copy link

CLAassistant commented Jun 25, 2019

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jun 25, 2019

Codecov Report

Merging #725 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #725   +/-   ##
=======================================
  Coverage   97.43%   97.43%           
=======================================
  Files          40       40           
  Lines        2109     2109           
=======================================
  Hits         2055     2055           
  Misses         46       46           
  Partials        8        8
Impacted Files Coverage Δ
zapcore/entry.go 90.1% <ø> (ø) ⬆️
zapcore/encoder.go 83.13% <ø> (ø) ⬆️

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 853ac18...556557f. Read the comment docs.

Copy link
Collaborator

@abhinav abhinav 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 the change!

This seems pretty reasonable to me. WDYT, @prashantv @jcorbin

Copy link
Collaborator

@prashantv prashantv left a comment

Choose a reason for hiding this comment

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

LGTM

@prashantv prashantv merged commit 9a9fa7d into uber-go:master Jul 9, 2019
cgxxv pushed a commit to cgxxv/zap that referenced this pull request Mar 25, 2022
* Explicitly document key omission in encoding

Currently encoders will omit fields when encoding log entries if the key
configured for that field is set to the empty string. However, this
behavior isn't documented or specified explicitly as being part of the
interface for encoding.

This adds documentation to the `Encoder` interface that explicitly
outlines the behavior that should be implemented when omitting fields.

* Add key for omission to package API

The empty string can be used to omit a key from a log line. However,
there's no explicitly exposed API for using this behavior.

This cleans up the API for users by allowing them to use a constant for
omitting the key instead of throwing around `""` everywhere, or making
their own constant and hoping that `""` will always omit the key.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants