Skip to content

chore: Accept 204 from FluentD responses#57158

Closed
cgetzen wants to merge 2 commits intogravitational:masterfrom
cgetzen-forks:chore-fluentd-status-code-acceptance
Closed

chore: Accept 204 from FluentD responses#57158
cgetzen wants to merge 2 commits intogravitational:masterfrom
cgetzen-forks:chore-fluentd-status-code-acceptance

Conversation

@cgetzen
Copy link
Copy Markdown
Contributor

@cgetzen cgetzen commented Jul 25, 2025

Problem

Many orgs already have logging infrastructure in place, but do not use FluentD. In this case, Teleport Audit Event Logging requires teams to install and manage FluentD in addition to existing infra.

Solution

This relaxes the status code validation for FluentD responses, in order to allow other log aggregators to accept the event handlers requests.

The changes in this PR allow Grafana Alloy to be used as a replacement for FluentD, by using the loki.sources.api component and configuring the Teleport event handler to point url and session-url to Alloy's /loki/api/v1/raw endpoint.

This change should accommodate any Log Collectors that support raw JSON and emit 200 or 204 status codes.

Without these changes in place, logs get duplicated in our logging infra:

Stack Trace:
        github.com/gravitational/teleport/integrations/event-handler/fluentd_client.go:134 main.(*FluentdClient).Send
        github.com/gravitational/teleport/integrations/event-handler/app.go:116 main.(*App).SendEvent
        github.com/gravitational/teleport/integrations/event-handler/events_job.go:313 main.(*EventsJob).sendEvent
        github.com/gravitational/teleport/integrations/event-handler/events_job.go:288 main.(*EventsJob).handleEvent
        github.com/gravitational/teleport/integrations/event-handler/events_job.go:282 main.(*EventsJob).handleEventV2
        github.com/gravitational/teleport@v0.0.0/lib/events/export/date_exporter.go:599 github.com/gravitational/teleport/lib/events/export.(*DateExporter).exportEvents
        github.com/gravitational/teleport@v0.0.0/lib/events/export/date_exporter.go:456 github.com/gravitational/teleport/lib/events/export.(*DateExporter).processChunk
        github.com/gravitational/teleport@v0.0.0/lib/events/export/date_exporter.go:433 github.com/gravitational/teleport/lib/events/export.(*DateExporter).startProcessingChunk.func2
        runtime/asm_amd64.s:1700 runtime.goexit
User Message: Failed to send event to fluentd (HTTP 204)] attempts:5 event-handler/app.go:133

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 25, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@github-actions github-actions bot requested review from jimbishopp and r0mant July 25, 2025 04:00
@programmerq
Copy link
Copy Markdown
Contributor

From the docs for the Fluentd HTTP input plugin:

use_204_response bool false
Respond status code with 204. This option will be deprecated at v2 because fluentd v2 will respond 204 as default.

It seems future versions of Fluentd will also respond with a 204, so we should definitely allow at least 200 and 204. I'll let our devs comment on whether we want to accept all 2XX response codes or not.

@cgetzen
Copy link
Copy Markdown
Contributor Author

cgetzen commented Aug 1, 2025

@programmerq nice find. This has been adjusted to only accept 200 and 204.

@programmerq programmerq self-requested a review August 1, 2025 17:18
Copy link
Copy Markdown
Contributor

@programmerq programmerq left a comment

Choose a reason for hiding this comment

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

LGTM

@programmerq programmerq added no-changelog Indicates that a PR does not require a changelog entry event-handler labels Aug 1, 2025
@cgetzen
Copy link
Copy Markdown
Contributor Author

cgetzen commented Aug 1, 2025

recheck

@cgetzen
Copy link
Copy Markdown
Contributor Author

cgetzen commented Aug 1, 2025

You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot

I am not sure if this, or the "CLA Assistant" CI check, is working correctly.

@cgetzen cgetzen changed the title chore: Accept all 2xx from FluentD responses chore: Accept 204 from FluentD responses Aug 4, 2025
@bernardjkim
Copy link
Copy Markdown
Contributor

bernardjkim commented Aug 5, 2025

Hey, change looks good to me. I'll go ahead and approve the GHA workflows and add the PR to the merge queue. I'll also follow up with the backport PRs.

Edit: Seems like our CI pipeline is having some issues with external contributions. I've created a buddy PR with your commits here: #57581.

@hugoShaka
Copy link
Copy Markdown
Contributor

Merged in: #57581

@hugoShaka hugoShaka closed this Aug 8, 2025
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.

5 participants