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-330 Provide a way to filter Kubernetes-related flows #176

Merged
merged 3 commits into from
Jul 19, 2022

Conversation

jpinsonneau
Copy link
Contributor

@jpinsonneau jpinsonneau commented Jun 16, 2022

Added Traffic Scope query options:

  • Infrastructure
  • Applications
  • Both

It uses regex (kube-|openshift-).* with matches / does not matches operators on both SrcNamespace and DstNamespace

image

@openshift-ci
Copy link

openshift-ci bot commented Jun 16, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from jpinsonneau after the PR has been reviewed.

The full list of commands accepted by this bot can be found 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

I found a bug while implementing this https://issues.redhat.com/browse/NETOBSERV-399

pkg/loki/flow_query.go Outdated Show resolved Hide resolved
web/locales/en/plugin__network-observability-plugin.json Outdated Show resolved Hide resolved
web/src/components/dropdowns/query-options-dropdown.tsx Outdated Show resolved Hide resolved
@jotak
Copy link
Member

jotak commented Jun 22, 2022

what about traffic from infra to apps, or from apps to infra? I dont' know if it should be included in the "apps" traffic... thoughts? cc @stleerh @eranra
That being said, maybe it's not as easy as it sounds to implement in logQL... not sure

@eranra
Copy link
Contributor

eranra commented Jun 22, 2022

what about traffic from infra to apps, or from apps to infra? I dont' know if it should be included in the "apps" traffic... thoughts? cc @stleerh @eranra That being said, maybe it's not to as easy as implement as it sounds, in logQL... not sure

My thoughts:

If 'infra' is involved (src or dst or both) it should be 'infra'
If 'app' is involved (src or dst or both) and only if 'infra' is not involved ( this includes:: 'app' to 'app', 'app' to internet, and 'internet' to 'app') it should be 'app'

BTW: This applies to services and pods

Hope this helps

@stleerh
Copy link
Contributor

stleerh commented Jun 23, 2022

I think anywhere that 'app' appears should be included as "apps" traffic.  Otherwise, you will miss router -> app.

@jotak
Copy link
Member

jotak commented Jun 23, 2022

I think anywhere that 'app' appears should be included as "apps" traffic. Otherwise, you will miss router -> app.

Exactly, we're missing ingress traffic coming to the application, that's the main issue with this approach I think

@jpinsonneau
Copy link
Contributor Author

I think anywhere that 'app' appears should be included as "apps" traffic.  Otherwise, you will miss router -> app.

Ok I'm digging in that direction now and see a lot of noise coming from kubernetes API (default namespace).

Should we filter openshift related content that uses API ? It's a nonsense for me to see it here.
image

@jpinsonneau
Copy link
Contributor Author

@stleerh @jotak @eranra I would suggest to add a configurable regex that match ingress namespace. Default value: .*-ingress$

With that we can force these to appear in the application view:
image

@jotak
Copy link
Member

jotak commented Jul 5, 2022

@stleerh @jotak @eranra I would suggest to add a configurable regex that match ingress namespace. Default value: .*-ingress$

Good idea :)

@openshift-ci
Copy link

openshift-ci bot commented Jul 6, 2022

@jpinsonneau: PR needs rebase.

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.

@jotak
Copy link
Member

jotak commented Jul 19, 2022

thanks @jpinsonneau
/lgtm

@jpinsonneau
Copy link
Contributor Author

/approved

@jpinsonneau jpinsonneau merged commit 2505d56 into netobserv:main Jul 19, 2022
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