Comments (13)
In my opinion if the operation makes sense and something is returned from that operation but we have ignored histogram in the vector ( like 2/foo ) then we can add an info level annotation stating that histograms are ignored in the operation and if the operation does not makes any sense (like float+histogram ) then I think we should add a warn level annotation that this operation for the given arguments is not possible (or something like that). These are my thoughts pls correct me.
That's also an option. If I understand you correctly, you would add an info annotation to the 2 / foo
expression if foo
contains both histograms and floats (so something is returned) but if foo
is a pure histogram vector, you would add a warning annotation (because nothing is returned). That might be hard to implement (you can only decide about the level of the annotation when looking at the whole vector), but maybe it's doable. I'm a bit more concerned about the change of behavior that a "float divided by histogram" operation becomes "more bad" if the histogram is not accompanied by a float in the same vector.
Another thought in this regard: A mix of floats and histograms in the same vector should be a very rare case. So maybe we don't need to think too much about treating that case specially or with concerns about spammy annotations. It shouldn't really happen in practice, and if it does, it's probably a faulty setup or a transition period and will be fixed soon. So maybe let's say for now, whenever we ignore a histogram in an operation because it doesn't make sense, we add an info-level annotation saying that an operation wasn't performed, even if the expression still yields a result from other vector elements. We can do this first, it's easy, and then iterate on it once we see how it plays out in practice.
from prometheus.
Related: #11973
from prometheus.
My thoughts:
I think the (float, histogram) args for arithmetic binary operators needs to be broken up. For example, float + histogram
or histogram - float
doesn't work, but float * histogram
and histogram / float
is just fine (but float / histogram
is not). histogram + histogram
is fine, but histogram * histogram
is not.
For min
and max
, I think it's fine to just return nothing for aggregations of only histograms. The warning should only be issued if the aggregation tries to aggregate histograms and floats into the same key. (For example, if foo
is a vector that has both floats and histograms, and all the histograms have a kind="histogram"
label and all the floats have a kind="float"
label, then min by (kind)(foo)
should not give a warning (but the only element in the result vector has kind="float"
), but min(foo)
should give a warning.)
I think the current behavior of count
is fine. count
just counts sample, and a float and a histogram should be counted the same. (I think that's even documented.)
histogram / 0
should return sum
and count
and the zero bucket "normally" (I assume in the way it is done already), but then all (normal) buckets should be removed (because all the non-represented empty buckets should be NaN
now, which we cannot represent. So better have no buckets at all. (This case is anyway pathologic.)
from prometheus.
The basic idea is this: Where operations and aggregations make sense (between histogram and histogram, or between histogram and float), they happen. Where they don't make sense, the histogram is treated as if the vector element with the histogram did not exist. This is not an error, but we attach a "warn"-level annotation (or maybe "info"-level only?) if any vector element is label-matched with an ignored histogram. That implies no annotation is added if two ignored histograms are matched or if an ignored histogram is matched with a scalar (which is not label-matching).
The rationale here is that label matching might be complex, and if a label match leads to an impossible combination, it might be accidental and the user should know about it. In contrast to that, 2 / foo
essentially means "Divide 2 by all the float elements and ignore the histogram elements" and that's just fine and can deliberately be used for filtering. However, if I write foo + bar
, the user's idea is probably that the label-matched elements should also have the same type, and if a float gets matched with a histogram, it's probably an error. (And the "probably" makes me think we should perhaps use "info" level, which implies that false positives are possible, while "warn" level is meant for annotations that are always triggered by something that is "wrong" in some way and could be fixed somehow.)
(This is all just my stream of thoughts. Feel free to express other viewpoints.)
from prometheus.
I think the (float, histogram) args for arithmetic binary operators needs to be broken up. For example, float + histogram or histogram - float doesn't work, but float * histogram and histogram / float is just fine (but float / histogram is not). histogram + histogram is fine, but histogram * histogram is not.
Yes, sorry, I missed this case. I'll break the operators further with different arguments and update the table.
For min and max, I think it's fine to just return nothing for aggregations of only histograms. The warning should only be issued if the aggregation tries to aggregate histograms and floats into the same key. (For example, if foo is a vector that has both floats and histograms, and all the histograms have a kind="histogram" label and all the floats have a kind="float" label, then min by (kind)(foo) should not give a warning (but the only element in the result vector has kind="float"), but min(foo) should give a warning.)
So the gist here is that after grouping the labels if the vector contains both histogram
and float
then we'll issue a warning otherwise we will return the min
in case of float
only vector and nothing in case of histogram
only vector. Please correct me if I misunderstood something.
I think the current behavior of count is fine. count just counts sample, and a float and a histogram should be counted the same. (I think that's even documented.)
yeah, my bad, I somehow got confused between count
and histogram_count
.
I'll make all these changes into the table.
from prometheus.
The rationale here is that label matching might be complex, and if a label match leads to an impossible combination, it might be accidental and the user should know about it. In contrast to that, 2 / foo essentially means "Divide 2 by all the float elements and ignore the histogram elements" and that's just fine and can deliberately be used for filtering. However, if I write foo + bar, the user's idea is probably that the label-matched elements should also have the same type, and if a float gets matched with a histogram, it's probably an error. (And the "probably" makes me think we should perhaps use "info" level, which implies that false positives are possible, while "warn" level is meant for annotations that are always triggered by something that is "wrong" in some way and could be fixed somehow.)
In my opinion if the operation makes sense and something is returned from that operation but we have ignored histogram in the vector ( like 2/foo
) then we can add an info level annotation stating that histograms are ignored in the operation and if the operation does not makes any sense (like float
+histogram
) then I think we should add a warn level annotation that this operation for the given arguments is not possible (or something like that). These are my thoughts pls correct me.
from prometheus.
So the gist here is that after grouping the labels if the vector contains both histogram and float then we'll issue a warning otherwise we will return the min in case of float only vector and nothing in case of histogram only vector.
Yes, that's the idea (but the case of how to deal with histogram-only could still be debated, see your later comment).
from prometheus.
I have updated the table above according to my last paragraph above.
from prometheus.
Thanks for updating the table Beorn. I was busy and couldn't update the table fully.
from prometheus.
@NeerajGartia21 I have updated the table once more with things I have stumbled upon. Maybe we have covered everything now. I have closed #11973 because this describes the missing pieces much better. You were assigned to #11973. Are you planning to work on this any time soon? I can assign you to this issue in that case. If you are not available any time soon, it would be good to know so that others can work on this.
from prometheus.
Thanks, @beorn7, for updating the table. In the last few months, I wasn't feeling well and was diagnosed with an autoimmune disease. However, I am now fine and can continue working on this.
from prometheus.
All the best for your further recovery. I'll assign this to you then, but no pressure! If you feel overwhelmed, just let me know.
from prometheus.
Thanks @beorn7 for re-assigning it to me. I'll let you know if I face any problem!
from prometheus.
Related Issues (20)
- [Error] Setup prometheus to scrap from ms instances registered in Eureka SD with random port HOT 5
- How to deal with breakpoint in Prometheus data graph HOT 1
- After adapting Prometheus to IPv6, the file auto-discovery feature becomes invalid. HOT 4
- Crash "panic: runtime error: invalid memory address or nil pointer dereference" HOT 14
- native histograms: Implement the < and > operators to "trim" a histogram. HOT 20
- Prometheus scrape config does not support recognizing IPv6 addresses. HOT 1
- CVE of prometheus/prometheus:v2.54.0 HOT 1
- Flaky TestQuerierShouldNotFailIfOOOCompactionOccursAfterRetrievingQuerier HOT 3
- scrape (histograms): Investigate (and address) protobuf scraping performance problems HOT 1
- Annotations for incompatible native histogram operations with binary operators have confusing message HOT 7
- Received an OS signal, exiting gracefully..." signal=terminated HOT 1
- chore(v3): set UTF-8 default-on HOT 5
- Prometheus remote write fail, 'Failed to send batch,retrying'
- Unable to set route on prometheus container HOT 2
- fsnotify: queue or buffer overflow HOT 1
- How to get Alert of Prometheus metrics in OpenSearch Dashboard? HOT 1
- Prometheus targets show 1/0 for all targets HOT 3
- Improvement of OpenSSF Scorecard Score
- Test certificates expiring in <1y HOT 1
- Inconsistent `vector cannot contain metrics with the same labelset` errors for functions over range vectors
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from prometheus.