NETOBSERV-2359 Implement metrics display by jpinsonneau · Pull Request #333 · netobserv/network-observability-cli

jpinsonneau · 2025-07-11T10:19:47Z

Description

Query prometheus metrics from collector pod
Display metrics graphs
Show legends on focus
allow time range selection

Screencast.From.2025-07-11.12-20-44.mp4

Dependencies

Based on #215

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

jpinsonneau · 2025-08-07T14:30:26Z

Rebased without changes

codecov · 2025-08-07T14:45:58Z

Codecov Report

❌ Patch coverage is 0.29155% with 684 lines in your changes missing coverage. Please review.
✅ Project coverage is 13.67%. Comparing base (1e7cfe6) to head (a105511).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/metric_display.go	0.00%	311 Missing ⚠️
cmd/display.go	0.00%	121 Missing ⚠️
cmd/prom.go	0.00%	90 Missing ⚠️
cmd/metric_capture.go	0.00%	70 Missing ⚠️
cmd/mocks.go	0.00%	49 Missing ⚠️
cmd/root.go	0.00%	17 Missing ⚠️
cmd/flow_display.go	6.25%	15 Missing ⚠️
cmd/map_format.go	12.50%	7 Missing ⚠️
cmd/flow_capture.go	0.00%	2 Missing ⚠️
cmd/packet_capture.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
- Coverage   17.14%   13.67%   -3.47%     
==========================================
  Files          15       18       +3     
  Lines        2141     2684     +543     
==========================================
  Hits          367      367              
- Misses       1748     2291     +543     
  Partials       26       26

Flag	Coverage Δ
unittests	`13.67% <0.29%> (-3.47%)`	⬇️

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

Files with missing lines	Coverage Δ
cmd/options.go	`25.00% <ø> (ø)`
cmd/flow_capture.go	`6.86% <0.00%> (+0.06%)`	⬆️
cmd/packet_capture.go	`0.00% <0.00%> (ø)`
cmd/map_format.go	`27.53% <12.50%> (-0.41%)`	⬇️
cmd/flow_display.go	`23.62% <6.25%> (+3.52%)`	⬆️
cmd/root.go	`23.21% <0.00%> (-3.32%)`	⬇️
cmd/mocks.go	`0.00% <0.00%> (ø)`
cmd/metric_capture.go	`0.00% <0.00%> (ø)`
cmd/prom.go	`0.00% <0.00%> (ø)`
cmd/display.go	`0.00% <0.00%> (ø)`
... and 1 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-08-26T08:24:12Z

New image:
quay.io/netobserv/network-observability-cli:47509a7

It will expire after two weeks.

To use this build, update your commands using:

USER=netobserv VERSION=47509a7 make commands

or download the updated commands.

jpinsonneau · 2025-08-26T08:24:57Z

cmd/metric_capture.go

+		false,
+		"/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt",
+		"/var/run/secrets/kubernetes.io/serviceaccount/token",
+		"https://thanos-querier.openshift-monitoring.svc:9091/",


These resources are hardcoded for now. Let me know if you feel that should be configurable at some point

how do we handle this in Operator, are these hardcoded there as well?

@memodi no in the operator it's configurable

I guess it can start as hardcoded and we'll make configurable later if someone asks

memodi · 2025-09-02T19:12:12Z

/ok-to-test

github-actions · 2025-09-02T19:14:10Z

New image:
quay.io/netobserv/network-observability-cli:a28a705

It will expire after two weeks.

To use this build, update your commands using:

USER=netobserv VERSION=a28a705 make commands

or download the updated commands.

memodi · 2025-09-02T19:36:09Z

@jpinsonneau - the metrics display work fine, I wonder if we can add an option to disable metrics display on terminal and just use the existing behavior? Currently when exiting kills entire cli.

memodi · 2025-09-02T19:55:55Z

@jpinsonneau - the metrics display work fine, I wonder if we can add an option to disable metrics display on terminal and just use the existing behavior? Currently when exiting kills entire cli.

never mind, I see you can do that using background option.

memodi · 2025-09-02T20:27:49Z

/label qe-approved

jpinsonneau · 2025-09-03T07:37:04Z

@jpinsonneau - the metrics display work fine, I wonder if we can add an option to disable metrics display on terminal and just use the existing behavior? Currently when exiting kills entire cli.

never mind, I see you can do that using background option.

Do you want me to clarify that in the help ?
The flows, packets and metrics features are now aligned on the usage but it feels strange when you are coming from previous versions 😆

jpinsonneau · 2025-09-19T07:54:49Z

@memodi I think integrations tests are flaky here:

Expected
    <string>: genev_sys_6081
not to equal
    <string>: genev_sys_6081 failed [FAILED] Flow should not contain excluded interface genev_sys_6081, but found it in interfaces: [genev_sys_6081]

This error message says we captured a flow with genev_sys_6081 whereas the capture didn't even run: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/netobserv_network-observability-cli/333/pull-ci-netobserv-network-observability-cli-main-integration-tests/1968639165360771072/artifacts/integration-tests/netobserv-cli-tests/artifacts/20250918-124247-flowInterfacesOutput

That happens since getFlowsJSONFile is considering the latest run to be the right one and it's not the case here.

leandroberetta · 2025-09-22T12:13:04Z

Hi @jpinsonneau, I tested the last commit, probably I have less artifacts but still having issues when going full screen:

Screencast.From.2025-09-22.09-08-06.mp4

Screencast.From.2025-09-22.09-07-02.mp4

jotak

/lgtm

memodi · 2025-09-22T13:48:47Z

This error message says we captured a flow with genev_sys_6081 whereas the capture didn't even run: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/netobserv_network-observability-cli/333/pull-ci-netobserv-network-observability-cli-main-integration-tests/1968639165360771072/artifacts/integration-tests/netobserv-cli-tests/artifacts/20250918-124247-flowInterfacesOutput

That happens since getFlowsJSONFile is considering the latest run to be the right one and it's not the case here

from the logs it looks it copy was skipped, need to look more as to why this happened despite it's running with --copy=true.

memodi · 2025-09-22T14:47:41Z

from the logs it looks it copy was skipped, need to look more as to why this happened despite it's running with --copy=true.

I ran tests locally from this PR and it passes fine, I wonder, some securityConstraints that were added in this PR to netobserv-cli NS https://github.com/netobserv/network-observability-cli/pull/374/files#diff-b76153cc13c8e1e4a3b15cfb6d3df444f17cc6160e0ae42e7688a10d2d0f98e5R103 could be causing this?

it's strange that it's consistently failing and prior to #374 being merged this test was passing.

/test integration-tests

jpinsonneau · 2025-09-25T08:08:14Z

Rebased without changes

jotak · 2025-09-25T13:23:47Z

/lgtm

memodi · 2025-09-25T13:30:27Z

/label qe-approved

openshift-ci · 2025-09-26T10:49:44Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-merge-robot added the needs-rebase label Jul 15, 2025

jpinsonneau changed the title ~~Implement metrics display~~ NETOBSERV-2359 Implement metrics display Aug 1, 2025

jpinsonneau marked this pull request as draft August 5, 2025 12:20

openshift-ci bot added the do-not-merge/work-in-progress label Aug 5, 2025

jpinsonneau force-pushed the graphs branch from a6ee0ad to 5bfc62f Compare August 7, 2025 14:29

openshift-merge-robot removed the needs-rebase label Aug 7, 2025

jpinsonneau force-pushed the graphs branch from 5bfc62f to aefb27f Compare August 7, 2025 14:41

jpinsonneau force-pushed the graphs branch 2 times, most recently from c5065c6 to 26adbd4 Compare August 26, 2025 08:21

jpinsonneau marked this pull request as ready for review August 26, 2025 08:22

openshift-ci bot removed the do-not-merge/work-in-progress label Aug 26, 2025

jpinsonneau added do-not-merge/work-in-progress needs-review Tells that the PR needs a review ok-to-test and removed do-not-merge/work-in-progress labels Aug 26, 2025

jpinsonneau commented Aug 26, 2025

View reviewed changes

github-actions bot removed the ok-to-test label Aug 26, 2025

jpinsonneau mentioned this pull request Aug 29, 2025

CLI UI rework netobserv/netobserv.github.io#27

Closed

openshift-ci bot added the ok-to-test label Sep 2, 2025

openshift-ci bot added the qe-approved label Sep 2, 2025

jpinsonneau requested a review from leandroberetta September 3, 2025 07:37

jotak approved these changes Sep 22, 2025

View reviewed changes

openshift-ci bot assigned jotak Sep 22, 2025

openshift-ci bot added the lgtm label Sep 22, 2025

jpinsonneau added 11 commits September 25, 2025 10:05

vendor

81e0963

metrics display

3512c47

add time range dropdown

e9416af

update tests

9f05666

fix error messages

b6a7fb8

moved heartbeat to display functions and don't crash on error

216a854

limit maximum metrics

de5dfba

run metrics integration test in background

9db7578

reduce FPS for metrics display

bff6ac6

fix and simplify connectivity check

868c541

force render on graph selections

a105511

jpinsonneau force-pushed the graphs branch from 6c17d80 to a105511 Compare September 25, 2025 08:08

openshift-ci bot removed the lgtm label Sep 25, 2025

jpinsonneau removed the needs-changes To be added to denote PR needs changes or some questions/comments to be addressed label Sep 25, 2025

openshift-ci bot added the lgtm label Sep 25, 2025

jpinsonneau mentioned this pull request Sep 25, 2025

[Backport] NETOBSERV-2359 Implement metrics display #387

Merged

openshift-ci bot added the qe-approved label Sep 25, 2025

jpinsonneau added the approved label Sep 26, 2025

openshift-merge-bot bot merged commit 5ba90b9 into netobserv:main Sep 26, 2025
13 checks passed

Conversation

jpinsonneau commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Checklist

Uh oh!

jpinsonneau commented Aug 7, 2025

Uh oh!

codecov bot commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Aug 26, 2025

Uh oh!

jpinsonneau Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

memodi Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

jotak Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

jotak Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

memodi commented Sep 2, 2025

Uh oh!

github-actions bot commented Sep 2, 2025

Uh oh!

memodi commented Sep 2, 2025

Uh oh!

memodi commented Sep 2, 2025

Uh oh!

memodi commented Sep 2, 2025

Uh oh!

jpinsonneau commented Sep 3, 2025

Uh oh!

jpinsonneau commented Sep 19, 2025

Uh oh!

leandroberetta commented Sep 22, 2025

Uh oh!

jotak left a comment

Choose a reason for hiding this comment

Uh oh!

memodi commented Sep 22, 2025

Uh oh!

memodi commented Sep 22, 2025

Uh oh!

jpinsonneau commented Sep 25, 2025

Uh oh!

jotak commented Sep 25, 2025

Uh oh!

memodi commented Sep 25, 2025

Uh oh!

openshift-ci bot commented Sep 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jpinsonneau commented Jul 11, 2025 •

edited

Loading

codecov bot commented Aug 7, 2025 •

edited

Loading