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

zapcore: Add LevelOf(LevelEnabler), UnknownLevel #1147

Merged
merged 5 commits into from Aug 16, 2022
Merged

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Aug 15, 2022

Add a new function LevelOf that reports the minimum enabled log level
for a LevelEnabler.

This works by looping through all known Zap levels in-order,
returning the newly introduced UnknownLevel if none of the known levels
are enabled.

A LevelEnabler or Core implementation may implement the Level() Level
method to override and optimize this behavior.
AtomicLevel already implemented this method.
This change adds the method to all Core implementations shipped with
Zap.

Note:
UnknownLevel is set at FatalLevel+1 to account for the possibility that
users of Zap are using DebugLevel-1 as their own custom log level--even
though this isn't supported, it's preferable not to break these users.
Users are less likely to use FatalLevel+1 since Fatal means "exit the
application."

Refs #1144
Supersedes #1143, which was not backwards compatible
Refs GO-1586

Add a new function LevelOf that reports the minimum enabled log level
for a LevelEnabler.

This works by looping through all known Zap levels in-order,
returning the newly introduced UnknownLevel if none of the known levels
are enabled.

A LevelEnabler or Core implementation may implement the `Level() Level`
method to override and optimize this behavior.
AtomicLevel already implemented this method.
This change adds the method to all Core implementations shipped with
Zap.

Note:
UnknownLevel is set at FatalLevel+1 to account for the possibility that
users of Zap are using DebugLevel-1 as their own custom log level--even
though this isn't supported, it's preferable not to break these users.
Users are less likely to use FatalLevel+1 since Fatal means "exit the
application."

Resolves #1144
Supersedes #1143, which was not backwards compatible
@codecov
Copy link

codecov bot commented Aug 15, 2022

Codecov Report

Merging #1147 (391e7b0) into master (4b03bc5) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1147      +/-   ##
==========================================
+ Coverage   98.32%   98.33%   +0.01%     
==========================================
  Files          49       49              
  Lines        2146     2160      +14     
==========================================
+ Hits         2110     2124      +14     
  Misses         28       28              
  Partials        8        8              
Impacted Files Coverage Δ
level.go 100.00% <ø> (ø)
zapcore/core.go 93.75% <100.00%> (+0.20%) ⬆️
zapcore/hook.go 100.00% <100.00%> (ø)
zapcore/increase_level.go 100.00% <100.00%> (ø)
zapcore/level.go 100.00% <100.00%> (ø)
zapcore/sampler.go 100.00% <100.00%> (ø)
zapcore/tee.go 100.00% <100.00%> (ø)
zaptest/observer/observer.go 100.00% <100.00%> (ø)

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

@abhinav abhinav requested review from mway and sywhang Aug 15, 2022
abhinav added a commit that referenced this pull request Aug 15, 2022
Add a `Level() Level` method on Logger and SugaredLogger
that reports the current minimum enabled log level for the logger.

This relies on the zapcore.LevelOf function added in #1147.

Resolves #1144
internal/level_enab.go Outdated Show resolved Hide resolved
zapcore/hook_test.go Outdated Show resolved Hide resolved
zapcore/level.go Outdated
// UnknownLevel is an invalid value for Level.
//
// Core implementations may panic if they see messages of this level.
UnknownLevel = _maxLevel + 1
Copy link
Member

@mway mway Aug 15, 2022

Choose a reason for hiding this comment

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

Can we reorganize these so that we have an invalid zero value? e.g.

const (
  InvalidLevel = iota
  DebugLevel
  // ...
  FatalLevel

  _minLevel = DebugLevel
  _maxLevel = FatalLevel
)

Copy link
Contributor

@sywhang sywhang Aug 15, 2022

Choose a reason for hiding this comment

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

Wouldn't this be a breaking change because the integer value of DebugLevel (and everything else) would change?

Copy link
Contributor Author

@abhinav abhinav Aug 15, 2022

Choose a reason for hiding this comment

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

I lean towards not changing values of published constants, even if the line around whether it's a breaking change is a bit blurry. Such a change will break someone, and we don't get much in exchange.

zapcore/level.go Outdated
// UnknownLevel is an invalid value for Level.
//
// Core implementations may panic if they see messages of this level.
UnknownLevel = _maxLevel + 1
Copy link
Member

@mway mway Aug 15, 2022

Choose a reason for hiding this comment

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

nit: s/Unknown/Invalid/

zapcore/tee_test.go Outdated Show resolved Hide resolved
abhinav added a commit that referenced this pull request Aug 15, 2022
Add a `Level() Level` method on Logger and SugaredLogger
that reports the current minimum enabled log level for the logger.

This relies on the zapcore.LevelOf function added in #1147.

Resolves #1144
abhinav added a commit that referenced this pull request Aug 15, 2022
Add a `Level() Level` method on Logger and SugaredLogger
that reports the current minimum enabled log level for the logger.

This relies on the zapcore.LevelOf function added in #1147.

Resolves #1144
mway
mway approved these changes Aug 16, 2022
@abhinav abhinav merged commit 4a895a2 into master Aug 16, 2022
8 checks passed
@abhinav abhinav deleted the abg/core-level branch Aug 16, 2022
abhinav added a commit that referenced this pull request Aug 16, 2022
Add a `Level() Level` method on Logger and SugaredLogger
that reports the current minimum enabled log level for the logger.

This relies on the zapcore.LevelOf function added in #1147.

Resolves #1144
abhinav added a commit that referenced this pull request Aug 24, 2022
Add a `Level() Level` method on Logger and SugaredLogger
that reports the current minimum enabled log level for the logger.

This relies on the zapcore.LevelOf function added in #1147.

Resolves #1144
abhinav added a commit that referenced this pull request Aug 30, 2022
Add a `Level() Level` method on Logger and SugaredLogger
that reports the current minimum enabled log level for the logger.

This relies on the zapcore.LevelOf function added in #1147.

Resolves #1144
Depends on #1147
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.

None yet

3 participants