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-1337 DSCP filter & side panel details #401

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

jpinsonneau
Copy link
Contributor

@jpinsonneau jpinsonneau commented Sep 22, 2023

Description

Add DSCP

  • Filter
  • Side panel details in Protocol field as same as ICMP.
    We can rename / move if necessary.

image
image

A followup will contains the following changes:

Dependencies

netobserv/netobserv-ebpf-agent#158
netobserv/flowlogs-pipeline#503

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.

  • Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).
  • Does this PR require product documentation?
    • If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.
  • Does this PR require a product release notes entry?
    • If so, fill in "Release Note Text" in the JIRA.
  • Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.
    • If so, make sure it is described in the JIRA ticket.
  • QE requirements (check 1 from the list):
    • Standard QE validation, with pre-merge tests unless stated otherwise.
    • Regression tests only (e.g. refactoring with no user-facing change).
    • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Attention: 44 lines in your changes are missing coverage. Please review.

Comparison is base (62bdb9d) 56.69% compared to head (e0059b1) 57.20%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #401      +/-   ##
==========================================
+ Coverage   56.69%   57.20%   +0.51%     
==========================================
  Files          29      169     +140     
  Lines        2025     7990    +5965     
  Branches        0      977     +977     
==========================================
+ Hits         1148     4571    +3423     
- Misses        761     3140    +2379     
- Partials      116      279     +163     
Flag Coverage Δ
uitests 58.09% <12.00%> (?)
unittests 54.71% <100.00%> (-1.98%) ⬇️

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

Files Coverage Δ
pkg/model/fields/fields.go 100.00% <100.00%> (ø)
web/src/api/ipfix.ts 100.00% <ø> (ø)
web/src/model/filters.ts 78.46% <ø> (ø)
web/src/utils/filter-definitions.ts 66.66% <ø> (ø)
web/src/components/netflow-record/record-field.tsx 64.56% <15.78%> (ø)
web/src/utils/dscp.ts 9.67% <9.67%> (ø)

... and 135 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jpinsonneau jpinsonneau changed the title WIP DSCP NETOBSERV-588 DSCP filter & side panel details Sep 22, 2023
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Sep 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 Sep 25, 2023
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Sep 25, 2023
@netobserv netobserv deleted a comment from github-actions bot Sep 25, 2023
@github-actions
Copy link

New image:
quay.io/netobserv/network-observability-console-plugin:055d42e

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=055d42e make set-plugin-image

@jpinsonneau jpinsonneau changed the title NETOBSERV-588 DSCP filter & side panel details NETOBSERV-1337 DSCP filter & side panel details Sep 29, 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 Oct 5, 2023
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Oct 5, 2023
@jpinsonneau jpinsonneau marked this pull request as ready for review October 5, 2023 13:38
@github-actions
Copy link

github-actions bot commented Oct 5, 2023

New image:
quay.io/netobserv/network-observability-console-plugin:3b262b1

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=3b262b1 make set-plugin-image

@Amoghrd
Copy link
Contributor

Amoghrd commented Oct 5, 2023

@jpinsonneau The branch still shows out-of-date with master, should it be rebased with master to get the duplicate field added to labels change or is it fine?

@jpinsonneau
Copy link
Contributor Author

@jpinsonneau The branch still shows out-of-date with master, should it be rebased with master to get the duplicate field added to labels change or is it fine?

It's the async overview; it's not related you can go ahead with the current version 😉

@Amoghrd
Copy link
Contributor

Amoghrd commented Oct 5, 2023

@jpinsonneau Question regarding the DSCP value filter. Even though there is DSCP related info present under Protocol section in the side panel, there is no filter button present, is this deliberate? I am asking since all other filters have a filter button too next to them in the side panel

@jpinsonneau
Copy link
Contributor Author

@jpinsonneau Question regarding the DSCP value filter. Even though there is DSCP related info present under Protocol section in the side panel, there is no filter button present, is this deliberate? I am asking since all other filters have a filter button too next to them in the side panel

You should have filter button next to "Protocol" field but indeed it filter on protocol and not on DSCP.
I'll add a dropdown in the same style as topology ones that allows to select multiple options (Source / Dest)

@Amoghrd
Copy link
Contributor

Amoghrd commented Oct 9, 2023

@jpinsonneau The above dropdown style for DSCP will be implemented in this PR or a later PR?

@jpinsonneau
Copy link
Contributor Author

@jpinsonneau The above dropdown style for DSCP will be implemented in this PR or a later PR?

As you prefer 😸 what is the easiest for you ?

  • If you want to test all at one I can add both graphs and dropdown in the same PR.
  • If you already tested this and wait for me we can just merge as is and I'll open a followup

@Amoghrd
Copy link
Contributor

Amoghrd commented Oct 9, 2023

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Oct 9, 2023
@jpinsonneau
Copy link
Contributor Author

Created a followup for graphs & dropdown:
https://issues.redhat.com/browse/NETOBSERV-1353

@msherif1234
Copy link
Contributor

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Oct 11, 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

@jpinsonneau
Copy link
Contributor Author

Thanks @Amoghrd @msherif1234 !

@openshift-ci openshift-ci bot merged commit 1982dbc into netobserv:main Oct 11, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm 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.

3 participants