test(starknet_sequencer_metrics): add testing utilities to the metrics

[Yael-Starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[Yael-Starkware]

• test(starknet_sequencer_metrics): add testing utilities to the metrics #4983 👈 (View in Graphite)
• main

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

[Itay-Tsabary-Starkware]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions

---

crates/starknet_sequencer_metrics/src/metrics.rs line 146 at r2 (raw file):

        &self,
        metrics_as_string: &str,
        expected_value: u64,

I think this ought to be generic, like self.parse_numeric_metric is.

Code quote:

u64

---

crates/starknet_sequencer_metrics/src/metrics.rs line 311 at r2 (raw file):

        &self,
        metrics_as_string: &str,
        expected_value: f64,

Same?

Code quote:

f64

---

crates/starknet_sequencer_metrics/src/metrics.rs line 373 at r2 (raw file):

    pub fn parse_histogram_metric(&self, metrics_as_string: &str) -> Option<HistogramValue> {
        parse_histogram_metric(metrics_as_string, self.get_name(), None)
    }

Should also be under #[cfg(any(feature = "testing", test))]

Code quote:

    pub fn parse_histogram_metric(&self, metrics_as_string: &str) -> Option<HistogramValue> {
        parse_histogram_metric(metrics_as_string, self.get_name(), None)
    }

[Itay-Tsabary-Starkware]

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

---

crates/starknet_sequencer_metrics/src/metrics.rs line 464 at r2 (raw file):

            expected_value,
            metric_value
        );

Is there a way to unify the code for the labeled and non-labeled cases?

Code quote:

    pub fn assert_eq(
        &self,
        metrics_as_string: &str,
        expected_value: &HistogramValue,
        label: &[(&'static str, &'static str)],
    ) {
        let metric_value = self.parse_histogram_metric(metrics_as_string, label).unwrap();
        assert!(
            metric_value.sum == expected_value.sum && metric_value.count == expected_value.count,
            "Metric histogram {} {:?} sum or count did not match the expected value. expected \
             value: {:?}, metric value: {:?}",
            self.get_name(),
            label,
            expected_value,
            metric_value
        );

[Yael-Starkware]

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @Itay-Tsabary-Starkware)

---

crates/starknet_sequencer_metrics/src/metrics.rs line 311 at r2 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Same?

Done.

---

crates/starknet_sequencer_metrics/src/metrics.rs line 373 at r2 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Should also be under #[cfg(any(feature = "testing", test))]

Done.

---

crates/starknet_sequencer_metrics/src/metrics.rs line 464 at r2 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Is there a way to unify the code for the labeled and non-labeled cases?

that was the reason I originally implemented this as a macro. it could treat counter, labeled_counter, gauge and labeled_gauge in one macro.

[Yael-Starkware]

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @Itay-Tsabary-Starkware)

---

crates/starknet_sequencer_metrics/src/metrics.rs line 146 at r2 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

I think this ought to be generic, like self.parse_numeric_metric is.

Done.

[Itay-Tsabary-Starkware]

Reviewed 1 of 3 files at r1, 2 of 2 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yael-Starkware)

---

crates/starknet_sequencer_metrics/src/metrics.rs line 464 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

that was the reason I originally implemented this as a macro. it could treat counter, labeled_counter, gauge and labeled_gauge in one macro.

I have a longshot idea on how to unify these without a macro, could you please add a todo on my name (tsabary) to try?

[Yael-Starkware]

Reviewable status: 2 of 5 files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)

---

crates/starknet_sequencer_metrics/src/metrics.rs line 464 at r2 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

I have a longshot idea on how to unify these without a macro, could you please add a todo on my name (tsabary) to try?

done

[Itay-Tsabary-Starkware]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yael-Starkware)

[Itay-Tsabary-Starkware]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yael-Starkware)