Skip to content

feat(plugins/playground): return all matching alerts + drop ephemeral workdir volume#2347

Merged
JocLRojas merged 2 commits into
release/v12.0.0from
feature/playground-multi-alert-output
Jul 15, 2026
Merged

feat(plugins/playground): return all matching alerts + drop ephemeral workdir volume#2347
JocLRojas merged 2 commits into
release/v12.0.0from
feature/playground-multi-alert-output

Conversation

@JocLRojas

Copy link
Copy Markdown
Contributor

Summary

Two related fixes to the playground plugin so the POST /playground/run
flow can actually surface everything the event-processor produces, and so the
ephemeral workdir stops leaking state between container restarts.

Reference: internal discussion, no tracked issue.

Changes

1. feat(plugins/playground): return all matching alerts with moving-window collection

What was broken. The run endpoint returned a single alert per event
even when multiple rules matched the same event. The correlation writer
(saw plugin in EventProcessor) already appends one NDJSON line per
alert to resulting_alert.json, but the consumer side of the pipeline
threw away everything past the first match:

  • RunResult.Alert was a singular json.RawMessage, so the struct
    physically could not carry more than one alert.
  • findAlertByEventID in scan.go short-circuited on the first line
    matching the event id.
  • awaitAlert waited a fixed AlertGrace (2s) after the event was
    seen and then returned, so alerts that arrived after that window
    were dropped even if the file contained them.

What changed.

  • RunResult.Alert (json.RawMessage) → Alerts ([]json.RawMessage).
    Breaking change in the JSON response: the alert field is gone
    and replaced by alerts.
  • New RunResult.StopReason field: quiet | hard_cap | no_alerts.
    Lets the client tell "these are all the alerts" (quiet) apart from
    "we cut the wait short, more may be pending" (hard_cap).
  • findAlertByEventIDfindAlertsByEventID — collects every
    matching line and dedupes by alert.id across polls via a
    caller-owned seen set.
  • awaitAlertawaitAlerts: moving quiet window bounded by an
    absolute hard cap:
    • Quiet window (AlertGrace, 2s) resets on every new alert
      observed, so bursts of alerts are not truncated mid-burst.
    • Hard cap (EventTimeout, 15s) is the absolute per-run deadline
      so a noisy rule cannot hold the HTTP request open indefinitely.
    • If no alert arrives before the initial AlertGrace elapses,
      we return no_alerts — same behavior as before for the empty
      case.

Why moving window instead of a fixed grace. A fixed grace fails
when the correlation pipeline is slow: multiple rules matching the same
event can produce alerts spread out over more than 2s. The moving
window adapts to the actual arrival rate; the hard cap prevents
pathological cases from hanging the request.

2. feat(installer): drop ephemeral playground-workdir volume

The playground workdir only needs to survive the lifetime of the
event-processor container: input/, resulting_log.json and
resulting_alert.json are all truncated at the start of every run,
and nothing is expected to persist across restarts.

…ow collection

The run endpoint previously returned a single alert per event even when
multiple rules matched. The correlation writer (saw plugin) already emits
one NDJSON line per alert, but the runner's awaitAlert stopped at the
first hit and RunResult carried a singular Alert field.

Changes:
- RunResult.Alert (json.RawMessage) -> Alerts ([]json.RawMessage).
- Add RunResult.StopReason: quiet | hard_cap | no_alerts so the client
  can distinguish a drained pipeline from a truncated wait.
- Replace findAlertByEventID (early return) with findAlertsByEventID,
  which collects every matching line and dedupes by alert id across
  polls via a caller-owned seen set.
- Rework awaitAlerts to use a moving quiet window (AlertGrace) reset on
  every new alert, bounded by an absolute hard cap (EventTimeout) so a
  noisy rule cannot hold the HTTP request open.
- Iterate NDJSON with strings.SplitSeq to avoid allocating a full
  []string on each of the ~150 polls per run.
The playground plugin only needs the workdir alive while the container
runs: input logs, resulting_log.json and resulting_alert.json are all
truncated on every run and no state is expected to survive a container
restart. Mounting a host bind for it forces old config.yaml files to
persist across upgrades and hides fresh defaults written by the
playground -init step.

Remove the /playground-workdir bind so the directory lives inside the
event-processor container's tmpfs / overlay. If the event-processor
restarts we lose the workdir, which is the intended behavior.
@github-actions

Copy link
Copy Markdown

🛑 AI review — Sensitive area, extra care recommended

This PR touches critical paths or introduces changes the model cannot judge with sufficient confidence. Review carefully before merging.

⚠️ architecture (gemini-3-flash-lite) — minor findings

Summary: Refactored playground runner logic to support multiple alerts; minor structural improvement needed for file scanning.

  • medium plugins/playground/internal/runner/scan.go:25 — The scanLines function is no longer used by the new findAlertsByEventID implementation. Please remove the unused helper to reduce code debt.
  • low plugins/playground/internal/runner/scan.go:45 — Reading the entire file on every poll iteration (os.ReadFile) is inefficient for large log files. Consider using a buffered scanner or tracking file offsets if this plugin scales.

🛑 bugs (gemini-3-flash-lite) — high/critical — please review

Summary: Potential resource leak and logic error in file reading within the playground runner.

  • high plugins/playground/internal/runner/scan.go:50 — The function findAlertsByEventID calls os.ReadFile on every tick of the ticker inside the loop in awaitAlerts. This will cause significant I/O overhead and potential file descriptor exhaustion or performance degradation as the log file grows.
  • medium plugins/playground/internal/runner/runner.go:184 — The return value for ctx.Err() in awaitAlerts returns an empty string for StopReason, which may be unexpected by callers expecting a valid StopReason enum value.

🛑 security (gemini-3-flash-lite) — high/critical — please review

Summary: Potential path traversal vulnerability in file reading operations within the playground runner.

  • medium plugins/playground/internal/runner/scan.go:23 — The function scanLines (and subsequently findAlertsByEventID) reads files from a path provided as an argument without validating that the path is restricted to the intended playground directory, potentially allowing path traversal if the 'path' variable is influenced by user input.

🔴 go-deps — pending updates

🔍 Discovered 26 Go projects

📦 Dependencies with updates available:

  📁 ./backend:
     - cloud.google.com/go/pubsub: v1.50.4 → v1.51.0

  📁 ./plugins/gcp:
     - cloud.google.com/go/pubsub: v1.50.4 → v1.51.0

❌ Please update dependencies before merging.

@JocLRojas JocLRojas merged commit d86ff41 into release/v12.0.0 Jul 15, 2026
1 check passed
@JocLRojas JocLRojas deleted the feature/playground-multi-alert-output branch July 15, 2026 21:46
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.

1 participant