feat(starknet_monitoring_endpoint): add metrics endpoint

[dan-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[dan-starkware]

• chore: rename tests for consistency #2521
• chore(starknet_monitoring_endpoint): revert app changes #2504
• feat(starknet_monitoring_endpoint): add config for collect metrics #2472
• feat(starknet_monitoring_endpoint): add metrics endpoint #2469 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-05T09:54:52Z
• Started at 2024-12-05T17:52:21Z
• Started at 2024-12-08T11:36:04Z
• Started at 2024-12-08T11:44:06Z
• Started at 2024-12-09T10:46:28Z

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.40%. Comparing base (e3165c4) to head (d9e044c).
Report is 760 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2469       +/-   ##
===========================================
+ Coverage   40.10%   77.40%   +37.29%     
===========================================
  Files          26      387      +361     
  Lines        1895    41449    +39554     
  Branches     1895    41449    +39554     
===========================================
+ Hits          760    32083    +31323     
- Misses       1100     7083     +5983     
- Partials       35     2283     +2248     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Itay-Tsabary-Starkware]

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @dan-starkware)

---

crates/starknet_monitoring_endpoint/src/monitoring_endpoint.rs line 101 at r1 (raw file):

        None => StatusCode::METHOD_NOT_ALLOWED.into_response(),
    }
}

Are we expected not to collect metrics at some point? IIUC this is turned on once, and we'll keep adding new relevant metrics as we go.
Could we avoid using Option here, and simply return handle.render().into_response()?

Code quote:

/// In case the node doesn’t collect metrics returns an empty response with status code 405: method
/// not allowed.
#[instrument(level = "debug", ret, skip(prometheus_handle))]
async fn metrics(prometheus_handle: Option<PrometheusHandle>) -> Response {
    match prometheus_handle {
        Some(handle) => handle.render().into_response(),
        None => StatusCode::METHOD_NOT_ALLOWED.into_response(),
    }
}

---

crates/starknet_monitoring_endpoint/src/monitoring_endpoint_test.rs line 51 at r1 (raw file):

#[tokio::test]
async fn test_ready() {

Could you please align the test names (with/without test_ prefix) within this module? I don't mind to which, just aiming for consistency.

Code quote:

async fn test_ready() {

---

crates/starknet_monitoring_endpoint/src/monitoring_endpoint_test.rs line 69 at r1 (raw file):

    absolute_counter!(metric_name, metric_value);

    let response = request_app(app, crate::monitoring_endpoint::METRICS).await;

Please import at top, and use METRICS here.

Code quote:

crate::monitoring_endpoint::METRICS

[dan-starkware]

crates/starknet_monitoring_endpoint/src/monitoring_endpoint_test.rs line 69 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Please import at top, and use METRICS here.

Done.

[dan-starkware]

crates/starknet_monitoring_endpoint/src/monitoring_endpoint_test.rs line 51 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Could you please align the test names (with/without test_ prefix) within this module? I don't mind to which, just aiming for consistency.

Done further up the stack

[Itay-Tsabary-Starkware]

Reviewed 2 of 4 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-starkware)

---

crates/starknet_monitoring_endpoint/src/monitoring_endpoint.rs line 101 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Are we expected not to collect metrics at some point? IIUC this is turned on once, and we'll keep adding new relevant metrics as we go.
Could we avoid using Option here, and simply return handle.render().into_response()?

Could you please add a todo on my name (tsabary) to handle the Option setup?
Changed to non-blocking

[dan-starkware]

Reviewed 1 of 4 files at r1, 1 of 2 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-starkware)

[dan-starkware]

Merge activity

• Dec 9, 7:01 AM EST: A user merged this pull request with Graphite.