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

Add batched exports to the Prometheus Remote Write Exporter #2249

Merged

Conversation

JasonXZLiu
Copy link
Member

@JasonXZLiu JasonXZLiu commented Dec 3, 2020

Description:

Currently, there is no limit on the size of the remote_write export request to a Cortex instance. This could create problems if one Prometheus scrape produces an extreme amount of timeseries metrics.

This PR creates a batch of concurrent requests where the max size of a request is 3MB and the max number of timeseries per request is 40,000.

A future PR will tackle making the max size of each request configurable.

Testing:

  • Unit tests were fixed
  • Performance tests were done with 15000 Prometheus metrics (around 100k timeseries) per scrape. The first and last metric of each Prometheus metric type were all found.

@JasonXZLiu JasonXZLiu requested a review from a team December 3, 2020 23:33
@codecov
Copy link

codecov bot commented Dec 3, 2020

Codecov Report

Merging #2249 (273fa79) into master (511cdaa) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2249      +/-   ##
==========================================
+ Coverage   91.98%   92.01%   +0.03%     
==========================================
  Files         271      271              
  Lines       15664    15681      +17     
==========================================
+ Hits        14408    14429      +21     
+ Misses        854      851       -3     
+ Partials      402      401       -1     
Impacted Files Coverage Δ
exporter/prometheusremotewriteexporter/exporter.go 93.95% <100.00%> (+0.82%) ⬆️
exporter/prometheusremotewriteexporter/helper.go 99.55% <100.00%> (+0.02%) ⬆️
internal/data/traceid.go 84.61% <0.00%> (-7.70%) ⬇️
exporter/otlpexporter/otlp.go 74.35% <0.00%> (+2.56%) ⬆️
exporter/exporterhelper/metricshelper.go 100.00% <0.00%> (+5.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 511cdaa...273fa79. Read the comment docs.

@tigrannajaryan
Copy link
Member

@huyan0 will you be able to review this as the original author of the exporter?

@JasonXZLiu
Copy link
Member Author

@amanbrar1999 is actually the current maintainer from AWS for the Prometheus Remote Write Exporter. Would also be great if we could get a review from @jmacd. Thank you!

cc - @alolita

Copy link
Contributor

@amanbrar1999 amanbrar1999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tigrannajaryan tigrannajaryan merged commit e1a6806 into open-telemetry:master Dec 11, 2020
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
* Add sfx_skip_repo option to ansible

* rename sfx_skip_repo to splunk_skip_repo, add the when clause to debian installs as well, add more documentation

Co-authored-by: asavenko <asavenko@redhat.com>
Co-authored-by: Antoine Toulme <atoulme@splunk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants