daemon: refactor Hubble Exporters as a cell #35596

devodev · 2024-10-28T15:42:24Z

Continuation of #35206

Introduce a new hubble-exporters cell that provides the static and dynamic exporters using observer Options.
The static exporter now uses the context received from OnDecodedEvent instead of embedding one at construct time.
Add OnExportEvent hook system to the exporter to provide extension capabilities.
Allow providing a custom writer to exporters.
Allow providing a custom encoder to exporters.

Related: #35514

pkg/hubble/cell/cell.go

pkg/hubble/exporter/exporter.go

kaworu

Nice work @devodev 👍

Some comments and questions inline. Also I noticed that in exporter.Stop() we're closing the writer, which can be setup with os.Stdout and I don't think we're supposed to close stdout. Not to be addressed in this PR, but something to look into. cc @chancez whom I added as a reviewer as well since he has context about export.

pkg/hubble/exporter/cell.go

pkg/hubble/exporter/exporter.go

pkg/hubble/exporter/cell.go

devodev · 2024-11-08T15:34:15Z

Nice work @devodev 👍

Some comments and questions inline. Also I noticed that in exporter.Stop() we're closing the writer, which can be setup with os.Stdout and I don't think we're supposed to close stdout. Not to be addressed in this PR, but something to look into. cc @chancez whom I added as a reviewer as well since he has context about export.

Looked into it, and it seems we use a noopWriteCloser to avoid calling Close on the underlying writer.

chancez · 2024-11-08T16:38:01Z

Looked into it, and it seems we use a noopWriteCloser to avoid calling Close on the underlying writer.

Yep, there was previously a bug where we were closing os.Stdout that I fixed in a49276f.

pkg/hubble/exporter/cell.go

pkg/hubble/exporter/config_watcher.go

glrf

Nice work 🚀

Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>

michi-covalent · 2024-11-18T22:05:00Z

/test

michi-covalent · 2024-11-19T14:51:45Z

@kaworu ping 🏓

pippolo84

Focused my review on the newly added cells and the usage of the "hive and cells" framework. LGTM, thanks! 💯

kaworu

Awesome work @devodev 👏 !

Hubble export allow/deny list parsing regressed in cilium#35596 when moving export to a hive cell due to how hive cells handle flags.StringSlices. Similar to cilium#35619. Redefine the flag as a string and parse it ourselves to avoid going through the implicit hive decoder hooks that convert strings to slices by splitting on comma. Also added tests for the hubble.ParseFlowFilters helper to make it clear which formats we support and which we don't. Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>

Hubble export allow/deny list parsing regressed in cilium#35596 when moving export to a hive cell due to how hive cells handle flags.StringSlices. Similar to cilium#35619. Redefine the flag as a string to avoid going through the implicit hive decoder hooks that convert strings to slices by splitting on comma. Update hubble.ParseFlowFilters to take in a string and decode all filters until EOF. Also added tests for the hubble.ParseFlowFilters helper to make it clear which formats we support and which we don't. Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>

Hubble export allow/deny list parsing regressed in #35596 when moving export to a hive cell due to how hive cells handle flags.StringSlices. Similar to #35619. Redefine the flag as a string to avoid going through the implicit hive decoder hooks that convert strings to slices by splitting on comma. Update hubble.ParseFlowFilters to take in a string and decode all filters until EOF. Also added tests for the hubble.ParseFlowFilters helper to make it clear which formats we support and which we don't. Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>

[ upstream commit cb3341b ] Hubble export allow/deny list parsing regressed in #35596 when moving export to a hive cell due to how hive cells handle flags.StringSlices. Similar to #35619. Redefine the flag as a string to avoid going through the implicit hive decoder hooks that convert strings to slices by splitting on comma. Update hubble.ParseFlowFilters to take in a string and decode all filters until EOF. Also added tests for the hubble.ParseFlowFilters helper to make it clear which formats we support and which we don't. Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com> Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>

Hubble export allow/deny list parsing regressed in cilium#35596 when moving export to a hive cell due to how hive cells handle flags.StringSlices. Similar to cilium#35619. Redefine the flag as a string to avoid going through the implicit hive decoder hooks that convert strings to slices by splitting on comma. Update hubble.ParseFlowFilters to take in a string and decode all filters until EOF. Also added tests for the hubble.ParseFlowFilters helper to make it clear which formats we support and which we don't. Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 28, 2024

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Oct 28, 2024

devodev commented Oct 28, 2024

View reviewed changes

pkg/hubble/cell/cell.go Outdated Show resolved Hide resolved

devodev commented Oct 28, 2024

View reviewed changes

pkg/hubble/exporter/exporter.go Outdated Show resolved Hide resolved

devodev force-pushed the pr/devodev/hubble-logging-cell branch 3 times, most recently from e817568 to e11af17 Compare November 4, 2024 22:16

kaworu added kind/cleanup This includes no functional changes. area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. release-note/misc This PR makes changes that have no direct user impact. sig/hubble labels Nov 5, 2024

maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Nov 5, 2024

kaworu self-requested a review November 5, 2024 16:18

devodev force-pushed the pr/devodev/hubble-logging-cell branch 2 times, most recently from e5b6738 to ccbdf7a Compare November 7, 2024 22:41

devodev marked this pull request as ready for review November 7, 2024 23:24

devodev requested a review from a team as a code owner November 7, 2024 23:24

kaworu requested a review from chancez November 8, 2024 08:00

kaworu requested changes Nov 8, 2024

View reviewed changes

glrf reviewed Nov 8, 2024

View reviewed changes

pkg/hubble/exporter/exporter.go Outdated Show resolved Hide resolved

pkg/hubble/exporter/exporter.go Outdated Show resolved Hide resolved

pkg/hubble/exporter/cell.go Outdated Show resolved Hide resolved

chancez reviewed Nov 8, 2024

View reviewed changes

pkg/hubble/exporter/cell.go Outdated Show resolved Hide resolved

chancez reviewed Nov 8, 2024

View reviewed changes

pkg/hubble/exporter/config_watcher.go Show resolved Hide resolved

devodev force-pushed the pr/devodev/hubble-logging-cell branch 3 times, most recently from b76d39d to 81255ab Compare November 8, 2024 21:34

glrf approved these changes Nov 18, 2024

View reviewed changes

hubble: provide exporters using the FlowLogExporter interface

2f7a251

Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>

devodev force-pushed the pr/devodev/hubble-logging-cell branch from 8c317f7 to 2f7a251 Compare November 18, 2024 15:05

michi-covalent requested review from a team, pippolo84 and bimmlerd and removed request for a team November 18, 2024 22:04

bimmlerd removed their request for review November 19, 2024 13:40

bimmlerd approved these changes Nov 19, 2024

View reviewed changes

michi-covalent enabled auto-merge November 19, 2024 14:51

pippolo84 approved these changes Nov 20, 2024

View reviewed changes

kaworu approved these changes Nov 20, 2024

View reviewed changes

michi-covalent added this pull request to the merge queue Nov 20, 2024

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 Nov 20, 2024

Merged via the queue into cilium:main with commit ef23827 Nov 20, 2024
70 checks passed

chancez mentioned this pull request Jan 24, 2025

Hubble export deny/allow list flag parsing broken #37233

Closed

3 tasks

devodev mentioned this pull request Jan 24, 2025

hubble-export: fix allowlist and denylist flow-filters parsing #37261

Merged

kaworu mentioned this pull request Feb 4, 2025

Convert Hubble logging sub-system into new cell #35514

Closed

kaworu mentioned this pull request May 23, 2025

CFP: Convert Hubble into a hive/cell based architecture #34501

Open

daemon: refactor Hubble Exporters as a cell #35596

daemon: refactor Hubble Exporters as a cell #35596

Uh oh!

Conversation

devodev commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kaworu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devodev commented Nov 8, 2024

Uh oh!

chancez commented Nov 8, 2024

Uh oh!

Uh oh!

Uh oh!

glrf left a comment

Choose a reason for hiding this comment

Uh oh!

michi-covalent commented Nov 18, 2024

Uh oh!

michi-covalent commented Nov 19, 2024

Uh oh!

pippolo84 left a comment

Choose a reason for hiding this comment

Uh oh!

kaworu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

devodev commented Oct 28, 2024 •

edited

Loading