-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
promtail: no need for GCP promtail_instance label now that loki supports out-of-order writes #4556
promtail: no need for GCP promtail_instance label now that loki supports out-of-order writes #4556
Conversation
Great catch. Are you willing to sign the CLA? |
f301cff
to
0b6ae2a
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @james-callahan. LGTM!.
@owen-d this could be a breaking change. should be add it to the upgrade guide?
Good call @kavirajk. Let's add a small note to the upgrade guide mentioning the lost label :) |
It also looks like we'll need one small change to pass the linter:
|
0b6ae2a
to
403f638
Compare
403f638
to
ca691cc
Compare
Fixed + rebased. The tests now seem to be failing for unrelated reasons? |
Hey, the reason it's failing is because by removing some code, it's actually changed the expected
You should be able to resolve this by running |
…rts out-of-order writes Closes grafana#4474
ca691cc
to
6b94342
Compare
huh. weird that this manifested so far away in the tests (only failures in compactor retention tests); while the
Done. |
Signed-off-by: Kaviraj <[email protected]>
Signed-off-by: Kaviraj <[email protected]>
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #4474
Special notes for your reviewer:
Checklist