Skip to content

feat(eventrecorder): add outputs name#5393

Open
siavashs wants to merge 1 commit into
prometheus:mainfrom
siavashs:fix/eventrecorder-metric-label-values
Open

feat(eventrecorder): add outputs name#5393
siavashs wants to merge 1 commit into
prometheus:mainfrom
siavashs:fix/eventrecorder-metric-label-values

Conversation

@siavashs

@siavashs siavashs commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Require every output to have a safe, unique name and use only the type-qualified name in metrics and logs. Avoid exposing file paths, webhook URLs, Kafka brokers, topics, and raw transport errors.

Which user-facing changes does this PR introduce?

[CHANGE] eventrecorder: All outputs now require a configured `name`, used instead of paths, URLs, brokers, topics, or other destination configuration in metric label values and logs.

@siavashs
siavashs requested a review from a team as a code owner July 17, 2026 11:31
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Event recorder outputs now require validated configured names. Type-qualified names are used for metrics, logs, output construction, duplicate detection, and configuration equality across file, webhook, Kafka, and stdout destinations.

Changes

Event recorder output naming

Layer / File(s) Summary
Output name contract and validation
eventrecorder/config.go, eventrecorder/*_output.go
Adds required name validation, type-qualified identifiers, duplicate detection, and name-aware configuration equality.
Destination identity and construction
eventrecorder/file.go, eventrecorder/webhook.go, eventrecorder/kafka.go, eventrecorder/stdout.go
Constructors and destination Name() methods now use configured names instead of paths, URLs, brokers, topics, or constants.
Recorder wiring and logging
eventrecorder/recorder.go, eventrecorder/file.go, eventrecorder/webhook.go, eventrecorder/kafka.go
Output creation uses validated identifiers, stdout entries are initialized through its constructor, and logs omit sensitive destination details and raw errors.
Tests and documentation
eventrecorder/*_test.go, config/config_test.go, docs/configuration.md, CHANGELOG.md
Updates fixtures and assertions for required names, duplicate validation, equality, identifier generation, log redaction, configuration examples, and changelog text.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description has a summary and release notes, but it omits the required checklist sections and issue/sign-off details from the template. Add the PR checklist sections from the template, include applicable issue links and status checkboxes, and state docs/sign-off details.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and clearly describes the main change: adding configured names for event recorder outputs.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@siavashs
siavashs marked this pull request as draft July 17, 2026 13:56
@siavashs

Copy link
Copy Markdown
Contributor Author

we actually need to fix this for other outputs too, for example the secret url from webhook leaks into metrics and logs.

Require every output to have a safe, unique name and use only the
type-qualified name in metrics and logs. Avoid exposing file paths,
webhook URLs, Kafka brokers, topics, and raw transport errors.

Signed-off-by: Siavash Safi <siavash@cloudflare.com>
@siavashs
siavashs force-pushed the fix/eventrecorder-metric-label-values branch from 3d8503f to 7953a47 Compare July 18, 2026 12:04
@siavashs siavashs changed the title fix(eventrecorder): kafka outputs name feat(eventrecorder): add outputs name Jul 18, 2026
@siavashs
siavashs marked this pull request as ready for review July 18, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant