feat(om2): classic histogram and summary as complex types #2679
Conversation
krajorama
changed the title
krajorama
force-pushed
the
krajo/om2.0-complex-hist-summ
branch
from
aca6583 to
4c7a987
Compare
bwplotka
left a comment
bwplotka
left a comment
There was a problem hiding this comment.
I would love that, but as we discussed on our sync - we need a bigger decision if we want to follow this path.
If we do, we likely need some rough uncomplex inlined format explained that is not meant to be used for scraping and can be surfaced with promtool etc.
krajorama
dismissed stale reviews from bwplotka and dashpole
this is a draft, changes incoming, also I made a mistake with the quantile values
This change is for demo purposes, exploring the benefits (and downsides) for the complex type format for OM2 captured in prometheus/docs#2679. This assumes Prometheus stores NS and NHCB (and NH) going forward (for best case efficiency), but is expected to work for classic mode too with little overhead (benchmarks will tell us). Part of the PromCon talk we do with @krajorama Signed-off-by: bwplotka <bwplotka@gmail.com>
This change is for demo purposes, exploring the benefits (and downsides) for the complex type format for OM2 captured in prometheus/docs#2679. This assumes Prometheus stores NS and NHCB (and NH) going forward (for best case efficiency), but is expected to work for classic mode too with little overhead (benchmarks will tell us). Part of the PromCon talk we do with @krajorama Signed-off-by: bwplotka <bwplotka@gmail.com>
This change is for demo purposes, exploring the benefits (and downsides) for the complex type format for OM2 captured in prometheus/docs#2679. This assumes Prometheus stores NS and NHCB (and NH) going forward (for best case efficiency), but is expected to work for classic mode too with little overhead (benchmarks will tell us). Part of the PromCon talk we do with @krajorama Signed-off-by: bwplotka <bwplotka@gmail.com>
|
Results are (expectedly) impressive: prometheus/prometheus#17293 One observation is that buckets are a bit dense (readability). However we don't have spaces in any other place (e.g. labels) |
This change is for demo purposes, exploring the benefits (and downsides) for the complex type format for OM2 captured in prometheus/docs#2679. This assumes Prometheus stores NS and NHCB (and NH) going forward (for best case efficiency), but is expected to work for classic mode too with little overhead (benchmarks will tell us). Part of the PromCon talk we do with @krajorama Signed-off-by: bwplotka <bwplotka@gmail.com>
krajorama
force-pushed
the
krajo/om2.0-complex-hist-summ
branch
from
8696022 to
8c8eeae
Compare
krajorama
force-pushed
the
krajo/om2.0-complex-hist-summ
branch
7 times, most recently
from
10e9bef to
181db08
Compare
|
This looks good to me overall. I think we can save float histograms for a follow-up. |
| foo{entity="replica",foo="a"} 1.0 | ||
| foo{entity="replica",foo="bb"} 0.0 | ||
| foo{entity="replica",foo="ccc"} 1.0 | ||
| foo{entity="controller",foo="a"} 1.0 |
There was a problem hiding this comment.
Maybe you meant
| foo{entity="controller",foo="a"} 1.0 | |
| foo{entity="controller",foo="ddd"} 1.0 |
Otherwise it's a duplicate?
There was a problem hiding this comment.
I've change it in a different way, by including another label.
Ref: prometheus/OpenMetrics#283 Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
krajorama
force-pushed
the
krajo/om2.0-complex-hist-summ
branch
from
181db08 to
578ba94
Compare
| foo_bucket{le="+Inf"} 17 st@1520430000.123 | ||
| foo_count 17 st@1520430000.123 | ||
| foo_sum 324789.3 st@1520430000.123 | ||
| foo {count:17,sum:324789.3,bucket:[0.0:0,1e-05:0,0.0001:5,0.1:8,1.0:10,10.0:11,100000.0:11,1e+06:15,1e+23:16,1.1e+23:17,+Inf:17]} st@1520430000.123 |
There was a problem hiding this comment.
Not for this PR, but one last thing to check is the readability/mode or accepting whitespaces (or at least [ \n\t].*) inside { }. We might want to compare pros and cons, but eating all those chars when in { is not too difficult (it might be prone to errors though).
This format asks for it (:
There was a problem hiding this comment.
foo {
count: 17,
sum: 324789.3,
bucket: [
0.0: 0,
1e05: 0,
0.0001: 5,
....
],
} st@1520430000.123
Pros:
- Looks nicer
Cons: - Not very practical (grep)
Header or tool (CLI)
I like the CLI idea
There was a problem hiding this comment.
Idea from @krajorama allow whitespaces, but SHOULD without?
Consensus among me and @krajorama : Keep it simple and deterministic and do not allow whitespaces for now, have a CLI tool for it.
There was a problem hiding this comment.
From WG: Machine readability = human readability is a nice aspect (reliability, confusion).
| foo_bucket{le="+Inf"} 17 st@1520430000.123 | ||
| foo_count 17 st@1520430000.123 | ||
| foo_sum 324789.3 st@1520430000.123 | ||
| foo {count:17,sum:324789.3,bucket:[0.0:0,1e-05:0,0.0001:5,0.1:8,1.0:10,10.0:11,100000.0:11,1e+06:15,1e+23:16,1.1e+23:17,+Inf:17]} st@1520430000.123 |
There was a problem hiding this comment.
Idea from @krajorama allow whitespaces, but SHOULD without?
Consensus among me and @krajorama : Keep it simple and deterministic and do not allow whitespaces for now, have a CLI tool for it.
Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com> Signed-off-by: George Krajcsovits <krajorama@users.noreply.github.com>
Ref: prometheus/OpenMetrics#283
It isn't hard to come up with a representation, based on previous work in https://github.com/prometheus/proposals/blob/main/proposals/2024-01-29_native_histograms_text_format.md .