-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NETOBSERV-1265 Overview / Topology fully drop metrics doesn't show #373
Conversation
jpinsonneau
commented
Aug 23, 2023
•
edited
Loading
edited
- Overview will show "No results found" on the entire page only if every metrics buckets are empty, else it will show per graph results.
- Topology will fallback on dropped metrics if metrics are empty, only for that particular case. Everything then appears red.
Codecov Report
@@ Coverage Diff @@
## main #373 +/- ##
==========================================
- Coverage 57.83% 57.27% -0.56%
==========================================
Files 166 167 +1
Lines 7679 7752 +73
Branches 918 921 +3
==========================================
- Hits 4441 4440 -1
- Misses 2971 3042 +71
- Partials 267 270 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=de948fa make set-plugin-image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes /LGTM just have one question
_.isEmpty(metrics) && | ||
_.isEmpty(droppedMetrics) && | ||
_.isEmpty(dnsLatencyMetrics) && | ||
_.isEmpty(dnsRCodeMetrics) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to also add || !totalMetric
to this condition ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made totalMetrics
optionnal here and manage these by graph as it depends of the kebab options of each graph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kebab options
?!? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah true story ! And it's not something you eat 🤣
https://github.com/netobserv/network-observability-console-plugin/blob/main/web/src/components/netflow-overview/panel-kebab.tsx#L208
Mainly in these you can select to compare your rates against total / total drops for exmple
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will review kebab
pieces at lunch time :)
/LGTM
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpinsonneau The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |