Skip to content

ui: heatmap visualization for histogram buckets#12995

Closed
Loori-R wants to merge 26 commits intoprometheus:mainfrom
Loori-R:flotjs-support-heatmap
Closed

ui: heatmap visualization for histogram buckets#12995
Loori-R wants to merge 26 commits intoprometheus:mainfrom
Loori-R:flotjs-support-heatmap

Conversation

@Loori-R
Copy link
Copy Markdown
Contributor

@Loori-R Loori-R commented Oct 17, 2023 •
edited
Loading

Description:

This Pull Request implements a heatmap visualization feature for histogram buckets on the Prometheus /graph page. It utilizes a flotjs plugin for rendering, aligning its functionality with that observed in the VictoriaMetrics UI (vmui), as discussed in issue: VictoriaMetrics#3384.

Key Changes:

  • flotjs Plugin Integration: Facilitates the rendering of heatmaps.
  • "Show Histogram Graph" Button Added: A new button placed adjacent to the "Show Stacked Graph" button.
  • Histogram Rendering: Clicking the "Show Histogram Graph" button renders the graph as a histogram.
  • Auto-switch to Line Graph: The graph automatically transitions to a linear representation if the query data lacks bucket data.
  • Button Activation Logic: The "Show Histogram Graph" button is enabled only when the data comprises a single histogram series.
  • Deactivation Logic: The button is disabled if the data includes more or fewer than one histogram series.

Future Enhancements:

  • Renderer Selection Option: Consider substituting the button with a selector to choose the histogram rendering type when multiple series are available.
Screenshot 2023-10-30 at 11 36 22

The PR has been moved to #13096

@Loori-R Loori-R requested a review from juliusv as a code owner October 17, 2023 15:29
@beorn7
Copy link
Copy Markdown
Member

beorn7 commented Oct 18, 2023

Generally, I like this. @juliusv has to look at the actual code as I'm mostly React-illiterate.

Here just high level thoughts, comments, and questions:

  • Of course, we want this even more for native histograms (where heatmap make way more sense), but that's certainly not in scope of this PR. Cf. ui (histograms): Render Native Histograms in Graph view #11268.
  • What happens if the query yields multiple different histogram series? Maybe there is already something in the code. Here are my high level ideas:
    • Most straightforward: Render a separate heatmap for each, arrange them in a grid next to each other (as "square" as possible, e.g. if there are 10 series, it should probably be a 3x4 grid).
    • Maybe even more straightforward: Select the histogram to be displayed from some selection element (list, dropdown).
    • Semi fancy: Cycle through histograms over time (with some controls for speed, pausing, …).
    • Very fancy: Use a different color palette for each histogram and overlay them with each other with some transparency. That would probably only scale up to a handful of histograms (could be combined with the selection element above). I imagine it to look like astro photographs where different parts of the spectrum are mapped into different colors.
  • As boring the old rendering of a classic histogram is, it should probably be possible to switch back to that mode of display.
  • The sorting of buckets in the screenshot seems off. Maybe it's done lexicographically rather than numerically? The +Inf bucket is right at the bottom, but even the normal numbers are mixed up.
  • There should also be a graphical representation in table view (also out of scope of this PR, just saying).
  • Is the code correctly "de-cumulating" the histogram? The screenshot looks like it doesn't.

@Loori-R
Copy link
Copy Markdown
Contributor Author

Loori-R commented Oct 18, 2023

@beorn7, Thank you for your insights and suggestions.

  • At present, the histogram displays only for one series. If there are multiple series, a line graph will be shown.
  • Regarding the issues with sorting and "de-cumulating": I will investigate this and make the necessary corrections.

@beorn7
Copy link
Copy Markdown
Member

beorn7 commented Oct 24, 2023

At present, the histogram displays only for one series. If there are multiple series, a line graph will be shown.

OK, that's certainly good enough for a first iteration. Maybe, to avoid confusions, you could show the line graph by default in any case, and then offer a button "render as heatmap" in case there is only one histogram (or, to get one step closer to treating multiple histograms properly, offer a button for each histogram shown).

@Loori-R
Copy link
Copy Markdown
Contributor Author

Loori-R commented Oct 30, 2023

@beorn7, I've updated the PR to address your comments, making the following changes:

  • Bucket Sorting: Now correctly numerical, including proper placement of the +Inf bucket.
  • Histogram De-cumulation: Ensured accurate rendering of the histogram's distribution of values.
  • Rendering Options: Line graph is set as default, with a "Show histogram graph" button for single histograms.

The PR description and screenshots have also been updated to reflect these changes.

@beorn7
Copy link
Copy Markdown
Member

beorn7 commented Nov 2, 2023

Thank you very much. I'll have a detailed look ASAP (and I'll need @juliusv anyway for the React parts). In the meantime, could you do the DCO signing to make the CI pass?

@juliusv
Copy link
Copy Markdown
Member

juliusv commented Nov 3, 2023

The UI part is not supposed to work yet, right? Since the histogram button doesn't do anything (// Do nothing). Let me know when that part is ready for review!

Loori-R and others added 23 commits November 3, 2023 13:24
@Loori-R
ui: add heatmap plugin support for flot graph
cec0f4e 
Signed-off-by: Yury Moladau <yurymolodov@gmail.com>
@jdellithorpe-db @Loori-R
Fix/clarify documentation for axn notation
079204d 
Signed-off-by: Jonathan Ellithorpe <jelli@databricks.com>
@linasm @Loori-R
Fix NaN sum check in [Float]Histogram.Equals method
93b2d48 
Signed-off-by: Linas Medziunas <linas.medziunas@gmail.com>
@linasm @Loori-R
Compare FloatHistogram.[Zero]Count float values as binary
8a20034 
Signed-off-by: Linas Medziunas <linas.medziunas@gmail.com>
@linasm @Loori-R
Expand docs comments
2338ada 
Signed-off-by: Linas Medziunas <linas.medziunas@gmail.com>
@linasm @Loori-R
Update comments, bitwise comparison of float buckets
a2be221 
Signed-off-by: Linas Medziunas <linas.medziunas@gmail.com>
@linasm @Loori-R
Mention bucket values in the comment
59e1f46 
Signed-off-by: Linas Medziunas <linas.medziunas@gmail.com>
@marctc @Loori-R
promql(histograms): Change sample total calculation for histograms
d113bcb 
Signed-off-by: Marc Tuduri <marctc@protonmail.com>
@marctc @Loori-R
PR feedback
2d1d370 
Signed-off-by: Marc Tuduri <marctc@protonmail.com>
@marctc @Loori-R
More periods
ad13a58 
Signed-off-by: Marc Tuduri <marctc@protonmail.com>
@Loori-R
Parse created timestamps from Prometheus Protobuf (#12973)
3a8245a 
Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
@zenador @Loori-R
Very minor refactor of the integer overflow fix
dcd6b97 
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
@beorn7 @Loori-R
textparse: Update comment about timestamp_ms protobuf parsing
a5a9162 
By now, we know better what the plan is.

Signed-off-by: beorn7 <beorn@grafana.com>
@beorn7 @Loori-R
prompb: Remove gogoproto.nullable) = true] from created_timestamp
880f807 
In proto3, this doesn't change anything. However, since the
`CreatedTimestamp` field is generated as a pointer
(`*types.Timestamp`), we are still able to detect the unset state.
(This is in contrast to the `timestamp_ms` field, which is a plain
int64, for which we cannot enforce generation as a pointer, see
comment updated in the previous commit for future actions.)

Signed-off-by: beorn7 <beorn@grafana.com>
@bwplotka @Loori-R
native-histograms: Fixed PrometheusProto scrape format preference. (#…
8a0de75 
…13010)

Broken by #12738. We have to update both global variables (as GlobalConfig is not a pointer here).
DefaultConfig is used when no global: section is provided, whereas DefaultGlobalConfig is used when it's provided and for individual scrape configs.

Reported on #prometheus-dev (thanks to @beorn7): https://cloud-native.slack.com/archives/C01AUBA4PFE/p1697733267205649

Tested manually, it would be nice to add test at some point (quick fix for now).

Signed-off-by: bwplotka <bwplotka@gmail.com>
@rgroothuijsen @Loori-R
Set test group interval default to evaluation interval (#13011)
f2b0f16 
Signed-off-by: Rens Groothuijsen <l.groothuijsen@alumni.maastrichtuniversity.nl>
@YannickTeKulve @Loori-R
Bump prometheus common to v0.45.0 (#13003)
a407f2e 
* Bump prometheus common to v0.44.0

Signed-off-by: Yannick te Kulve <738464+YannickTeKulve@users.noreply.github.com>

* Fix golang_protobuf_extensions sum

Signed-off-by: Yannick te Kulve <738464+YannickTeKulve@users.noreply.github.com>

* Remove unused deps

Signed-off-by: Yannick te Kulve <738464+YannickTeKulve@users.noreply.github.com>

---------

Signed-off-by: Yannick te Kulve <738464+YannickTeKulve@users.noreply.github.com>
@gillesdemey @Loori-R
ui: Pass unexpected boot errors to StartingContent component (#13016)
614dc79 
Signed-off-by: Gilles De Mey <gilles.de.mey@gmail.com>
@dannykopping @Loori-R
Refactoring manager.go into separate concerns
7eb5dc0 
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
@bboreham @Loori-R
TSDB: Pre-size buffer to read samples from WAL
dae2c01 
When reading the WAL this method is called with buffers from a pool, on
multiple goroutines. Pre-allocating sufficient size avoids slow growth
and many reallocations in `append`.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
@msscaroso @Loori-R
Expose --storage.tsdb.retention.time in metric prometheus_tsdb_retent…
6969bd1 
…ion_limit_seconds (#12986)

* Expose --storage.tsdb.retention.time in a metric

Signed-off-by: Marcio Caroso <msscaroso@gmail.com>

---------

Signed-off-by: Marcio Caroso <msscaroso@gmail.com>
@ptodev @Loori-R
Scrape metrics can now be registered with a non-default registry.
aec1728 
* A registerer is passed to the scrape Manager,
and all scrape metrics register with it.
* For now the registry which we pass to the scrape
Manager is still the global one.

Signed-off-by: Paulin Todev <paulin.todev@gmail.com>
@zenador @Loori-R
Hide position info for warnings when position is unknown (empty query…
80ca4e8 
… string passed in)

Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
bboreham and others added 3 commits November 3, 2023 13:24
@bboreham @Loori-R
Build: remove -a from build to speed up rebuilds (#13026)
f907564 
I think this is a hold-over from when Go was less careful about separating architectures.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
@zenador @Loori-R
Make it possible to unwrap annotation errors
83b0d22 
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
@Loori-R
ui: fix heatmap graph rendering logic
6d47c59
@Loori-R Loori-R marked this pull request as draft November 3, 2023 12:31
@Loori-R Loori-R mentioned this pull request Nov 3, 2023
Merged
@Loori-R
Copy link
Copy Markdown
Contributor Author

Loori-R commented Nov 3, 2023

@beorn7, sorry, when I tried to do the DCO signing, I got a bit confused and something went wrong) I created a pull request #13096, there is a DCO signature there.

@juliusv, the entire UI should work, when you press the button, the graph is drawn as a histogram. You probably saw the function in the tests (other old functions were done similarly there).

@Loori-R
Copy link
Copy Markdown
Contributor Author

Loori-R commented Nov 3, 2023

This pull request has been closed as a new pull request #13096 has been created.

@Loori-R Loori-R closed this Nov 3, 2023
@Loori-R Loori-R deleted the flotjs-support-heatmap branch November 3, 2023 14:45
@juliusv
Copy link
Copy Markdown
Member

juliusv commented Nov 3, 2023

@Loori-R Oh yeah, I misread that from the test file, but I also tried out the PR locally first and the button didn't do anything. It shows up, but it's disabled and not clickable.

@Loori-R
Copy link
Copy Markdown
Contributor Author

Loori-R commented Nov 4, 2023

@juliusv, The button is active when the results have bucket data for a single series, example query:
sum(increase(go_gc_pauses_seconds_total_bucket[30m])) by (le)

For testing, I used the playground: https://prometheus.demo.do.prometheus.io
It's possible that you're not receiving the data that isn't intended for the histogram.
Can you show the query or its result?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juliusv juliusv Awaiting requested review from juliusv juliusv is a code owner

@dgl dgl Awaiting requested review from dgl dgl is a code owner

@roidelapluie roidelapluie Awaiting requested review from roidelapluie roidelapluie is a code owner

@jesusvazquez jesusvazquez Awaiting requested review from jesusvazquez jesusvazquez is a code owner

@codesome codesome Awaiting requested review from codesome codesome will be requested when the pull request is marked ready for review codesome is a code owner

@bwplotka bwplotka Awaiting requested review from bwplotka bwplotka will be requested when the pull request is marked ready for review bwplotka is a code owner

@krajorama krajorama Awaiting requested review from krajorama krajorama will be requested when the pull request is marked ready for review krajorama is a code owner

@beorn7 beorn7 Awaiting requested review from beorn7 beorn7 will be requested when the pull request is marked ready for review beorn7 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.