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

[WIP] Prometheus remote read support #2195

Closed
wants to merge 4 commits into from

Conversation

DanCech
Copy link
Member

@DanCech DanCech commented Jan 10, 2018

This PR adds a new endpoint (currently /render/prometheus but could move to /prometheus/read or similar) that implements the Prometheus remote read API, allowing Prometheus to query data out of Graphite.

It accepts remote read requests (as snappy-compressed protobuf objects), converts them into native seriesByTag requests and executes the queries, then encodes and returns the results to Prometheus.

In order to use this you need to pip install python-snappy protobuf (for that to work you'll likely need to sudo apt install libsnappy-dev first), I didn't add them to requirements.txt due to the non-trivial installation for python-snappy.

This is still a work in progress, I intend to add tests and docs, and I'm working on a matching patch for carbon to add native remote write support. Right now the simplest way to get Prometheus metrics into Graphite is to use prometheus/prometheus#3533 but you can also use this PR to read regular Graphite data from within Prometheus.

@DanCech DanCech self-assigned this Jan 10, 2018
@deniszh
Copy link
Member

deniszh commented Jan 10, 2018

Cool, nice feature!

@iksaif
Copy link
Member

iksaif commented Jan 12, 2018

Note that we plan to add tags support to https://github.com/criteo/graphite-remote-adapter very soon, at least for writes. That would work nicely with this.

Our graphite-remote-adapter should hopefully be more up to date than the one in prometheus/documentation/example. I'll try to write more doc for it and advertise it a little bit more.

@DanCech
Copy link
Member Author

DanCech commented Jan 12, 2018

Once I add native support in carbon we won't need an adapter at all for the standard use-case, though looking at graphite-remote-adapter it seems like it would be useful for cases where more control over the final series names etc are needed.

@iksaif
Copy link
Member

iksaif commented Jan 12, 2018

Exactly, also when you have multiple Prometheus but a single Graphite cluster you want to be able to control the prefix (as a way to namespace the metrics and not pollute every prometheus instance)

@deniszh
Copy link
Member

deniszh commented Jan 12, 2018

More is better in this case. 👍

@codecov-io
Copy link

codecov-io commented Jan 14, 2018

Codecov Report

Merging #2195 into master will decrease coverage by 3.56%.
The diff coverage is 7.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2195      +/-   ##
==========================================
- Coverage   79.63%   76.07%   -3.57%     
==========================================
  Files          85       90       +5     
  Lines        8862     9315     +453     
  Branches     1899     1916      +17     
==========================================
+ Hits         7057     7086      +29     
- Misses       1542     1965     +423     
- Partials      263      264       +1
Impacted Files Coverage Δ
webapp/graphite/urls.py 80% <ø> (ø) ⬆️
webapp/graphite/prometheus/gogo_pb2.py 0% <0%> (ø)
webapp/graphite/prometheus/types_pb2.py 0% <0%> (ø)
webapp/graphite/prometheus/urls.py 100% <100%> (ø)
webapp/graphite/prometheus/views.py 24% <24%> (ø)
webapp/graphite/prometheus/remote_pb2.py 5.26% <5.26%> (ø)
webapp/graphite/render/views.py 71.02% <77.77%> (+0.13%) ⬆️
... and 3 more

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 d483c17...658a6df. Read the comment docs.

iksaif pushed a commit to iksaif/graphite-remote-adapter that referenced this pull request Jan 15, 2018
Write:
- Support tags, in carbon and openmetrics format
- TODO: Support for upcomming Carbon native Prometheus support: graphite-project/carbon#735

Read:
- Support tags (TODO: add tests)
- TODO: Support for upcomming Graphite native Prometheus support: graphite-project/graphite-web#2195
iksaif pushed a commit to iksaif/graphite-remote-adapter that referenced this pull request Jan 15, 2018
Write:
- Support tags, in carbon and openmetrics format
- TODO: Support for upcomming Carbon native Prometheus support: graphite-project/carbon#735

Read:
- Support tags (TODO: add tests)
- TODO: Support for upcomming Graphite native Prometheus support: graphite-project/graphite-web#2195

Some of this code comes from https://github.com/prometheus/prometheus/pull/3533/files
iksaif pushed a commit to iksaif/graphite-remote-adapter that referenced this pull request Jan 15, 2018
Write:
- Support tags, in carbon and openmetrics format
- TODO: Support for upcomming Carbon native Prometheus support: graphite-project/carbon#735

Read:
- Support tags (TODO: add tests)
- TODO: Support for upcomming Graphite native Prometheus support: graphite-project/graphite-web#2195

Some of this code comes from https://github.com/prometheus/prometheus/pull/3533/files
iksaif pushed a commit to iksaif/graphite-remote-adapter that referenced this pull request Jan 17, 2018
Write:
- Support tags, in carbon and openmetrics format

Read:
- Support tags

TODO in a later patch:
- Support Graphite native Prometheus support: graphite-project/graphite-web#2195
- Support for upcomming Carbon native Prometheus support: graphite-project/carbon#735
- For both of these, just make sure the prefix is handled correctly.

Some of this code comes from https://github.com/prometheus/prometheus/pull/3533/files
iksaif pushed a commit to iksaif/graphite-remote-adapter that referenced this pull request Jan 17, 2018
Write:
- Support tags, in carbon and openmetrics format

Read:
- Support tags

TODO in a later patch:
- Support Graphite native Prometheus support: graphite-project/graphite-web#2195
- Support for upcomming Carbon native Prometheus support: graphite-project/carbon#735
- For both of these, just make sure the prefix is handled correctly.

Some of this code comes from https://github.com/prometheus/prometheus/pull/3533/files
iksaif pushed a commit to iksaif/graphite-remote-adapter that referenced this pull request Jan 18, 2018
Write:
- Support tags, in carbon and openmetrics format

Read:
- Support tags

TODO in a later patch:
- Support Graphite native Prometheus support: graphite-project/graphite-web#2195
- Support for upcomming Carbon native Prometheus support: graphite-project/carbon#735
- For both of these, just make sure the prefix is handled correctly.

Some of this code comes from https://github.com/prometheus/prometheus/pull/3533/files
@stale
Copy link

stale bot commented Apr 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 13, 2020
@stale stale bot closed this Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants