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

compact: Add traffic direction to policy verdict events #759

Merged
merged 1 commit into from
Jul 11, 2022

Conversation

michi-covalent
Copy link
Collaborator

Signed-off-by: Michi Mutsuzaki michi@isovalent.com

@michi-covalent michi-covalent requested review from a team and glibsm and removed request for a team July 7, 2022 18:51
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label PR is blocked until the release note is set label Jul 7, 2022
Comment on lines 229 to 232
api.PolicyMatchType(f.GetPolicyMatchType()).String() + " " +
f.GetTrafficDirection().String()

Copy link
Member

Choose a reason for hiding this comment

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

There are no cases when this is empty, right?

Also, minor nit comment: fmt.Sprintf would maybe be easier to read here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah i think it is always set to either ingress or egress. i'll change the code to use Sprintf instead 👍

Copy link
Member

Choose a reason for hiding this comment

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

It's either INGRESS, EGRESS or TRAFFIC_DIRECTION_UNKNOWN if unset (see here). The latter is rather verbose but I don't think it's expected to ever show up.

Copy link
Member

Choose a reason for hiding this comment

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

TRAFFIC_DIRECTION_UNKNOWN can be set for non-policy-verdict flows. But yes, for policy verdict it should always be set to EGRESS or INGRESS.

@michi-covalent
Copy link
Collaborator Author

sample output

Screen Shot 2022-07-07 at 4 05 01 PM

@michi-covalent michi-covalent added the release-note/minor This PR introduces functionality that users may find relevant to operating Hubble. label Jul 7, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label PR is blocked until the release note is set label Jul 7, 2022
Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 11, 2022
@michi-covalent michi-covalent merged commit 1ae3fab into master Jul 11, 2022
@michi-covalent michi-covalent deleted the pr/michi/direction branch July 11, 2022 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR introduces functionality that users may find relevant to operating Hubble.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants