Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Fix regression by handling nil logger correctly #507

Merged
merged 3 commits into from
Apr 28, 2020

Conversation

vprithvi
Copy link
Contributor

@vprithvi vprithvi commented Apr 28, 2020

  • Jaeger components may be initialized with nil loggers.
  • Verify that the logger is non-nil before logging in the DebugLoggingAdaptor to preserve old behavior

Signed-off-by: Prithvi Raj p.r@uber.com

- Verify that the logger is non-nil before logging

Signed-off-by: Prithvi Raj <p.r@uber.com>
Signed-off-by: Prithvi Raj <p.r@uber.com>
@codecov
Copy link

codecov bot commented Apr 28, 2020

Codecov Report

Merging #507 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #507   +/-   ##
=======================================
  Coverage   88.73%   88.74%           
=======================================
  Files          60       60           
  Lines        3782     3784    +2     
=======================================
+ Hits         3356     3358    +2     
  Misses        309      309           
  Partials      117      117           
Impacted Files Coverage Δ
log/logger.go 100.00% <100.00%> (ø)

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 93622f9...f225553. Read the comment docs.

log/logger.go Outdated
@@ -117,8 +117,11 @@ func DebugLogAdapter(logger Logger) DebugLogger {
if debugLogger, ok := logger.(DebugLogger); ok {
return debugLogger
}
logger.Infof("debug logging disabled")
return debugDisabledLogAdapter{logger: logger}
if logger != nil {
Copy link
Member

Choose a reason for hiding this comment

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

It seems that nil check should come first (even if the cast works on nil).

Signed-off-by: Prithvi Raj <p.r@uber.com>
@vprithvi vprithvi changed the title Fix nil logger Fix regression by handling nil logger correctly Apr 28, 2020
@vprithvi vprithvi merged commit e7a6498 into jaegertracing:master Apr 28, 2020
@vprithvi vprithvi deleted the fix-nil-logger branch April 28, 2020 16:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants