bookingcom / carbonapi

High-performance Graphite frontend in Go
Other
81 stars 23 forks source link

Support noNullPoints flag #221

Open dorroddorrod opened 4 years ago

dorroddorrod commented 4 years ago

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

grzkv commented 4 years ago

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

dorroddorrod commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

azhiltsov commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@grzkv there is no support for it in grafana https://github.com/grafana/grafana/issues/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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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.