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

log line inconsistency #1817

Closed
frzifus opened this issue Mar 16, 2022 · 5 comments · Fixed by #2020
Closed

log line inconsistency #1817

frzifus opened this issue Mar 16, 2022 · 5 comments · Fixed by #2020
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@frzifus
Copy link
Member

frzifus commented Mar 16, 2022

Describe the bug
Seems we use plain logrus in some places instead of the propagated logr.Logger wrapper. on one hand this leads to bad readability, on the other hand i had problems to change the log level in all places at once.

To Reproduce
Steps to reproduce the behavior:

  1. Deploy Operator
  2. Deploy Jaeger
  3. View Operator logs

Expected behavior

  • Seeing the same format of logs in every line.
  • Being able to change log-levels globally.

Screenshots
image

Version (please complete the following information):

  • Jaeger version: 1.32.0
@frzifus frzifus added the bug Something isn't working label Mar 16, 2022
@iblancasa
Copy link
Collaborator

What do you think about adding something to ensure the logging is not done using logrus directly (from the CI, for instance)? Is something I can work on.

@frzifus
Copy link
Member Author

frzifus commented Mar 22, 2022

sure, what did you have in mind? Some kind of linting rule? I wouldn't prioritize it too highly, but it would certainly be helpful.

@iblancasa
Copy link
Collaborator

Yes. Something like looking for usages of logrus.<Logging function> and fail when something is found.
I know we first need to fix this but is to avoid new issues in the future. What do you think?

@frzifus
Copy link
Member Author

frzifus commented Mar 23, 2022

I'm not sure if this will adequately address the issue. Imports are often renamed. Found this in cmd/generate.

e.g.

	log "github.com/sirupsen/logrus"

@rubenvp8510 rubenvp8510 added enhancement New feature or request and removed bug Something isn't working labels Apr 5, 2022
@frzifus frzifus added good first issue Good for newcomers help wanted Extra attention is needed labels Jun 26, 2022
@iblancasa
Copy link
Collaborator

iblancasa commented Jul 13, 2022

Where is the logr.Logger wrapper located? I'm not able to find it @frzifus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants