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 noNullPoints flag #221

Open
dorroddorrod opened this issue Dec 8, 2019 · 10 comments
Open

Support noNullPoints flag #221

dorroddorrod opened this issue Dec 8, 2019 · 10 comments
Labels
good first issue Good for newcomers

Comments

@dorroddorrod
Copy link

Hi,
Can you please add support of noNullPoints flag ?
This is really useful feature to avoid retrieving null data

@grzkv
Copy link
Member

grzkv commented Dec 9, 2019

Hi, have you tried trasformNull and removeEmtpySeries? These may do what you want.

@dorroddorrod
Copy link
Author

Hi @grzkv ,
Thanks for your reply,
NoNullPionts is a query param in the url and not a function on the metric itself .
I would like to use it as a flag without editing the target metric

@grzkv
Copy link
Member

grzkv commented Dec 9, 2019

Yes, I understand that it's a query param. You don't have to edit the metric with the above solution, just the functions on top of it.

@dorroddorrod
Copy link
Author

I am aware of it but again, don’t want to wrap my metric with additional function

@azhiltsov
Copy link
Contributor

We are currently using carbonapi with go-carbon and it does not have support for noNullPoints.
@dorroddorrod
With what back-ends are you using the carbonapi?
Could you please explain your use-case a bit in details?
@grzkv the reference implementations is here: https://github.com/graphite-project/graphite-web/pull/2257/files

@dorroddorrod
Copy link
Author

Sure,
I'm using carbonapi with go-carbon + graphiteweb for debugging.
When i use graphiteweb to query with noNullPoints=true, i'm getting back datapoint without any null values but it looks like carbonapi does not support this flag.
A bit about my use-case :
We have an alerting system that is based on graphite metrics, i want to avoid getting back null datapoint to decrease the amount of traffic from carbonapi to the alerting system.

@azhiltsov
Copy link
Contributor

azhiltsov commented Dec 9, 2019

@grzkv there is no support for it in grafana grafana/grafana#11171 currently,
so implementing it won't make any benefits for us from Front-end point of view.
To make a decision we need to understand how effectively we are packing nulls in protobufs and how much of memory in zipper and api and bandwidth on storages we can gain if we would strip nulls away.
Additionally to that. There could be edge cases while dealing with metrics with only one value surrounded by nulls (not sure that there is resolution metadata in protobuf v2 to reconstruct such metric on zipper)
Over all, I would say it not a high prio and most probably high effort.

UPD: seems there is enough data to reconstruct, but from my understanding we wont be able to implement noNullPoints without changing the protocol https://github.com/go-graphite/protocol/blob/master/carbonapi_v2_pb/carbonapi_v2_pb.pb.go#L30 am I right?

@Civil
Copy link
Contributor

Civil commented Dec 9, 2019

Protobuf doesn't have any special compression for doubles, so for each value you won't pass on a wire you'll save at least 65 bit of memory (Value + IsAbsent). On a wire it will be less if you still use compression.

Also without changing the protocol you indeed won't be able to implement noNullPoints on a wire, except for removing them from the start and end of the response. Both v2 and v3 doesn't have that feature, as it would require to pass the timestamp for each datapoint as well, instead of just start time and stop time. And the assumption for both versions of the storage protocols is that usually you mostly have non-Null data in the responses.

@Civil
Copy link
Contributor

Civil commented Dec 10, 2019

Also, protocol level support can be added without introducing backward incompatibility. If you add a new field for timestamps to current protocol and add a flag for the request type, on application level you can treat it differently without a problem and protocols would still be wire-compatible.

@grzkv
Copy link
Member

grzkv commented Dec 10, 2019

@dorroddorrod You can do the implementation in carbonapi if needed, on the front-end. Just by removing the null points in the HTTP handler for render. This will not be efficient but will achieve what you need. It will decrease the amount of traffic between carbonapi and the alerting system.

@grzkv grzkv added the good first issue Good for newcomers label Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants