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

refactor(rcmgr): use default libp2p rcmgr metrics #9947

Merged
merged 5 commits into from
Nov 27, 2024
Merged

Conversation

marten-seemann
Copy link
Member

Removes the bespoke rcmgr metrics with the default libp2p metrics. This allows us to use the libp2p-maintained dashboard on all of our Grafana instances.

It would be great if this could be merged for the v0.21 release. There've been quite a few instances recently where libp2p dashboards have helped us pinpoint a problem. Not having rcmgr metrics has made debugging more complicated than necessary.

@marten-seemann marten-seemann requested a review from a team as a code owner June 14, 2023 09:20
@marten-seemann
Copy link
Member Author

Not sure why sharness is failing. Has something to do with Prometheus metrics. I don't have a sharness setup locally, so I won't be able to debug this. I'd also argue (again) that it doesn't make sense to have any test that asserts what metrics downstream libraries are exposing.

@lidel lidel requested a review from Jorropo July 31, 2023 13:48
@lidel
Copy link
Member

lidel commented Jul 31, 2023

Triage note: @Jorropo will take a look

@Jorropo Jorropo force-pushed the libp2p-rcmgr-metrics branch from 3ba1080 to cd25750 Compare July 31, 2023 15:05
@BigLep BigLep mentioned this pull request Aug 3, 2023
@Jorropo Jorropo removed their request for review March 4, 2024 07:57
@Jorropo Jorropo removed their assignment Mar 4, 2024
@gammazero gammazero added the need/triage Needs initial labeling and prioritization label Nov 5, 2024
@lidel lidel force-pushed the libp2p-rcmgr-metrics branch from 61c83f0 to 887c036 Compare November 27, 2024 19:22
@lidel lidel mentioned this pull request Nov 27, 2024
40 tasks
@lidel lidel changed the title rcmgr: use default libp2p rcmgr metrics refactor(rcmgr): use default libp2p rcmgr metrics Nov 27, 2024
@lidel lidel self-assigned this Nov 27, 2024
we no longer expect those extra ones
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

This is a nice cleanup.

The sharness was failing because we have early warning when exposed metrics change, and this PR introduced diff:

-libp2p_rcmgr_memory_allocations_allowed_total
-libp2p_rcmgr_memory_allocations_blocked_total
-libp2p_rcmgr_peer_blocked_total
-libp2p_rcmgr_peers_allowed_total

Rebased, updated tests and added changelog entry informing people about removal, and that Kubo switched to upstream ones.

Once CI is green, will ship in 0.33.

@lidel lidel merged commit 778a418 into master Nov 27, 2024
11 checks passed
@lidel lidel deleted the libp2p-rcmgr-metrics branch November 27, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
No open projects
Status: 🔎 In Review
Development

Successfully merging this pull request may close these issues.

4 participants