mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 16:09:05 +00:00
Vamsi Karthik R
99b1a23773
Adds automated test coverage for **SLOs** (#13547, #13577, #13579, #13704, #13761, #13784), which shipped with none. **149 tests** — 77 Playwright + 72 pytest — all three SLI types, both query languages, happy / negative / edge paths. Clean branch off `main`. **UI (Playwright · `tests/ui-testing`)** - `pages/sloPages/` — 4 page objects (verified `data-test` selectors, hydration + Monaco-sync waits, OSelect commit-retry, virtualized-list filtering). - `playwright-tests/SLO/slo-crud.spec.js` (24) — create count/time-slice/grouped through the form, detail, edit, SLI-type change, tags, pause, delete, filters; negative: dismissed delete, empty search, cancel, unmeasured SLI is absent not zero. - `playwright-tests/SLO/slo-form-validation.spec.js` (20) — server errors surfacing in the form, conditional fields per SLI type, count + time-slice previews, **PromQL count and time-slice created end to end**; negative: no stream, invalid aggregate, over-long name, target boundaries. - `playwright-tests/SLO/slo-alert-sli.spec.js` (8) — **the `alert` SLI type**: source picker, create, config carries only the alert id, detail names the source; negative: ineligible source offered-but-disabled, grouped alert SLI, slice finer than source cadence. - `playwright-tests/SLO/slo-timeslice.spec.js` (10) — **#13761 P0 regression** (measured SLI, not the preview), PromQL, previews, gaps, language-flip clearing. - `playwright-tests/SLO/slo-alerts.spec.js` (15) — burn-rate + error-budget kinds, presets, edit, freeze door, burndown; negative: no destination, cancel, blank/invalid name. - `playwright-tests/utils/slo-seed.js` — backdated seeding, metrics seeding, eligible-source-alert fixture, ground-truth query, fail-fast env guard, transient-store retry, full fixture teardown (alerts → streams → destinations → templates). **API (pytest · `tests/api-testing`)** - `tests/slos/test_slo_validation.py` (72) — full REST surface (create/get/update/enable/delete/groups/slo-eligible), validation on **both create and update** (name, target, window, slice, config shapes, comparators, float-threshold round-trip), boundaries (target 0.001/99.999, name 256 vs 257, threshold 0/negative/1e12/NaN, unicode names), alert-source eligibility (silence-gated, too-infrequent, cron), PromQL config shapes, auth 401/404. **CI** - Two shards: `SLO` (`workers: 3`) and `SLO-Measurement` (`workers: 1`). Split because the backfill job runs with `ZO_SCHEDULER_SLO_BACKFILL_CONCURRENCY=1`, so measurement specs sharing an instance starve each other's waits — `fullyParallel` races spec *files*, which `mode:'serial'` cannot prevent. The 3-worker pin is SQLite: at 5, concurrent SLO/alert/destination writes trip `database is locked` on the single-node meta store. - New optional per-shard `ingest_allowed_upto` (`240`) alongside the existing `quick_mode_enabled`: the workflow default of 5 **hours** silently drops the 8 days of history an SLO window needs, while still answering `200`. Other shards keep the default. Same precedent as `api-testing.yml`'s `48`. **Feature code**: 9 additive `data-test`/`dataTest` attributes in `SloDetail.vue`, `SloList.vue`, `SloAlertCondition.vue` — no logic changes, `vue-tsc` clean. The stat tiles, tabs and burn-window inputs had no stable selectors; `OStatStrip`/`OTab` already forward the attribute. **Behaviour**: self-cleaning (`e2e_slo_*`, worker-scoped since `afterAll` runs per worker). **Verified in CI** on this branch: `e2e / SLO` **52 passed** (1.8m, workers pinned to 3), `e2e / SLO-Measurement` **25 passed** (11.3m, workers pinned to 1) — the measurement shard passing confirms the per-shard `ingest_allowed_upto` took effect, since those tests are impossible under the 5-hour default. pytest **72 passed** locally; in CI's 599-test run one setup delete hit a transient 500 (see findings) and now retries. **Findings for the feature owner** (reported, not fixed here): - A blank "good when" shows `Request failed with status code 422` — `pruned()` drops the empty key, so the API's real 400 message never reaches `save()`. - A rejected alert name shows no banner: `SloAlertForm.submit()` returns on `nameError` before setting `saveError`. - `avg(latency_ms); DROP TABLE users--` is accepted at create time (parsed only at measurement). - The name-length message says "less than 256 characters" but 256 is accepted — 257 is the first rejection. - Duplicate alert names on one SLO are accepted (duplicate SLO names are a 409), and the panel lists them by name, so they are indistinguishable. - SLO names accept characters that break the list's own `slos-slolist-edit-${name}` selectors; alert names correctly reject them. - All chart/preview root `data-test` ids are unreachable — a parent's fallthrough attribute overrides the child root's. - Both edit forms hydrate asynchronously, so a blank form can flash before the stored values appear. - `DELETE /slos/{id}` on a freshly-created SLO returned **500** once under CI load (not reproducible locally in 12 rapid iterations). Contention is arguably a retryable 503, and the body carried no actionable message. - **A scheduled alert whose source stream is deleted is retried forever** (`Stream "…" not found in schema`), and the SLO backfill lane shares the scheduler's worker budget — so accumulated dead alerts *starve SLO measurement*. Observed: 65 of them held coverage at 1 of 7 chunks indefinitely; deleting them restored 0.143 → 1.0 in 240s. Harmless in CI (fresh instance per shard) but it degrades any long-lived environment. The suite now cleans up after itself; the retry-forever behaviour itself may deserve a look. **Paired ENT PR: openobserve/o2-enterprise#2434.** ENT's matrix is this repo's manifest plus an overlay, merged by this repo's `build-ci-matrix.js`, so both SLO shards reach ENT automatically — verified: 31 merged shards, both SLO entries carrying `ingest_allowed_upto=240` and their worker pins, no other shard affected. What ENT was missing is *consuming* those two fields, which #2434 adds. Merge the two together: this one alone leaves ENT booting the SLO shards on the 5-hour ingest window, and #2434 alone is inert. *(An earlier review reported ENT as using an inline matrix with no SLO shard — that turned out not to be the case.)*