Prometheus on Thin #3445
Merged
Prometheus on Thin #3445
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove http_status - Remove log_count - Remove outstanding_requests
- Adjusted tests to be more specific
- Removed one allow for a method, which is no longer called on PromUpdater - Removed unnecessary allows for updaters where updaters are not being tested
svkrieger
force-pushed
the
prometheus-on-thin
branch
from
5eb2588 to
716a3e2
Compare
svkrieger
force-pushed
the
prometheus-on-thin
branch
from
7a4f401 to
174ed46
Compare
- Use base formats like seconds instead of milliseconds - Change requests_completed to counter metric - Use one metric for queue lengths and use labels for different queues - Register metrics and initialize them for discoverability - Change histogram buckets
The metrics `report_diego_cell_sync_duration`, `report_deployment_duration`, `update_synced_invalid_lrps` are being emitted on the scheduler VM. There is no web server and therefore also no endpoint, which could serve those metrics. For now we decided to remove those prometheus metrics and just keep the statsd metrics. If those metrics should be also available through prometheus in the future, we probably have to deploy additional jobs on the scheduler VM, which take care of publishing the metrics, so they can be collected by the prom_scraper job.
svkrieger
force-pushed
the
prometheus-on-thin
branch
from
174ed46 to
b6ed2b7
Compare
svkrieger
commented
Oct 13, 2023
johha
reviewed
Nov 2, 2023
philippthun
reviewed
Nov 6, 2023
spec/unit/lib/cloud_controller/metrics/prometheus_updater_spec.rb
Outdated
Show resolved
Hide resolved
philippthun
reviewed
Nov 7, 2023
philippthun
reviewed
Nov 7, 2023
svkrieger
force-pushed
the
prometheus-on-thin
branch
from
f37fd5f to
630b624
Compare
philippthun
reviewed
Nov 13, 2023
philippthun
approved these changes
Nov 20, 2023
rroberts2222
pushed a commit
to loggregator/cloud_controller_ng
that referenced
this pull request
Dec 19, 2023
- Remove deprecated metrics and metrics which have been found not useful according to discussions in the community - Make use of the DependencyLocator for retrieving a singleton of the PrometheusUpdater and PeriodicUpdater - Change vitals_uptime to vitals_started_at - Emit cc_staging_requests_total metric - Apply prometheus best practices like naming, base units, using labels, initialising metrics for discoverability - Use counter metrics for metrics which do not decrease - Remove metrics, which are emitted on the scheduler VM. Those metrics currently cannot be collected and will be still emitted via statsd Co-authored-by: Andrew Crump <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A short explanation of the proposed change:
Adjust Prometheus endpoint:
This implementation is meant for experimental usage. It is difficult to decide on the best metric type or bucket size initially, without having it used in productive environments. The metrics emitted via Prometheus can be collected and displayed in dashboards while still using the statsd metrics for alert/real monitoring. We should communicate that breaking changes are likely, as long as this is treated as a experimental feature.
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests