Skip to content

Conversation

@ronensc
Copy link
Collaborator

@ronensc ronensc commented Mar 30, 2022

This PR is on top of #152. Should be merged after that one is merged.

Solves issues: #19 #9

Print screens of the added visualizations are attached below:
Heatmap in the details dashboard
image
Histogram in the totals dashboard
image

The visualizations are based on:
https://grafana.com/blog/2020/06/23/how-to-visualize-prometheus-histograms-in-grafana/

@ronensc ronensc self-assigned this Mar 30, 2022
@ronensc ronensc force-pushed the histogram-visualization branch from b66a4b7 to dbaf984 Compare April 3, 2022 07:52
@ronensc ronensc requested a review from eranra April 3, 2022 07:52
@ronensc ronensc force-pushed the histogram-visualization branch from dbaf984 to 79f62ba Compare April 3, 2022 08:49
Copy link
Collaborator

@eranra eranra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good to me ... all the suggestions are updates for names, variables etc ... in general we do not need to see any more mice and elephants anywhere ... everything can move to be "flows length"

},
"targets": [
{
"expr": "sum(rate(flp_mice_elephants_histogram_bucket[$__interval])) by (le)",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ronensc Please change in all places, documentation, metrics names, etc from mice-elephants to flows-length ... the original mice elephant was sub set of the functionality and the name "flows length" fits now much better

parameters: ">=0"
extract:
aggregates:
- name: bytes_hist
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/bytes_hist/flows_bytes_hist

- by
- aggregate
buckets:
- 100
Copy link
Collaborator

@eranra eranra Apr 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ronensc can you update to 128, 2048, 10K, 100K, 1M, and above ... I think that those are more relevant buckets for flows.

ronensc and others added 2 commits April 3, 2022 12:22
Co-authored-by: Eran Raichstein <eranra@il.ibm.com>
Co-authored-by: Eran Raichstein <eranra@il.ibm.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2022

Codecov Report

Merging #164 (fb341ba) into main (6e05f32) will decrease coverage by 0.57%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #164      +/-   ##
==========================================
- Coverage   58.32%   57.74%   -0.58%     
==========================================
  Files          54       54              
  Lines        3105     3117      +12     
==========================================
- Hits         1811     1800      -11     
- Misses       1173     1195      +22     
- Partials      121      122       +1     
Flag Coverage Δ
unittests 57.74% <0.00%> (-0.58%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/confgen/grafana_jsonnet.go 0.00% <0.00%> (ø)
pkg/pipeline/utils/exit.go 52.17% <0.00%> (-47.83%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e05f32...fb341ba. Read the comment docs.

@ronensc ronensc merged commit 86eb797 into netobserv:main Apr 3, 2022
| **Description** | A histogram of flowlog bytes |
|:---|:---|
| **Details** | Flows length distribution over time |
| **Usage** | Evaluate flows length behavior including mice/elephant use-case |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the mention of "mice/elephants" here (and below) still intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eran has updated it recently so I guess it is intended
#164 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KalmanMeth this is the only place where we explain that the histogram is useful for the mice/elephant use-case --- hope that this make sense to you

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants