Prysm: [Instrumentation] P2P Message Monitoring Metrics

[prestonvanloon]

Create and add a p2p middleware adapter to increment monitoring metrics counters via prometheus.

Some of the requirements to close this issue:

• [ ] Brief design on what metrics are interesting to monitor
• [ ] An easily reusable p2p middleware adapter to monitor incoming and outgoing p2p messages
• [ ] Exposing metrics to prometheus via HTTP server (preferrably something like :8080/metrics). This port should be configurable via flag --monitoring-port.
• [ ] Add flag to disable metrics monitoring --disable-monitoring.

If there are more requirements discovered in the design, please add them to the issue in a comment or editing this description.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__This issue now has a funding of 250.0 DAI (250.0 USD @ $1.0/DAI) attached to it.__

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
  
  
  
  • Want to chip in? Add your own contribution here.
  
  
  • Questions? Checkout Gitcoin Help or the Gitcoin Slack
  
  
  • $26,592.92 more funded OSS Work available on the Gitcoin Issue Explorer
  
  

[ceresstation]

Hey @pfilippi24 great to hear that you're interested! Do you have a rough timeline for completion?

[pfilippi24]

Hi @ceresstation
no real timeline, because I need to start digging through you code before I can give a real estimate. But I have nothing else to do over the weekend (and we got bad weather ahead of us). So should be pretty quick.

[ceresstation]

Hey @pfilippi24 are you still interested in working on this? If not I'll pass it back to the crowd, thanks :)

[ceresstation]

Alternatively, is this of interest to you @zachzundel? :)

[leandro-lugaresi]

@ceresstation I am interested!

[ceresstation]

@leandro-lugaresi Awesome, once you apply to start work on the issue I'll accept you :)

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__Work has been started__.

These users each claimed they can complete the work by 1 week, 4 days from now.
Please review their action plans below:

1) leandro-lugaresi has been approved to start work.

HI! I want to start working on this issue.
I only take an overview of the project but my initial approach is these:
Create a new http server as a service using the flags required on the issue description.
Register the metrics:

• The Prometheus client will add automatically all the runtime metrics for us so no work here.
• A good starting point for the p2p metrics is the READ approach, so if possible, I would add Request rate (messages in this case), Errors rate (I need to discover what errors we will consider and how to get them) and the duration of the operations.

Learn more on the Gitcoin Issue Details page.

[gitcoinbot]

@leandro-lugaresi Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[leandro-lugaresi]

@prestonvanloon We are using the flag EnableTracingFlag to enable tracing (the default will be disabled) and the monitoring will be enabled by default and the DisableMonitoringFlag will be used.
Why? It's not better to use the same pattern here? Both enable or disable by default and the flags consistent?
I think it's better to have both (tracing and metrics) enabled by default. WDYT?

[prestonvanloon]

@leandro-lugaresi That's a great observation.

In my opinion, these services should be disabled by default because they may cause a performance impact.

With that said, if someone actually cared about a high-performance configuration then they could disable the flag. In fact, the author that added EnableTracingFlag originally had the inverse logic and I asked them to deviate (I wasn't aware that it would cause an inconsistency).

Let's make these consistent and enable both by default. Thanks!

[leandro-lugaresi]

@prestonvanloon I am with some doubts about how we will disable the metrics and I want to discuss this with you.
I think in two/three ways:

1. The metrics types used by the Prometheus client are very performant and shouldn't harm the performance of the application. considering these we can use the metric types inside the services without any if or guard to validate the flag. In case of the metrics are disable we just don't register them and don't start the HTTP server.
2. Use a generic library with the PrometheusAdapter and a NoOperationAdapter.
3. I didn't like this alternative but, we can build a wrapper for the Prometheus metric system and handle in a single place the metric disabled.

I am really ok with the first option, all the metrics are built with atomic operations and are really fast. WDYT?

[prestonvanloon]

Thanks for investigating these options. I agree that it may not be worth the work at the moment to support a flag. As long as we are not expensively updating metrics then it should be OK.

We should be able to profile this at a later time and determine if we need to disable.

Following the pattern of You Ain't Gonna Need It, let's omit the flag for this for now. It wasn't as straightforward as tracing where tracing provides a no-op and we already know tracing can be expensive.

Thanks!

[mkosowsk]

@leandro-lugaresi did @prestonvanloon's comment provide the proper guidance that you need to complete this ticket? Thanks!

[leandro-lugaresi]

Yes!
I will open a WIP PR tomorrow. I think we can get some metrics but there are some problems that I will discuss in the pull request.

[mkosowsk]

Hi @leandro-lugaresi, how's the WIP PR coming along? Thanks! 👍

[leandro-lugaresi]

@mkosowsk Yes, I will work on this issue today and on the weekend!

[mkosowsk]

@leandro-lugaresi just a friendly check-in here, how's this coming along? Thanks!

[gitcoinbot]

@leandro-lugaresi Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

@leandro-lugaresi Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[leandro-lugaresi]

Yes, I will open the PR today, I just need to finish the bazel files and make some tests with the service running

[gitcoinbot]

@leandro-lugaresi Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

@leandro-lugaresi Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• [x] warning (3 days)
• [ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

@leandro-lugaresi due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

• [x] warning (3 days)
• [x] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[rmshea]

Hey @mkosowsk, looks like there's some good discussion going on in #673 -- will snooze the bot for 5 more days. Let me know if you have any questions ! @leandro-lugaresi

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__Work for 250.0 DAI (250.0 USD @ $1.0/DAI) has been submitted by__:

1. @leandro-lugaresi

@ceresstation please take a look at the submitted work:

• PR by @leandro-lugaresi

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $52,757.15 more funded OSS Work available on the Gitcoin Issue Explorer

[leandro-lugaresi]

@prestonvanloon Everything ok with that issue, right?

[leandro-lugaresi]

@ceresstation :point_up:

[ceresstation]

Hey @leandro-lugaresi sorry for the delay, paying you out now!

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

__The funding of 250.0 DAI (250.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @leandro-lugaresi.__

• Learn more on the Gitcoin Issue Details page
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $37,856.37 more funded OSS Work available on the Gitcoin Issue Explorer