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

Create-Plugin: Backend template: Switch from Info to Debug level #146

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

xnyo
Copy link
Member

@xnyo xnyo commented Nov 15, 2022

What this PR does / why we need it:

  • Switched the log level in the backend template from Info to Debug
  • Adds a comment to remind to not log sensitive information at Info level

Which issue(s) this PR fixes:

Fixes #140

Special notes for your reviewer:

@xnyo xnyo added enhancement New feature or request create-plugin related to the create-plugin tool labels Nov 15, 2022
@xnyo xnyo requested a review from wbrowne November 15, 2022 14:08
@xnyo xnyo self-assigned this Nov 15, 2022
log.DefaultLogger.Info("QueryData called", "request", req)
// when logging at a non-Debug level, make sure you don't include sensitive information in the message
// (like the *backend.QueryDataRequest)
log.DefaultLogger.Debug("QueryData called", "request", req)
Copy link
Member

Choose a reason for hiding this comment

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

In this case I'd remove the *backend.QueryDataRequest param completely just in case people don't think to read the comment, don't understand it or just blindly delete it 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

Like this?

Suggested change
log.DefaultLogger.Debug("QueryData called", "request", req)
log.DefaultLogger.Debug("QueryData called")

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think that should be fine! You could also add something like:

log.DefaultLogger.Debug("QueryData called", "numQueries", len(req.Queries))

to demonstrate how to pass key values to the log. Totally up to you 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! I think demonstrating how to pass key/value to the log is valuable 👍 👍

@xnyo xnyo requested a review from wbrowne November 15, 2022 14:24
Copy link
Member

@wbrowne wbrowne left a comment

Choose a reason for hiding this comment

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

👏

@xnyo xnyo merged commit f1e24f0 into main Nov 15, 2022
@xnyo xnyo deleted the giuseppe/140-log-level branch November 15, 2022 17:06
@grafanabot
Copy link
Contributor

🚀 PR was released in @grafana/create-plugin@0.5.2 🚀

@grafanabot grafanabot added the released This issue/pull request has been released. label Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
create-plugin related to the create-plugin tool enhancement New feature or request released This issue/pull request has been released.
Projects
Development

Successfully merging this pull request may close these issues.

Enhancement: Backend template logging improvements
3 participants