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

chore: Logging improvement #213

Merged
merged 2 commits into from
Feb 3, 2023
Merged

chore: Logging improvement #213

merged 2 commits into from
Feb 3, 2023

Conversation

0lmi
Copy link
Contributor

@0lmi 0lmi commented Feb 1, 2023

The intention of the change is to improve logging to reduce amount of log entries, which will contribute to infra costs savings and making work with logs more comfortable.

The idea is to use /_health endpoint for the healthchecks by a load balancer and don't log those calls.

Currently we log all events when the healthcheck logs are mostly
useful during configuring/debugging but not during normal
operations. Bigger amount of logs makes it longer to find an
event and increases logs saving costs.

Changelog: Title
Ticket: None
Signed-off-by: Alex Miliukov <oleksandr.miliukov@northern.tech>
@mender-test-bot
Copy link

Merging these commits will result in the following changelog entries:

Changelogs

integration-test-runner (master)

New changes in integration-test-runner since master:

  • disable logs for health endpoint

main.go Outdated

r := gin.New()
r.Use(
gin.LoggerWithWriter(gin.DefaultWriter, "/_health"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I'm not completely following, so if I'm running at the info level, then what will this show? Will it still ignore _health?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the idea is to ignore logging for _health endpoint for log levels above debug and only have them for debug and trace.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood, thx 👍 I was a bit lazy, but I see now that _health is the explicit filter to the logger.
Small suggestion, create a string locally, with filter := "_health" and pass it into the gin.LoggerWithWriter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Thanks!

@@ -207,16 +207,22 @@ func processGitHubWebhook(
if conf.isProcessPREvents {
pr := webhookEvent.(*github.PullRequestEvent)
return processGitHubPullRequest(ctx, pr, githubClient, conf)
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

Changelog: None
Ticket: None
Signed-off-by: Alex Miliukov <oleksandr.miliukov@northern.tech>
@0lmi 0lmi merged commit db40672 into mendersoftware:master Feb 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.

None yet

3 participants