Skip to content
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-765 Restore metrics #315

Merged
merged 2 commits into from May 5, 2023
Merged

NETOBSERV-765 Restore metrics #315

merged 2 commits into from May 5, 2023

Conversation

jotak
Copy link
Member

@jotak jotak commented Mar 28, 2023

No description provided.

@jotak
Copy link
Member Author

jotak commented Mar 28, 2023

Hey @OlivierCazade @jpinsonneau ,
I'm not 100% decided on what to do here. Those metrics don't carry any sensitive data. It's not like FLP metrics.
The alternative could be to cover /metrics with AuthChecker, which would require a bearer token config in ServiceMonitor.

@jpinsonneau
Copy link
Contributor

The alternative could be to cover /metrics with AuthChecker, which would require a bearer token config in ServiceMonitor.

How hard is it to implement ?

Copy link
Collaborator

@OlivierCazade OlivierCazade left a comment

Choose a reason for hiding this comment

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

There should be changes in go.mod and go.sum files right?

@OlivierCazade
Copy link
Collaborator

Another possibility would be to serve the metrics on a different port.

@jotak
Copy link
Member Author

jotak commented Mar 29, 2023

There should be changes in go.mod and go.sum files right?

No change there, prometheus is already imported, there's just a couple more files being used hence added to vendors

@jotak
Copy link
Member Author

jotak commented Mar 29, 2023

@OlivierCazade I implemented your suggestion; see also netobserv/network-observability-operator#310

@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Merging #315 (4196fcb) into main (2a47af3) will decrease coverage by 0.24%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
- Coverage   58.31%   58.08%   -0.24%     
==========================================
  Files         148      149       +1     
  Lines        6592     6615      +23     
  Branches      792      792              
==========================================
- Hits         3844     3842       -2     
- Misses       2531     2556      +25     
  Partials      217      217              
Flag Coverage Δ
uitests 58.97% <ø> (ø)
unittests 55.56% <0.00%> (-0.87%) ⬇️

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

Impacted Files Coverage Δ
cmd/plugin-backend.go 0.00% <0.00%> (ø)
pkg/server/metrics_server.go 0.00% <0.00%> (ø)
pkg/server/routes.go 89.65% <ø> (-0.67%) ⬇️

@memodi
Copy link
Contributor

memodi commented Apr 25, 2023

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 25, 2023
@github-actions
Copy link

New image: ["quay.io/netobserv/network-observability-console-plugin:59a7515"]. It will expire after two weeks.

@memodi
Copy link
Contributor

memodi commented Apr 26, 2023

@jotak - I tested this with plugin image listed here and used NOO image from PR: netobserv/network-observability-operator#310. I enabled user workload monitoring, but I am not able to see plugin metrics being scraped, in below screenshot you can find metrics from just FLP container, anything I am missing?

image

Copy link
Collaborator

@OlivierCazade OlivierCazade left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link

openshift-ci bot commented May 4, 2023

New changes are detected. LGTM label has been removed.

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label May 4, 2023
@jotak
Copy link
Member Author

jotak commented May 4, 2023

@memodi I retried this morning and it worked for me. Same steps as you: deploy my operator PR image, set this console plugin image, enable user workload monitoring... wait a minute or so to get metrics populated, and they are:

Capture d’écran du 2023-05-04 11-40-54

Also if you trigger some queries to Loki via the console plugin, you should get dedicated metrics such as netobserv_http_api_calls_duration_bucket

Capture d’écran du 2023-05-04 11-41-27

If you retry, could you check the Targets page to see if the console plugin is listed and w/o errors?
E.g:

Capture d’écran du 2023-05-04 11-46-42

@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label May 4, 2023
@github-actions
Copy link

github-actions bot commented May 4, 2023

New image: ["quay.io/netobserv/network-observability-console-plugin:9bfd9ec"]. It will expire after two weeks.

@memodi
Copy link
Contributor

memodi commented May 5, 2023

yes, I can see those metrics today. Not sure what I had missed other day. Anyway, thank you for checking.

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label May 5, 2023
@jotak
Copy link
Member Author

jotak commented May 5, 2023

/approve

@openshift-ci
Copy link

openshift-ci bot commented May 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jotak

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label May 5, 2023
@jotak jotak merged commit 862ff9e into netobserv:main May 5, 2023
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. qe-approved QE has approved this pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants