Skip to content

feat(opensearch): support reason-filtered, unscoped, paginated event queries - #384

Draft
LakshanSS wants to merge 1 commit into
openchoreo:mainfrom
LakshanSS:delivery-insights-events
Draft

feat(opensearch): support reason-filtered, unscoped, paginated event queries#384
LakshanSS wants to merge 1 commit into
openchoreo:mainfrom
LakshanSS:delivery-insights-events

Conversation

@LakshanSS

Copy link
Copy Markdown
Contributor

Summary

Delivery Insights (DORA metrics, openchoreo/openchoreo#4248) needs the observer's aggregator to sweep controller-emitted delivery lifecycle events (DeploymentStarted/Succeeded/Failed/Recovered) across every namespace on a timer, via this module's events API. The shared adapter contract (observability-logs-adapter-api.yaml) has been extended with reasons, searchAfter/nextCursor, and an optional searchScope, but this module's QueryEvents handler still required a scope and had no reason filter or pagination beyond a single page — every aggregator call would 400.

This is scoped to the observability-logs-opensearch module only, to unblock verifying the Delivery Insights event pipeline end-to-end on local k3d. The same gap exists in aws-cloudwatch, gcp-cloudlogging, and openobserve (tracked as a separate follow-up issue for the full release).

  • Allow searchScope to be omitted when reasons is set, running an unscoped sweep across all namespaces instead of 400ing.
  • Add a reasons terms filter to the component, workflow, and new unscoped query builders.
  • Add searchAfter/nextCursor pagination: sort now includes _seq_no as a tiebreaker (a numeric, always-doc-valued metadata field, unlike _id) so paging is stable when multiple events share a timestamp; nextCursor is set from the last hit's sort values when a page fills up.
  • Regenerate models.gen.go/server.gen.go from the openchoreo laki-obs branch's updated observability-logs-adapter-api spec (not yet on main, since feat: add Delivery Insights (DORA metrics) store and read API to observer openchoreo#4248 hasn't merged) so the new request/response fields exist. The regen was done from the module's own last-generated spec snapshot patched with only the new fields, to keep the diff scoped — not a full resync against current upstream main.
  • Fixed Client.Search not copying each hit's sort values through to the local Hit type, which the new cursor logic depends on.

Draft because it depends on openchoreo/openchoreo#4248 landing first (that PR is intentionally held open until the whole Delivery Insights feature — including this — is ready), and the generated code here will need re-syncing against main's spec once that merges.

Test plan

  • go build ./...
  • go vet ./...
  • go test ./... (all packages pass; existing TestQueryEvents_*/TestBuildComponentEventsQuery_* tests updated for the SearchScope pointer change and new sort shape)
  • New tests: unscoped reason-filtered sweep, missing-scope-without-reasons 400, invalid searchAfter cursor 400, nextCursor populated when a page fills up, cursor encode/decode round-trip, reasons terms filter on all three query builders
  • End-to-end verification on local k3d against a real OpenSearch cluster (pending #4248 merge + INSIGHTS_EVENTS_SOURCE_ENABLED=true)

🤖 Generated with Claude Code

…queries

Delivery Insights (DORA metrics, openchoreo/openchoreo#4248) needs the
observer's aggregator to sweep controller-emitted delivery lifecycle events
(DeploymentStarted/Succeeded/Failed/Recovered) across every namespace on a
timer, via the logs adapter's events API. The adapter's contract has been
extended with reasons, searchAfter/nextCursor, and an optional searchScope,
but this module's QueryEvents handler still required a scope and had no
reason filter or pagination beyond a single page.

- Allow searchScope to be omitted when reasons is set, running an unscoped
  sweep across all namespaces instead of 400ing.
- Add a reasons terms filter to the component, workflow, and new unscoped
  query builders.
- Add search_after/nextCursor pagination: sort now includes _seq_no as a
  tiebreaker (a numeric, always-doc-valued metadata field, unlike _id) so
  paging is stable when multiple events share a timestamp; nextCursor is
  set from the last hit's sort values when a page fills up.
- Regenerate models.gen.go/server.gen.go from the openchoreo laki-obs
  branch's updated observability-logs-adapter-api spec (not yet on main,
  since PR #4248 hasn't merged) so the new request/response fields exist.

Client.Search wasn't copying each hit's sort values through to the local
Hit type, which the new cursor logic depends on.

Signed-off-by: LakshanSS <lakshan230897@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e73f870e-b97a-413b-9652-014b9b93f53c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ 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.

@codecov-commenter

Copy link
Copy Markdown

@LakshanSS

Copy link
Copy Markdown
Contributor Author

Heads-up on the ordering dependency: the spec side of this is now split out of openchoreo/openchoreo#4248 into its own small PR — openchoreo/openchoreo#4597 (48 lines, spec + regenerated client, no Delivery Insights behaviour).

Once #4597 lands on main, this module's codegen will pick up reasons, searchAfter/nextCursor and the optional searchScope, and this PR can come out of draft without waiting for the full #4248 review.

#4597 cites the enforcement in this PR — the 400 for an unscoped, reason-less query and TestQueryEvents_NoScope_NoReasons — as the answer to why the rule isn't expressed as a schema-level anyOf.

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