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

Replacing go-kit/kit/log with go-kit/log #4484

Merged
merged 5 commits into from
Oct 15, 2021

Conversation

dannykopping
Copy link
Contributor

@dannykopping dannykopping commented Oct 15, 2021

Re-implementing #4164 after it went stale

What this PR does / why we need it:
This PR aims to align Loki with Prometheus (see prometheus/prometheus#8927) in its use of go-kit's logger. go-kit/kit/log was split out into go-kit/log, and that's all we use of go-kit ostensibly.

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
This bit me recently when trying to understand why debug logs were printed when I'd explicitly set -log.level=info. It turns out that if a log message is written from within the vendored Prometheus code, it uses the newer go-kit/log, so this coercion mysterious fails even though the type seems correct. That was a fun one to debug 🙃

I upgraded weaveworks/common after weaveworks/common#226 was merged, since it was suffering from the same issue, same for cortex (cortexproject/cortex#4421) although that was revendored yesterday.

Checklist

  • Documentation added
  • Tests updated

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Danny Kopping added 4 commits October 15, 2021 17:12
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Cleaning go mod

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
@dannykopping dannykopping marked this pull request as ready for review October 15, 2021 16:49
@dannykopping dannykopping requested a review from a team as a code owner October 15, 2021 16:49
@@ -48,6 +48,12 @@ linters-settings:
goimports:
local-prefixes: github.com/grafana/loki/pkg

depguard:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice

Copy link
Collaborator

@slim-bean slim-bean left a comment

Choose a reason for hiding this comment

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

LGTM!

@slim-bean slim-bean merged commit cff8324 into grafana:main Oct 15, 2021
@dannykopping dannykopping deleted the dannykopping/logger branch October 15, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants