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

support carbonapi #328

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

support carbonapi #328

wants to merge 1 commit into from

Conversation

tbauriedel
Copy link
Member

This PR not only adds support for carbonapi, it supports graphite-api which implements API version 0.9.8 and carbonapi which implements the necessary endpoints of API version 1.11.7 (and not metrics/expand).

Endpoint Documentation:

I have tested it with the latest available graphite-api and carbonapi v0.16.0

Possible solution for #300

Feedback welcome!

CC @dgoetz

@widhalmt
Copy link
Member

Tested it in my environment as well. Works flawlessly so far. THANK YOU!

@citfs
Copy link

citfs commented Aug 30, 2024

It also works perfectly in our environment. Thank you very much.

@nilmerg
Copy link
Member

nilmerg commented Sep 2, 2024

Just to be sure, this keeps 100% compatibility with how it worked before?

@tbauriedel
Copy link
Member Author

tbauriedel commented Sep 2, 2024

We use another API endpoint now. The return of the API is a little bit different, which is why the data is stored slightly differently into the results.

Based on the API endpoint documentations that will work with all possible components.
As mentioned: It is only tested on graphite-api and carbonapi. I had no graphite-web to test it there also. Based on the documentation it should work there also.

Besides of that it works how it worked before.

Long story short:
It was possible for me and others to switch from graphite-api to carbonapoi without changing anything on the module.
Drop-in replacement.

@tbauriedel
Copy link
Member Author

While working on another topic for the module I have found a difference that will break the current behaviour.

The changes merge graphs with placeholders into one * and only one graph is rendered.
(Also in the graphite-api)
I'll take another look at the whole thing in detail and try to adapt it accordingly.

Please wait for that before merging!

Before:
Before
After:
After

@tbauriedel tbauriedel marked this pull request as draft October 24, 2024 05:53
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.

4 participants