feature: type-and-unit-labels (PROM-39 implementation) #16228
Conversation
4858450 to
e50a616
Compare
d352464 to
0a274f5
Compare
PR with the implementation is also available: prometheus/prometheus#16228 Signed-off-by: bwplotka <bwplotka@gmail.com>
0a274f5 to
87f94b0
Compare
7417d2c to
69ccac6
Compare
There was a problem hiding this comment.
Not in this PR, but in the same feature flag we should:
- Update UI as proposed in the proposal
Just to clarify what you mean here. Do you want this PR to hide type and unit labels, and the following PR will have that checkbox to show them? Or do you want to leave hiding type and unit to the following PR as well?
I'm asking because while testing, I noticed that those labels are present during query-time
|
Will adress all comments after KubeCon! Thanks for reviewing! |
69ccac6 to
d97435b
Compare
|
Thanks for the epic reviews @ArthurSens @ywwg ! I am progressing on the rebase and update, currently cannot decide on naming of that internal group of metadata/metric family/metric descriptor/metric desc, will play around those... 🙈
As in our updated proposal I propose to not change UI for now, so we can talk about this later. Personally I think UI option for hiding those is nice, but majority of ecosystem will have those labels "exposed" anyway, so it's not a helping a lot, thus not a blocker/game changer. |
|
Thanks for helping @ArthurSens -- what's next for this? I believe we need to add some test cases for proto and OM parsing and what we expect from the storage (if we don't have this already). Then also that PromQL engine warning I ended up with "MetricDescriptor" for the internal structure -- might be good enough. |
Sounds good, I want to find time to work on this during this week or during the next one at max
I was talking with @carrieedwards, and she volunteered to tackle this part since PromQL Engine is completely alien to me 😭. She is busy for a while though; she is presenting at GrafanaCon this week and will be at an offsite in the following week. Could you confirm, Carrie?
Looks good to me :) |
Yes, I will take a look at this next week or the week after! |
|
Cool, amazing! I will try to progress in the meantime. I decided to refactor the weird "MetricDescriptor" into schema.Metadata -- might be more clear. I will add test cases and I propose we tackle PromQL engine warning on mixed units/types in separate PR (: |
009c8d9 to
73100ba
Compare
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
7cad4e1 to
5e91463
Compare
d6d166b to
a86b297
Compare
Signed-off-by: bwplotka <bwplotka@gmail.com>
* Update mimir-prometheus * Replace references to `InstanceName` removed in prometheus/prometheus#16228 * Fix native histogram mangling behaviour to be compatible with prometheus/prometheus#16565 * Make build tags consistent with Prometheus * Update MQE tests to match upstream * Reinstate test cases that were skipped due to upstream issues * Bring in changes from grafana/mimir-prometheus#880 * Add changelog entries * Regenerate OTLP files * Fix breaking change * Update upstream tests
PR with the implementation is also available: prometheus/prometheus#16228 Signed-off-by: bwplotka <bwplotka@gmail.com>
Implementation of prometheus/proposals#39
Previous (unmerged) experiments:
TODO:
Not in this PR, but in the same feature flag we should: