Skip to content

Conversation

cy83rc0llect0r
Copy link
Contributor

Enhancement #1710

Description

This pull request introduces support for stdout, file and direct-stdout export modes in Tetragon, addressing the enhancement request in Enhancement #1710. The new direct-stdout mode allows users to send JSON events directly to stdout with an EVENT: prefix, eliminating the need for a sidecar container and volume mounts in Kubernetes deployments. Tetragon internal logs are now sent to stderr, ensuring clean separation from JSON events. The Helm chart is updated to support the new export.mode configuration, with conditional sidecar and volume management. The stdout mode is deprecated with a fallback to file for backward compatibility.

The changes include:

  • Implementation of JSONStdoutEncoder with EVENT: prefix for direct-stdout mode, enabling easy filtering of JSON events (e.g., kubectl logs | grep '^EVENT:').
  • Configuration of logrus to send Tetragon logs to stderr and addition of export-mode flag in pkg/option/config.go.
  • Update to startExporter in cmd/tetragon/main.go to handle file and direct-stdout modes, with go exporter.Start() for non-blocking event processing.
  • Enhancement of the Helm chart to support export.mode, disabling sidecar and volume for direct-stdout.

Use Case Example
Users can now deploy Tetragon in direct-stdout mode to simplify Kubernetes setups by removing sidecar dependencies, making it ideal for lightweight deployments or environments where direct log processing (e.g., via kubectl logs | jq) is preferred. The EVENT: prefix allows easy parsing of JSON events, while stderr logs facilitate debugging. The file mode remains available for scenarios requiring persistent event storage.

Changelog

Added support for `file` and `direct-stdout` export modes in Tetragon, with JSON events prefixed by `EVENT:` in `direct-stdout` mode and internal logs sent to stderr. Updated Helm chart to manage sidecar and volume based on `export.mode`. Deprecated `stdout` mode with fallback to `file`.

@cy83rc0llect0r cy83rc0llect0r requested review from a team and mtardy as code owners April 24, 2025 13:22
Copy link

netlify bot commented Apr 24, 2025

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 215be57
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/68110f067bf3570009a9cd3d
😎 Deploy Preview https://deploy-preview-3667--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Implement JSONStdoutEncoder to write JSON events to stdout with an EVENT: prefix in direct-stdout mode. This allows users to easily filter JSON events from Tetragon logs using `grep '^EVENT:'`.
The encoder uses os.Stdout.Write with Sync for efficient output and includes debug logs for troubleshooting.

Enhancement cilium#1710

Signed-off-by: Amir Reza Nazarizadeh <[email protected]>
@cy83rc0llect0r cy83rc0llect0r force-pushed the pr/cy83rc0llect0r/add-stdout-support-without-file branch from 6795ce5 to dc7cb76 Compare April 24, 2025 13:53
@cy83rc0llect0r cy83rc0llect0r changed the title Pr/cy83rc0llect0r/add stdout support without file Add stdout support without volume and sidecar Apr 24, 2025
@ghost ghost added the release-note/minor This PR introduces a minor user-visible change label Apr 28, 2025
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Thank you for tackling this @cy83rc0llect0r

Let's first clarify what export configurations should be available to the user. Currently there are:

  • no export (events exposed only via gRPC server)
  • export to a file
  • export to a file + stdout (via sidecar)

IMO we should keep these configurations available. After quickly going over the PR, my understanding is that (1) no export is no longer an option (2) export to a file but not stdout is no longer an option. What I would expect here is to replace the sidecar with a built-in stdout export, but not break other export configurations. Does it make sense?

I don't have a strong opinion about moving Tetragon diagnostics logs to stderr, but it is a technically a breaking change. Tetragon project documents such changes in the upgrade notes - could you add a point in contrib/upgrade-notes/latest.md about this?

Copy link

Choose a reason for hiding this comment

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

There is quite a bit of validation and normalization logic here for the export-mode flag. Could we move it to pkg/option, when the flag is read and set? I think it's more readable and maintainable that way.

@cy83rc0llect0r
Copy link
Contributor Author

Thank you for tackling this @cy83rc0llect0r

Let's first clarify what export configurations should be available to the user. Currently there are:

  • no export (events exposed only via gRPC server)
  • export to a file
  • export to a file + stdout (via sidecar)

IMO we should keep these configurations available. After quickly going over the PR, my understanding is that (1) no export is no longer an option (2) export to a file but not stdout is no longer an option. What I would expect here is to replace the sidecar with a built-in stdout export, but not break other export configurations. Does it make sense?

I don't have a strong opinion about moving Tetragon diagnostics logs to stderr, but it is a technically a breaking change. Tetragon project documents such changes in the upgrade notes - could you add a point in contrib/upgrade-notes/latest.md about this?

@lambdanis Thank you for the detailed feedback and for clarifying the expected export configurations! I’ve reviewed your comments and would like to outline the current state of the PR, address your concerns, and propose a path forward to ensure all configurations are supported as intended.

Current State of the PR

The PR currently implements the following export modes:

  • "" (empty): No export, events are exposed only via the gRPC server (equivalent to the "no export" configuration).
  • file: Events are exported only to a file.
  • direct-stdout: Events are exported directly to stdout in the main Tetragon container with an EVENT: prefix, eliminating the sidecar.
  • stdout: The legacy mode (file export + stdout via sidecar), which is currently deprecated with a fallback to file for backward compatibility.

Additionally:

  • Tetragon internal logs are redirected to stderr to separate them from JSON events on stdout, improving log clarity (e.g., for kubectl logs | grep '^EVENT:').
  • The Helm chart supports these modes via the export.mode value, with conditional volume management (enabled for file and stdout, disabled for direct-stdout and "").

Addressing Your Feedback

I understand your concern that all existing configurations (no export, file, file+stdout) should remain available, and the sidecar should be replaced with a built-in stdout export without breaking other modes. Based on your feedback, it seems the PR might not clearly convey that "" (no export) and file (file only) are still supported. Let me clarify and propose adjustments:

  1. Preserving All Export Configurations:

    • The "" mode is equivalent to no export (gRPC only) and is fully supported.
    • The file mode is equivalent to file only and is also supported.
    • The stdout mode (legacy file+stdout with sidecar) is deprecated in favor of direct-stdout, which replaces the sidecar with a built-in stdout export.
    • Proposal: If you prefer to keep the legacy stdout mode (file + stdout with sidecar) without deprecation, I can restore it as a valid mode alongside direct-stdout. Alternatively, I can add a new file+stdout mode that combines file export with built-in stdout export (no sidecar), ensuring all configurations are available:
      • none (or ""): gRPC only.
      • file: File only.
      • stdout: Built-in stdout only (no sidecar).
      • file+stdout: File + built-in stdout (no sidecar).
      • (Optional) Legacy stdout: File + stdout with sidecar, if you prefer to retain it.
  2. Redirecting Tetragon Logs to stderr:

    • I introduced stderr for Tetragon internal logs to align with Kubernetes logging best practices, where application logs (stderr) are separated from structured data (stdout, e.g., JSON events). This allows users to filter events easily (e.g., kubectl logs | grep '^EVENT:') while accessing Tetragon logs via kubectl logs --stderr.
    • Proposal: If you prefer reverting to the previous behavior (Tetragon logs on stdout), I can undo this change. However, I believe redirecting to stderr is a more standard approach, especially for direct-stdout and file+stdout modes, as it avoids mixing logs and events. I’d appreciate your thoughts on whether to keep stderr or revert.

Looking forward to your feedback!

@cy83rc0llect0r cy83rc0llect0r force-pushed the pr/cy83rc0llect0r/add-stdout-support-without-file branch 6 times, most recently from e6a3962 to 215be57 Compare April 29, 2025 17:40
@cy83rc0llect0r
Copy link
Contributor Author

Thank you for tackling this @cy83rc0llect0r
Let's first clarify what export configurations should be available to the user. Currently there are:

  • no export (events exposed only via gRPC server)
  • export to a file
  • export to a file + stdout (via sidecar)

IMO we should keep these configurations available. After quickly going over the PR, my understanding is that (1) no export is no longer an option (2) export to a file but not stdout is no longer an option. What I would expect here is to replace the sidecar with a built-in stdout export, but not break other export configurations. Does it make sense?
I don't have a strong opinion about moving Tetragon diagnostics logs to stderr, but it is a technically a breaking change. Tetragon project documents such changes in the upgrade notes - could you add a point in contrib/upgrade-notes/latest.md about this?

@lambdanis Thank you for the detailed feedback and for clarifying the expected export configurations! I’ve reviewed your comments and would like to outline the current state of the PR, address your concerns, and propose a path forward to ensure all configurations are supported as intended.

Current State of the PR

The PR currently implements the following export modes:

  • "" (empty): No export, events are exposed only via the gRPC server (equivalent to the "no export" configuration).
  • file: Events are exported only to a file.
  • direct-stdout: Events are exported directly to stdout in the main Tetragon container with an EVENT: prefix, eliminating the sidecar.
  • stdout: The legacy mode (file export + stdout via sidecar), which is currently deprecated with a fallback to file for backward compatibility.

Additionally:

  • Tetragon internal logs are redirected to stderr to separate them from JSON events on stdout, improving log clarity (e.g., for kubectl logs | grep '^EVENT:').
  • The Helm chart supports these modes via the export.mode value, with conditional volume management (enabled for file and stdout, disabled for direct-stdout and "").

Addressing Your Feedback

I understand your concern that all existing configurations (no export, file, file+stdout) should remain available, and the sidecar should be replaced with a built-in stdout export without breaking other modes. Based on your feedback, it seems the PR might not clearly convey that "" (no export) and file (file only) are still supported. Let me clarify and propose adjustments:

  1. Preserving All Export Configurations:

    • The "" mode is equivalent to no export (gRPC only) and is fully supported.

    • The file mode is equivalent to file only and is also supported.

    • The stdout mode (legacy file+stdout with sidecar) is deprecated in favor of direct-stdout, which replaces the sidecar with a built-in stdout export.

    • Proposal: If you prefer to keep the legacy stdout mode (file + stdout with sidecar) without deprecation, I can restore it as a valid mode alongside direct-stdout. Alternatively, I can add a new file+stdout mode that combines file export with built-in stdout export (no sidecar), ensuring all configurations are available:

      • none (or ""): gRPC only.
      • file: File only.
      • stdout: Built-in stdout only (no sidecar).
      • file+stdout: File + built-in stdout (no sidecar).
      • (Optional) Legacy stdout: File + stdout with sidecar, if you prefer to retain it.
  2. Redirecting Tetragon Logs to stderr:

    • I introduced stderr for Tetragon internal logs to align with Kubernetes logging best practices, where application logs (stderr) are separated from structured data (stdout, e.g., JSON events). This allows users to filter events easily (e.g., kubectl logs | grep '^EVENT:') while accessing Tetragon logs via kubectl logs --stderr.
    • Proposal: If you prefer reverting to the previous behavior (Tetragon logs on stdout), I can undo this change. However, I believe redirecting to stderr is a more standard approach, especially for direct-stdout and file+stdout modes, as it avoids mixing logs and events. I’d appreciate your thoughts on whether to keep stderr or revert.

Looking forward to your feedback!

@lambdanis Any comments?

@ghost
Copy link

ghost commented May 7, 2025

@cy83rc0llect0r Thank you for the clarification and the wait :)

Preserving All Export Configurations

IMO it's fine to deprecate the stdout mode. It was intended mainly for debugging/demos, and depends on another image. Keeping only the built-in stdout export would (a) make its purpose clearer (b) remove the dependency. Although I wouldn't commit to removing it yet - instead I would highlight the deprecation in the release/upgrade notes, in case anyone has feedback against the removal.

Regarding file export. What's the behaviour if the export mode is set to file, but the export file name or directory is not specified? I think we should keep the "file + built-in stdout (no sidecar)" option, and maybe use the config fields similarly to how they're used now - the export mode would configure whether the stdout export is enabled, and the export file name/directory would configure whether the file export is enabled. WDYT?

Redirecting Tetragon Logs to stderr

I thought about it again, and I'm leaning towards reverting the change. The thing is, most of Tetragon logs are not errors, and users might have tooling (e.g. a logs collector) it place that assumes stderr is all errors. This change might have unexpected consequences down the pipeline, while with the EVENT: prefix, there is an easy way to filter out either events or Tetragon logs.

Add export-mode flag to support none, file, stdout, and file+stdout modes, defaulting to file for backward compatibility.

Enhancement cilium#1710

Signed-off-by: Amir Reza Nazarizadeh <[email protected]>
Update startExporter to support file and direct-stdout modes, deprecating the stdout mode.
For direct-stdout, use JSONStdoutEncoder in a goroutine to ensure non-blocking event processing.
For file mode, retain existing lumberjack.Logger logic.
Add fallback to file for invalid modes.

Enhancement cilium#1710

Signed-off-by: Amir Reza Nazarizadeh <[email protected]>
Update Helm chart to support export.mode (stdout, file and direct-stdout), with conditional sidecar and volume management.
Default to stdout mode for backward compatibility.

Enhancement cilium#1710

Signed-off-by: Amir Reza Nazarizadeh <[email protected]>
Enhancement cilium#1710

Signed-off-by: Amir Reza Nazarizadeh <[email protected]>
@cy83rc0llect0r cy83rc0llect0r force-pushed the pr/cy83rc0llect0r/add-stdout-support-without-file branch from 215be57 to cf911b3 Compare May 8, 2025 16:00
Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

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

I'll join Anna here and also it seems the PR needs rebase now, thanks!

@mtardy mtardy added the needs-rebase This PR needs to be rebased because it has merge conflicts. label May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase This PR needs to be rebased because it has merge conflicts. release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants