Skip to content

refactor(eventrecorder): move configuration to eventrecorder package#5189

Merged
siavashs merged 1 commit into
prometheus:mainfrom
TheMeier:evrecorder_config
Apr 24, 2026
Merged

refactor(eventrecorder): move configuration to eventrecorder package#5189
siavashs merged 1 commit into
prometheus:mainfrom
TheMeier:evrecorder_config

Conversation

@TheMeier
Copy link
Copy Markdown
Contributor

@TheMeier TheMeier commented Apr 24, 2026

Pull Request Checklist

Which user-facing changes does this PR introduce?

NONE

Summary by CodeRabbit

  • Refactor
    • Reorganized event recorder configuration to a dedicated module for improved code organization and maintainability.

Avoid import cycles when moving notifier configuration types from config
to their own package in the future.

Signed-off-by: Christoph Maser <christoph.maser+github@gmail.com>
@TheMeier TheMeier requested a review from a team as a code owner April 24, 2026 12:40
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

📝 Walkthrough

Walkthrough

Configuration for the event recorder is consolidated by moving type definitions from the config/ package to the eventrecorder/ package. A new eventrecorder/config.go file defines EventRecorderConfig and EventRecorderOutput with validation logic, while the main config/config.go is updated to reference external definitions instead of local duplicates.

Changes

Cohort / File(s) Summary
Type Definition Consolidation
config/config.go
Updated Config.EventRecorder field type from local EventRecorderConfig to eventrecorder.EventRecorderConfig; removed local type definitions EventRecorderConfig and EventRecorderOutput along with their UnmarshalYAML validation method.
New Eventrecorder Configuration Module
eventrecorder/config.go
New file defining EventRecorderConfig with outputs list and EventRecorderOutput with fields for file path, webhook URL, HTTP client config, timeout, workers, retry limits, and backoff. Implements UnmarshalYAML validation enforcing required fields per output type.
Eventrecorder Package Updates
eventrecorder/eventrecorder.go, eventrecorder/webhook.go
Updated function signatures (NewRecorderFromConfig, ApplyConfig, NewWebhookOutput) to use local EventRecorderConfig/EventRecorderOutput types; removed config package imports and internal type comparisons now reference package-local types.
Test Suite Updates
eventrecorder/eventrecorder_test.go, eventrecorder/webhook_test.go
Updated test references to use local EventRecorderConfig/EventRecorderOutput types instead of imported config package types; removed corresponding config package imports.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete; it only checks 2 of 7+ checklist items and omits critical sections like issue references, testing details, and performance impact disclosure. Complete the PR checklist by specifying if this is a bugfix/feature (with tests), whether it affects performance, if it's a breaking change, and add documentation updates. Link related issues if applicable.
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'refactor(eventrecorder): move configuration to eventrecorder package' accurately describes the main change—moving EventRecorderConfig types from config package to eventrecorder package.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
eventrecorder/config.go (1)

14-23: Duplicate package doc comment.

The same // Package eventrecorder ... doc block already exists at the top of eventrecorder/eventrecorder.go (lines 14–22). Having the package comment in two files risks future drift and is non-idiomatic; godoc will only surface one. Consider dropping it here (or consolidating both into a dedicated doc.go).

♻️ Proposed drop
-// Package eventrecorder provides a structured event recorder for
-// significant Alertmanager events.  Events are serialized as JSON and
-// fanned out to one or more configured destinations (JSONL file,
-// webhook, etc.).
-//
-// RecordEvent never blocks the caller: events are serialized and
-// placed on a bounded in-memory queue.  A background goroutine
-// drains the queue and sends to destinations.  If the queue is full,
-// events are dropped and a metric is incremented.
 package eventrecorder
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@eventrecorder/config.go` around lines 14 - 23, Remove the duplicate package
doc comment for package eventrecorder that appears in config.go; keep the
canonical package comment only in the existing eventrecorder.go (or move both
into a dedicated doc.go), and ensure you delete the top comment block beginning
"// Package eventrecorder ..." from config.go so godoc isn’t duplicated and the
package builds cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@eventrecorder/config.go`:
- Around line 14-23: Remove the duplicate package doc comment for package
eventrecorder that appears in config.go; keep the canonical package comment only
in the existing eventrecorder.go (or move both into a dedicated doc.go), and
ensure you delete the top comment block beginning "// Package eventrecorder ..."
from config.go so godoc isn’t duplicated and the package builds cleanly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4f64b2ef-8b8c-42ec-8797-1bc4526af239

📥 Commits

Reviewing files that changed from the base of the PR and between e3b2552 and 1da26e8.

📒 Files selected for processing (6)
  • config/config.go
  • eventrecorder/config.go
  • eventrecorder/eventrecorder.go
  • eventrecorder/eventrecorder_test.go
  • eventrecorder/webhook.go
  • eventrecorder/webhook_test.go

@siavashs siavashs merged commit c769adc into prometheus:main Apr 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants