-
Notifications
You must be signed in to change notification settings - Fork 526
runtimes/core: Add metrics #2109
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
base: main
Are you sure you want to change the base?
Conversation
All committers have signed the CLA. |
actually, I will refactor this a bit since metrics-rs isn't flexible enough with the different types that we support. Drafting this for now |
1f5a82a
to
333ecb2
Compare
refactored and added system metrics to this ps |
runtimes/core/src/metrics/manager.rs
Outdated
) -> Arc<dyn Exporter + Send + Sync> { | ||
match self { | ||
Self::Gcp(config) | Self::EncoreCloud(config) => { | ||
runtime_handle.block_on(Self::create_gcp_exporter(config, env, http_client)) |
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.
A bit annoying to need to block here. I guess the only reason we need to do that is because the container metadata needs to be collected? Can we do that lazily/in the background, similar to the lazy gcp client?
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.
I refactored to using a lazy cached client and avoiding the block_on by fetching the container data in the collect step
c7fc1d1
to
2b617df
Compare
Adds the ground work for adding metrics to encore.ts, adds the e_requests_total counter for apps running in gcp/encore cloud that export metrics to gcp cloud monitoring.
Will add support for aws, datadog and prometheus in a follow up.
This also doesnt expose any sys metrics, will add that in a follow up pr as well.