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

Format log message before logging with logr #4143

Merged
merged 8 commits into from
Jun 26, 2023

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented May 27, 2023

Fixes #4141

The logr calling convention does not support fmt semantics for message formatting. Format the message before it is sent to logr for logging.

Fixes open-telemetry#4141

The logr calling convention does not support fmt semantics for message
formatting. Format the message before it is sent to logr for logging.
@codecov
Copy link

codecov bot commented May 27, 2023

Codecov Report

Merging #4143 (02299fb) into main (5e69812) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #4143     +/-   ##
=======================================
- Coverage   83.5%   83.5%   -0.1%     
=======================================
  Files        181     181             
  Lines      14162   14162             
=======================================
- Hits       11832   11830      -2     
- Misses      2103    2105      +2     
  Partials     227     227             
Impacted Files Coverage Δ
exporters/zipkin/zipkin.go 67.3% <100.0%> (ø)

... and 1 file with indirect coverage changes

Don't shadow the log pkg.
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Just nit suggestions.

exporters/zipkin/zipkin_test.go Show resolved Hide resolved
exporters/zipkin/zipkin_test.go Show resolved Hide resolved
exporters/zipkin/zipkin_test.go Show resolved Hide resolved
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Just nit suggestions.

MrAlias and others added 2 commits June 26, 2023 09:39
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@MrAlias MrAlias merged commit 1b02c91 into open-telemetry:main Jun 26, 2023
@MrAlias MrAlias deleted the fix-4141 branch June 26, 2023 16:54
@MrAlias MrAlias added this to the v1.17.0 milestone Aug 3, 2023
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.

Problem with logging after introducing logr
5 participants