Skip to content
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

Prometheus client should not be used to aggregate metrics points #370

Closed
paivagustavo opened this issue Dec 4, 2019 · 0 comments · Fixed by #385
Closed

Prometheus client should not be used to aggregate metrics points #370

paivagustavo opened this issue Dec 4, 2019 · 0 comments · Fixed by #385
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics
Milestone

Comments

@paivagustavo
Copy link
Member

With the bootstrap implementation of the prometheus exporter (#334), we have been using the prometheus client lib to aggregate and export metrics to the Prometheus format. To make that works, we needed to keep every recorded metric point on the sdk (i.e., not aggregate at all), so prometheus could aggregate itself, this is could be expensive.

We could start using the Prometheus client to only export data and let the metrics SDK do all the necessary aggregation work but there is some work that need to be done before. This would optimize the metrics SDK that would not need to keep track of all the metric points to calculate a histogram, for example.

From what I've understood of the OpenCensus Prometheus Exporter, we would need to implement the prometheus.Collector to send const metric points to the prometheus client that would then export these via the promhttp.Handler.

To achieve something similar to OpenCensus we would need:

  1. Implement the histogram aggregator (Implement Histogram aggregator #317);
  2. Implement a pull based controller;
    the usage of this controller would be close to: promhttp.Handler -> Otel Prometheus Exporter -> PullController.Tick()
  3. Change the Prometheus exporter to only create these const metrics points and implement the prometheus.Collector interface.

I'm not sure if we should make a PullControler or make a cache of the last CheckpointSet of the existing PushController, since calling PullController.Tick() could be expensive?

This has been discussed on the Otel Metrics office hour and I'm opening this issue to track this suggestion. More investigation need to be done to make sure that we are not missing anything here.

I'll do a PoC for this to check how viable this proposal is and to find out what we would need to do first.

@paivagustavo paivagustavo added exporters area:metrics Part of OpenTelemetry Metrics labels Dec 4, 2019
@paivagustavo paivagustavo self-assigned this Dec 4, 2019
hstan referenced this issue in hstan/opentelemetry-go Oct 15, 2020
…aws (#370)

* Bump github.com/aws/aws-sdk-go from 1.34.30 to 1.34.32 in /detectors/aws

Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.34.30 to 1.34.32.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Changelog](https://github.com/aws/aws-sdk-go/blob/master/CHANGELOG.md)
- [Commits](aws/aws-sdk-go@v1.34.30...v1.34.32)

Signed-off-by: dependabot[bot] <[email protected]>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants