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

cleanup/metrics: Remove deprecated metrics in 1.9 #13659

Merged
merged 1 commit into from
Nov 14, 2020

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Oct 21, 2020

This commit is to clean up all deprecated metrics and labels in 1.9.

Signed-off-by: Tam Mach sayboras@yahoo.com

TODO:

  • Update docs (release note)
cleanup/metrics: Cleanup deprecated metrics

@sayboras sayboras added the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Oct 21, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 21, 2020
@sayboras sayboras force-pushed the deprecate/metrics branch 3 times, most recently from e4f2be9 to 0ba14b4 Compare October 21, 2020 06:56
@sayboras sayboras added area/metrics Impacts statistics / metrics gathering, eg via Prometheus. kind/cleanup This includes no functional changes. labels Oct 21, 2020
@sayboras sayboras force-pushed the deprecate/metrics branch 3 times, most recently from 9fb5720 to e88043c Compare November 7, 2020 02:04
@aanm aanm removed the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Nov 10, 2020
@sayboras sayboras force-pushed the deprecate/metrics branch 2 times, most recently from c175496 to 77c840d Compare November 11, 2020 10:40
@sayboras sayboras changed the title cleanup/metrics: Cleanup deprecated metrics cleanup/metrics: Remove deprecated metrics in 1.9 Nov 11, 2020
@sayboras sayboras added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Nov 11, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 11, 2020
@sayboras
Copy link
Member Author

test-me-please

This commit is to clean up all deprecated metrics in 1.9.

Signed-off-by: Tam Mach <sayboras@yahoo.com>
@sayboras
Copy link
Member Author

test-me-please

@sayboras
Copy link
Member Author

test failed due to docker limit, I will re-run later (maybe morning my time)

21:58:58   > [builder 1/4] FROM docker.io/library/golang:1.15.4:
21:58:58  ------
21:58:58  failed to solve with frontend dockerfile.v0: failed to solve with frontend gateway.v0: rpc error: code = Unknown desc = failed to build LLB: failed to load cache key: failed to copy: httpReaderSeeker: failed open: unexpected status code https://registry-1.docker.io/v2/library/golang/manifests/sha256:ef9a25dc0bf0a4b5e28c27cf0e8f0cd2d29fb12b4cc9cc5089471328dd0d808c: 429 Too Many Requests - Server message: toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit
21:58:58  Makefile.docker:98: recipe for target 'docker-plugin-image' failed
21:58:58  make: *** [docker-plugin-image] Error 1

@sayboras sayboras marked this pull request as ready for review November 11, 2020 11:06
@sayboras sayboras requested a review from a team November 11, 2020 11:06
@sayboras sayboras requested a review from a team as a code owner November 11, 2020 11:06
@sayboras sayboras requested a review from a team November 11, 2020 11:06
@sayboras sayboras requested review from a team as code owners November 11, 2020 11:06
@sayboras
Copy link
Member Author

test-me-please

@sayboras
Copy link
Member Author

retest-4.9

Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

🧹 🎉

@christarazi
Copy link
Member

test-me-please

@sayboras
Copy link
Member Author

retest-runtime

Copy link
Member

@nebril nebril left a comment

Choose a reason for hiding this comment

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

Nice. Should we also update grafana dashboards, or have they already been updated to not use deprecated metrics?

@sayboras
Copy link
Member Author

Nice. Should we also update grafana dashboards, or have they already been updated to not use deprecated metrics?

Yes, it's already done as part of #12826, so grafana dashboards in 1.9 stopped using deprecated metrics/label.

Copy link
Contributor

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Great! Since 1.9 is now released, shouldn't this target 1.10 instead? I see multiple 1.10 references in removed TODOs 👀

@sayboras
Copy link
Member Author

Great! Since 1.9 is now released, shouldn't this target 1.10 instead? I see multiple 1.10 references in removed TODOs

Not sure if I understand correctly. As 1.9 is released, master branch will be for 1.10 only, this change will not be backported to earlier version. Feel free to let me know otherwise.

@christarazi
Copy link
Member

Great! Since 1.9 is now released, shouldn't this target 1.10 instead? I see multiple 1.10 references in removed TODOs eyes

I think you might be confused by the PR title slightly. It should read Remove deprecated metrics _from_ 1.9.

Merging.

@christarazi christarazi merged commit c158fb1 into cilium:master Nov 14, 2020
@sayboras sayboras deleted the deprecate/metrics branch November 15, 2020 00:23
@ti-mo
Copy link
Contributor

ti-mo commented Nov 16, 2020

It should read Remove deprecated metrics _from_ 1.9.

Gotcha, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Impacts statistics / metrics gathering, eg via Prometheus. kind/cleanup This includes no functional changes. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants