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: Always print original source on the left #533

Merged
merged 1 commit into from
Apr 23, 2021

Conversation

michi-covalent
Copy link
Collaborator

Keep the original source on the left and use "<-" for reply packets.

Before:

2021-04-22T03:51:42Z: 10.168.0.11:33192 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: SYN)
2021-04-22T03:51:42Z: kube-system/kube-dns-5d54b45645-p9kvw:8081 -> 10.168.0.11:33192 to-stack FORWARDED (TCP Flags: SYN, ACK)
2021-04-22T03:51:42Z: 10.168.0.11:33192 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK)
2021-04-22T03:51:42Z: 10.168.0.11:33192 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK, PSH)
2021-04-22T03:51:42Z: kube-system/kube-dns-5d54b45645-p9kvw:8081 -> 10.168.0.11:33192 to-stack FORWARDED (TCP Flags: ACK, PSH)
2021-04-22T03:51:42Z: kube-system/kube-dns-5d54b45645-p9kvw:8081 -> 10.168.0.11:33192 to-stack FORWARDED (TCP Flags: ACK, FIN)
2021-04-22T03:51:42Z: 10.168.0.11:33192 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK, FIN)

After:

2021-04-22T03:50:02Z: 10.168.0.11:60982 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: SYN)
2021-04-22T03:50:02Z: 10.168.0.11:60982 <- kube-system/kube-dns-5d54b45645-p9kvw:8081 to-stack FORWARDED (TCP Flags: SYN, ACK)
2021-04-22T03:50:02Z: 10.168.0.11:60982 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK)
2021-04-22T03:50:02Z: 10.168.0.11:60982 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK, PSH)
2021-04-22T03:50:02Z: 10.168.0.11:60982 <- kube-system/kube-dns-5d54b45645-p9kvw:8081 to-stack FORWARDED (TCP Flags: ACK, PSH)
2021-04-22T03:50:02Z: 10.168.0.11:60982 <- kube-system/kube-dns-5d54b45645-p9kvw:8081 to-stack FORWARDED (TCP Flags: ACK, FIN)
2021-04-22T03:50:02Z: 10.168.0.11:60982 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK, FIN)

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

@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 Apr 22, 2021
@michi-covalent michi-covalent added the release-note/minor This PR introduces functionality that users may find relevant to operating Hubble. label Apr 22, 2021
@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 Apr 22, 2021
@michi-covalent
Copy link
Collaborator Author

this is a breaking change since it modifies an existing format. we could introduce another format instead but i wasn't sure if it's worth it.

Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

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

this is a breaking change since it modifies an existing format. we could introduce another format instead but i wasn't sure if it's worth it.

It is indeed a breaking change but I'm not too worried about it. Users wanting to process the hubble observe output likely use a format that is more appropriate for this purpose such as JSON. I think that this change makes the output much more readable so imho it is for the better. We should make it clear in the release notes that this is a breaking change though.

@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 Apr 22, 2021
pkg/printer/printer.go Outdated Show resolved Hide resolved
Keep the original source on the left and use "<-" for reply packets.
If IsReply field is not set, the direction is set to "<>".

Before:
```
2021-04-22T03:51:42Z: 10.168.0.11:33192 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: SYN)
2021-04-22T03:51:42Z: kube-system/kube-dns-5d54b45645-p9kvw:8081 -> 10.168.0.11:33192 to-stack FORWARDED (TCP Flags: SYN, ACK)
2021-04-22T03:51:42Z: 10.168.0.11:33192 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK)
2021-04-22T03:51:42Z: 10.168.0.11:33192 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK, PSH)
2021-04-22T03:51:42Z: kube-system/kube-dns-5d54b45645-p9kvw:8081 -> 10.168.0.11:33192 to-stack FORWARDED (TCP Flags: ACK, PSH)
2021-04-22T03:51:42Z: kube-system/kube-dns-5d54b45645-p9kvw:8081 -> 10.168.0.11:33192 to-stack FORWARDED (TCP Flags: ACK, FIN)
2021-04-22T03:51:42Z: 10.168.0.11:33192 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK, FIN)
```

After:
```
2021-04-22T03:50:02Z: 10.168.0.11:60982 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: SYN)
2021-04-22T03:50:02Z: 10.168.0.11:60982 <- kube-system/kube-dns-5d54b45645-p9kvw:8081 to-stack FORWARDED (TCP Flags: SYN, ACK)
2021-04-22T03:50:02Z: 10.168.0.11:60982 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK)
2021-04-22T03:50:02Z: 10.168.0.11:60982 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK, PSH)
2021-04-22T03:50:02Z: 10.168.0.11:60982 <- kube-system/kube-dns-5d54b45645-p9kvw:8081 to-stack FORWARDED (TCP Flags: ACK, PSH)
2021-04-22T03:50:02Z: 10.168.0.11:60982 <- kube-system/kube-dns-5d54b45645-p9kvw:8081 to-stack FORWARDED (TCP Flags: ACK, FIN)
2021-04-22T03:50:02Z: 10.168.0.11:60982 -> kube-system/kube-dns-5d54b45645-p9kvw:8081 to-endpoint FORWARDED (TCP Flags: ACK, FIN)
```

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@michi-covalent michi-covalent merged commit 8183d93 into master Apr 23, 2021
@michi-covalent michi-covalent deleted the pr/michi/print branch April 23, 2021 01:19
@kaworu
Copy link
Member

kaworu commented Apr 23, 2021

@michi-covalent do we need to "port" this patch to cilium-cli as well?

https://github.com/cilium/cilium-cli/blob/271da5361610232b04dbcd26d254a3d49357128c/connectivity/check/check.go#L453-L493

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