-
Notifications
You must be signed in to change notification settings - Fork 30
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
pipeline-manager: add metrics to compiler server #1031
Conversation
f7cd3d8
to
c87e8d6
Compare
crates/pipeline_manager/Cargo.toml
Outdated
@@ -51,6 +51,8 @@ refinery = {version = "0.8.10", features = ["tokio-postgres"]} | |||
reqwest = {version = "0.11.18", features = ["json"]} | |||
url = {version = "2.4.0"} | |||
dirs = "5.0" | |||
prometheus-client = "0.22.0" | |||
lazy_static = "1.4.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check if can use std lib instead (see rust-lang-nursery/lazy-static.rs#214)
https://doc.rust-lang.org/nightly/std/sync/struct.OnceLock.html
https://doc.rust-lang.org/nightly/std/sync/struct.Once.html
invocations: Family::<MetricLabel, Counter>::default(), | ||
latency: Family::<MetricLabel, Histogram>::new_with_constructor(|| { | ||
// These are buckets for measuring SQL and rust compilation times | ||
let buckets = [1.0, 5.0, 10.0, 20.0, 50.0, 100.0, 200.0, 400.0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do the numbers mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Histogram CDF points. So if compilation takes say, 15 seconds, it'll add 1 to buckets 20, 50, 100, 200 and 400.
I don't have a good sense for what the most useful CDF points should be yet, I hope to tweak them as we go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it maybe this can be a comment :)
c87e8d6
to
c06951b
Compare
Signed-off-by: Lalith Suresh <lalith@feldera.com>
c06951b
to
5bee31e
Compare
This commit starts adding metrics to the pipeline-manager components. We'll start with the compiler server for now. We'll expose histograms for compiler latencies as well as a counter for the number of invocations. These two metrics are faceted by two labels: the phase (SQL vs Rust) and the status (Success vs Error).
For now, we'll always host metrics via a scrape endpoint on a different port (0.0.0.0:9000/metrics) than the usual APIs (80/8080). This is to make sure that we can avoid exposing the scraping endpoint outside the docker ensemble or cluster in a real deployment.
Is this a user-visible change (yes/no): yes