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-669: overview export PNG #322

Merged
merged 11 commits into from
May 26, 2023

Conversation

Amoghrd
Copy link
Contributor

@Amoghrd Amoghrd commented Apr 7, 2023

Export overview page and each panel in overview page
Update topology export function

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Apr 7, 2023

@Amoghrd: This pull request references NETOBSERV-669 which is a valid jira issue.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@codecov
Copy link

codecov bot commented Apr 7, 2023

Codecov Report

Merging #322 (0cedc6b) into main (a5117f8) will increase coverage by 2.74%.
The diff coverage is 25.71%.

❗ Current head 0cedc6b differs from pull request most recent head 48804ea. Consider uploading reports for the commit 48804ea to get more accurate results

@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
+ Coverage   55.18%   57.93%   +2.74%     
==========================================
  Files          29      150     +121     
  Lines        1745     6642    +4897     
  Branches        0      795     +795     
==========================================
+ Hits          963     3848    +2885     
- Misses        694     2577    +1883     
- Partials       88      217     +129     
Flag Coverage Δ
uitests 58.77% <25.71%> (?)
unittests 55.56% <ø> (+0.37%) ⬆️

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

Impacted Files Coverage Δ
...c/components/netflow-overview/netflow-overview.tsx 80.00% <0.00%> (ø)
...omponents/netflow-topology/2d/topology-content.tsx 28.79% <ø> (ø)
web/src/utils/export.ts 14.28% <14.28%> (ø)
web/src/components/netflow-traffic.tsx 53.53% <20.00%> (ø)
...eb/src/components/netflow-overview/panel-kebab.tsx 66.66% <57.14%> (ø)
...onents/netflow-overview/netflow-overview-panel.tsx 100.00% <100.00%> (ø)

... and 118 files with indirect coverage changes

@Amoghrd Amoghrd force-pushed the netobserv-669 branch 3 times, most recently from 04c3c12 to 0056e65 Compare April 7, 2023 17:43
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Apr 20, 2023

@Amoghrd: This pull request references NETOBSERV-669 which is a valid jira issue.

In response to this:

Export overview page and each panel in overview page
Update topology export function

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Amoghrd Amoghrd marked this pull request as ready for review April 24, 2023 14:19
@Amoghrd
Copy link
Contributor Author

Amoghrd commented May 2, 2023

@jpinsonneau Could you please review this when you are free

Copy link
Contributor

@jpinsonneau jpinsonneau left a comment

Choose a reason for hiding this comment

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

Small changes but LGTM 🥳 thanks @Amoghrd !

Do you want to centralize the toPng calls in a single place like utils/export.ts ?
It would avoid repeating the options, the link creation and the catch. That can be a followup if you prefer.

web/src/components/netflow-overview/panel-kebab.tsx Outdated Show resolved Hide resolved
web/src/components/netflow-overview/panel-kebab.tsx Outdated Show resolved Hide resolved
web/src/components/netflow-overview/panel-kebab.tsx Outdated Show resolved Hide resolved
web/src/components/netflow-overview/panel-kebab.tsx Outdated Show resolved Hide resolved
@Amoghrd
Copy link
Contributor Author

Amoghrd commented May 5, 2023

/hold

@Amoghrd Amoghrd force-pushed the netobserv-669 branch 2 times, most recently from 5160816 to 0f46231 Compare May 8, 2023 15:29
@Amoghrd
Copy link
Contributor Author

Amoghrd commented May 8, 2023

@jpinsonneau When implementing the utils/export.ts change, I am getting
<e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:9001/api/loki/flows?filters=&limit=50&recordType=flowLog&reporter=destination&timeRange=300&rateInterval=30s&step=15s&type=count to http://localhost:9002/ [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors) <e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:9001/api/frontend-config to http://localhost:9002/ [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors) <e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:9001/api/prometheus/api/v1/rules?type=alert to http://localhost:9002/ [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors) <e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:9001/api/alertmanager/api/v2/silences to http://localhost:9002/ [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors) <e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:9001/api/loki/topology?filters=&limit=5&recordType=flowLog&reporter=destination&timeRange=300&rateInterval=30s&step=15s&type=bytes&scope=namespace to http://localhost:9002/ [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors) <e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:9001/api/loki/topology?filters=&limit=5&recordType=flowLog&reporter=destination&timeRange=300&rateInterval=30s&step=15s&type=bytes&scope=app to http://localhost:9002/ [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors) <e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:9001/api/loki/ready to http://localhost:9002/ [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors)
when I do make-standalone-mock. Could you PTAL and let me know if there's something wrong in the utils file

@jpinsonneau
Copy link
Contributor

@jpinsonneau When implementing the utils/export.ts change, I am getting ` [webpack-dev-server] [HPM] Error occurred while proxying request

@Amoghrd sorry about that. I didn't checked the impact of https://github.com/netobserv/network-observability-console-plugin/pull/315/files on standalone mode that actually conflicts.
I forced the metrics port to 9003 for this specific usage.

Also took the opportunity to refactor a bit the export function

  • renamed to exportToPng
  • check the HTMLElement inside it
  • log error is provided element is undefined

@jpinsonneau
Copy link
Contributor

/lgtm

@Amoghrd
Copy link
Contributor Author

Amoghrd commented May 9, 2023

/unhold

@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label May 11, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 15, 2023

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm label May 15, 2023
@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 15, 2023
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label May 22, 2023
@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 22, 2023
@jpinsonneau jpinsonneau added lgtm ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. labels May 23, 2023
@github-actions
Copy link

New image: quay.io/netobserv/network-observability-console-plugin:97aed2d. It will expire after two weeks.

@openshift-ci openshift-ci bot removed the lgtm label May 24, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 24, 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 24, 2023
@memodi
Copy link
Contributor

memodi commented May 25, 2023

/label qe-approved

1 similar comment
@memodi
Copy link
Contributor

memodi commented May 25, 2023

/label qe-approved

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

openshift-ci bot commented May 26, 2023

[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

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-merge-robot openshift-merge-robot merged commit 8a60339 into netobserv:main May 26, 2023
@Amoghrd Amoghrd deleted the netobserv-669 branch May 26, 2023 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants