apollo_dashboard: add new module queries_builder

[matanl-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[matanl-starkware]

• apollo_dashboard: use query_builder increase fn in panels #9866
• apollo_dashboard: add new module queries_builder #9863 👈 (View in Graphite)
• apollo_dashboard: create MetricCommon trait #9857
• apollo_dashboard: simpler new fn for Panel #9856
• main-v0.14.1

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

[Itay-Tsabary-Starkware]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion

---

crates/apollo_dashboard/src/lib.rs line 9 at r1 (raw file):

mod metric_definitions_test;
mod panels;
mod queries_builder;

Please annotate this with #[cfg(test)], and remove the inner-file annotations
(it seems like the entire module is just for testing)

Code quote:

mod queries_builder;

[Itay-Tsabary-Starkware]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @matanl-starkware)

---

crates/apollo_dashboard/src/queries_builder.rs line 3 at r1 (raw file):

pub(crate) mod queries {
    #[cfg(test)]
    use apollo_metrics::metrics::MetricCommon;

Move to the top

Code quote:

use apollo_metrics::metrics::MetricCommon;

[Itay-Tsabary-Starkware]

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

---

crates/apollo_dashboard/src/queries_builder.rs line 13 at r1 (raw file):

#[cfg(test)]
mod tests {
    use apollo_metrics::metrics::{MetricGauge, MetricScope};

Move to the top

Code quote:

use apollo_metrics::metrics::{MetricGauge, MetricScope};

[Itay-Tsabary-Starkware]

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

---

crates/apollo_dashboard/src/queries_builder.rs line 1 at r1 (raw file):

pub(crate) mod queries {

Remove this, and instead define fn increase as a fn of this module; no need for pub(crate) as well.

Code quote:

pub(crate) mod queries {

[Itay-Tsabary-Starkware]

Reviewable status: 0 of 2 files reviewed, 5 unresolved discussions (waiting on @matanl-starkware)

---

crates/apollo_dashboard/src/queries_builder.rs line 22 at r1 (raw file):

        let q = queries::increase(&m, "5m");
        assert_eq!(q, r#"increase(testing{cluster=~"$cluster", namespace=~"$namespace"}[5m])"#);
    }

Move outside of mod teststo the top hierachy of the queries_build module , and then delete mod tests

Code quote:

    #[test]
    fn increase_formats_correctly() {
        let m = MetricGauge::new(MetricScope::Batcher, "testing", "Fake description");
        let q = queries::increase(&m, "5m");
        assert_eq!(q, r#"increase(testing{cluster=~"$cluster", namespace=~"$namespace"}[5m])"#);
    }

[Itay-Tsabary-Starkware]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion

---

crates/apollo_dashboard/src/lib.rs line 9 at r1 (raw file):

mod metric_definitions_test;
mod panels;
mod queries_builder;

Saw pr #9866 so retracted all previous comments.

Please:

1. rename queries_builder to query_builder
2. create a query_builder_tests module
3. add query_builder_tests to query_builder with the cfg-test annotation

Code quote:

mod queries_builder;

[Itay-Tsabary-Starkware]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @matanl-starkware)

---

crates/apollo_dashboard/src/queries_builder.rs line 21 at r1 (raw file):

        let m = MetricGauge::new(MetricScope::Batcher, "testing", "Fake description");
        let q = queries::increase(&m, "5m");
        assert_eq!(q, r#"increase(testing{cluster=~"$cluster", namespace=~"$namespace"}[5m])"#);

We have this defined as a macro, please check metric_label_filter

1. I'll note that afaiu it doesn't have to be a macro anymore, and can be changed to a normal const instead
2. We also have a util struct called Template that can be relevant here as well

Code quote:

{cluster=~"$cluster", namespace=~"$namespace"}

[matanl-starkware]

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

---

crates/apollo_dashboard/src/lib.rs line 9 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Saw pr #9866 so retracted all previous comments.

Please:

1. rename queries_builder to query_builder
2. create a query_builder_tests module
3. add query_builder_tests to query_builder with the cfg-test annotation

Done.

---

crates/apollo_dashboard/src/queries_builder.rs line 1 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Remove this, and instead define fn increase as a fn of this module; no need for pub(crate) as well.

Without pub(crate), it becomes private, and unittest can't use it.

---

crates/apollo_dashboard/src/queries_builder.rs line 21 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

We have this defined as a macro, please check metric_label_filter

1. I'll note that afaiu it doesn't have to be a macro anymore, and can be changed to a normal const instead
2. We also have a util struct called Template that can be relevant here as well

Done.

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware reviewed 3 of 4 files at r2, all commit messages.
Reviewable status: 3 of 4 files reviewed, all discussions resolved

---

crates/apollo_dashboard/src/query_builder.rs line 1 at r2 (raw file):

use apollo_metrics::metrics::MetricCommon;

Please add a new line after the use section (non-blocking)

[Itay-Tsabary-Starkware]

Reviewable status: 3 of 4 files reviewed, all discussions resolved

---

crates/apollo_dashboard/src/lib.rs line 11 at r2 (raw file):

// TODO(MatanL): Remove cfg(test) when used
#[cfg(test)]

Not requried afaiu

Code quote:

// TODO(MatanL): Remove cfg(test) when used
#[cfg(test)]

[matanl-starkware]

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

---

crates/apollo_dashboard/src/lib.rs line 11 at r2 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Not requried afaiu

It is required (increase not used in this PR, unless under test)

[matanl-starkware]

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

[graphite-app]

Merge activity

• Nov 2, 11:30 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware reviewed 1 of 4 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @matanl-starkware)