mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 08:09:27 +00:00
Design at: #13822 ## Summary `distinct_values_*` metadata streams existed to serve one reader: the `_values` API. They are a redundant — and lossier — materialization of what the tantivy secondary index already holds, and the index has the collectors needed to replace them outright (`SimpleTopN` / `SimpleDistinct`). This PR removes the distinct streams from the write and read paths and lets the `_values` API be served by the tantivy index optimizer. ### 1. Treat distinct value fields as secondary index fields (read-time union) - `get_stream_setting_index_fields()` — the single accessor used by both the index build side (ingester parquet/pack jobs, compactor merge) and the query side (leader/follower index optimizers) — now unions `distinct_value_fields` into the effective index field set, gated on `enable_distinct_fields`. Fields that cannot be secondary-indexed (full text search keys, partition keys, reserved columns) are skipped; the values API falls back to a normal scan for those. - The distinct list is deliberately **not merged into `settings.index_fields`**: the two stay independent, so removing a distinct value field (or disabling `enable_distinct_fields`) cleanly stops its indexing with no residue, and the user-managed `index_fields` list is never mutated behind their back. - Correctness gate: a distinct-only field is considered indexed since its `added_ts` (`get_stream_setting_index_updated_at_for_fields`), so parquet files older than that are served by DataFusion scan instead of the index (`split_file_list_by_time_range`). `added_ts` is set on add and reset when `enable_distinct_fields` is re-enabled (existing behavior). For streams configured before this release the fields only start being indexed at upgrade time, so `added_ts` is additionally clamped by a release-wide cutoff `/tantivy/distinct_fields/updated_at` — a lazy marker initialized to the first access after upgrade, following the existing `ttv_secondary_index_updated_at` pattern. No data migration and no `DB_SCHEMA_VERSION` bump needed. ### 2. `_values` API always queries the original stream - Removed `can_use_distinct_stream()` and the `distinct_values_*` / `SUM(count)` / `StreamType::Metadata` rewrite from `values_inner` (GET `/_values`, formerly `values_v1`) and `build_search_request_per_field` (POST `/_values_stream`). - The generated SQL shapes are exactly what the index optimizer detects: - `SELECT f AS zo_sql_key, COUNT(*) AS zo_sql_num FROM s GROUP BY zo_sql_key ORDER BY zo_sql_num DESC, zo_sql_key ASC` + limit → `SimpleTopN` - `no_count`: `SELECT f AS zo_sql_key FROM s GROUP BY zo_sql_key ORDER BY zo_sql_key ASC` + limit → `SimpleDistinct` - The `keyword` filter (`str_match_ignore_case`) is a valid index condition, so keyword-filtered values queries stay on the index path too. ### 3. Remove distinct values generation - Dropped the per-record distinct value collection in logs and traces ingestion and the whole `core::metadata` module (channel, mem-table, flush job, shutdown hook). - Removed the now-unused `DISTINCT_FIELDS` static and the `ZO_FEATURE_DISTINCT_EXTRA_FIELDS` / `ZO_DISTINCT_VALUES_INTERVAL` / `ZO_DISTINCT_VALUES_HOURLY` config knobs, including the `SUM(count)` merge rollup the last one gated — legacy `distinct_values_*` streams now compact as ordinary metadata streams. The default fields `DISTINCT_FIELDS` carried (`service_name`, `operation_name`) are already default secondary index fields via `SQL_SECONDARY_INDEX_SEARCH_FIELDS`, so traces `_values` queries on them remain index-served. - Renamed `values_v1` to `values_inner`: the `v2` variant was removed long ago (#5087) and the distinct stream switch inside `v1` is gone now too, so the version suffix was pure history. ### 4. Default fields after this change - `service_name`, `operation_name`: default secondary index fields (unchanged, via `SQL_SECONDARY_INDEX_SEARCH_FIELDS`). - `trace_id`: now also a built-in default **bloom filter** field (`BLOOM_FILTER_DEFAULT_FIELDS`, gated on `ZO_FEATURE_DEFAULT_INDEX_FIELDS_ENABLED`), in addition to being a default index field — bloom sits on top of the secondary index and prunes files for high-cardinality equality lookups. - There is no "default distinct fields" concept anymore. ### 5. Fix surfaced by e2e: SimpleDistinct returned null values as `""` The tantivy index encodes null/missing field values as empty strings (`index_builder` adds `""` for null docs), so the `SimpleDistinct` term-dictionary walk returned `""` as a distinct value — sorted first in ascending order, it surfaced as a blank first option in dashboard variable dropdowns, which the DataFusion `GROUP BY` path never returns (its null group sorts last). This was exposed once distinct fields became index-served; `handle_simple_distinct` now skips empty terms (with a unit test). ### 6. Drive-by: fix the failing `api_fuzz_tests` CI job `CreateAlertRequestSchema` was an untagged documentation union with two identical variants, producing `oneOf: [CreateAlertRequestBody, CreateAlertRequestBody]` in the OpenAPI spec. Under `oneOf` semantics (exactly one branch must match) no instance can ever validate, so schemathesis filtered out every generated example and `api_fuzz_tests` failed on `POST /api/v2/{org_id}/alerts` on every CI run since that schema landed. The handler deserializes `CreateAlertRequestBody` directly, so the annotation now documents exactly that type; runtime behavior is unchanged. ## What stays (intentionally) - `distinct_value_fields` / `enable_distinct_fields` in stream settings: still the user-facing way to declare values-API fields; the list now drives the read-time index-field union described above. - The `distinct_value_fields` DB table that tracks dashboard/report usage of these fields (still gates removal in the settings API). - `DISTINCT_STREAM_PREFIX` helpers, retention sync and stream-deletion cleanup for existing `distinct_values_*` streams — legacy stream data keeps compacting (as ordinary metadata streams) and ages out via retention; no new records are written to them. ## Testing - `cargo check --workspace --all-targets` and `cargo clippy --workspace --all-targets` clean. - Enterprise build verified with `Cargo.toml.openobserve` (`cargo check --all-targets`). - New unit tests for the index-field union (skip rules for FTS/partition/reserved fields, `enable_distinct_fields` gating) and the `added_ts` / release-cutoff indexed-since resolution; schema, infra, config and api-search unit tests pass. A few pre-existing failures on a clean tree (config `ssrf_guard`, one flaky sqlite-backed destinations test, one `.env`-dependent index-fields test) are local-environment issues and unrelated. Closes #13822
OpenObserve API Tests
End-to-end pytest suite against the OpenObserve REST API. Runs in CI on every
PR (.github/workflows/api-testing.yml); also runnable locally.
Quick start
# one-time setup
brew install rye # or: curl -sSf https://rye.astral.sh/get | bash
rye sync # install Python + deps from requirements.lock
cargo build # build the OO debug binary (~10 min first time)
# every run
ZO_ROOT_USER_EMAIL=root@example.com \
ZO_ROOT_USER_PASSWORD='Complexpass#123' \
ZO_BASE_URL=http://localhost:5080/ \
WS_ZO_BASE_URL=ws://localhost:5080/ \
target/debug/openobserve &
rye run pytest # full suite
rye run pytest -m framework # just the new-framework self-tests
rye run pytest tests/test_kvstore.py -v # one file
Layout
tests/api-testing/
├── pyproject.toml # pytest + ruff config
├── conftest.py # legacy session-wide fixtures (do not extend)
├── support/ # NEW FRAMEWORK — write new tests against this
│ ├── client.py # OpenObserveClient: HTTP entry point
│ ├── endpoints/ # one wrapper class per resource group
│ │ ├── search.py # client.search.sql(...), .hits(...), .count(...)
│ │ ├── streams.py # client.streams.list(), .delete(...), .ingest_json(...)
│ │ ├── users.py
│ │ ├── dashboards.py
│ │ ├── alerts.py
│ │ ├── folders.py
│ │ └── kvstore.py
│ ├── factories.py # payload builders (user_payload, dashboard_payload, …)
│ ├── wait.py # wait_until(predicate, timeout, ...) — REPLACES time.sleep
│ └── fixtures.py # client, temp_stream_name, temp_user_email, …
├── tests/ # test files
│ ├── conftest.py
│ ├── test_framework_selfcheck.py # 14 tests that prove support/ works
│ └── test_*.py # 40+ legacy test files (in revamp progress)
├── helpers/ # non-test shared code
│ ├── sourcemap_helpers.py
│ └── workflow/ # workflow chain helpers (with embedded pages/)
├── fixtures/ # static test data
│ └── rum/ # RUM ingest scripts
└── README.md # this file
Writing a new test — the rules
Use the new framework. Don't copy-paste from old test_*.py files; they
are scheduled for rewrite (see revamp plan).
# tests/<area>/test_my_feature.py
from support.client import OpenObserveClient
from support.factories import user_payload
from support.fixtures import client, temp_user_email # noqa: F401
from support.wait import wait_until
def test_create_user_returns_complete_record(client: OpenObserveClient, temp_user_email: str):
"""Create a user — verify the response has the email AND role we sent."""
payload = user_payload(email=temp_user_email, role="admin")
resp = client.users.create(payload)
assert resp.status_code == 200, resp.text
body = resp.json()
# VALIDATE THE BODY, not just status_code
assert body.get("data", {}).get("email") == temp_user_email
assert body.get("data", {}).get("role") == "admin"
# cleanup happens automatically via temp_user_email fixture
Mandatory patterns
| ✅ Do | ❌ Don't |
|---|---|
from support.fixtures import client |
session = create_session; url = base_url; org_id = "default" (307 occurrences in legacy code) |
client.streams.list() |
session.get(f"{url}api/{org_id}/streams") |
body = resp.json(); assert body["foo"] == ... |
assert resp.status_code == 200 and stop |
wait_until(lambda: client.search.count(sql) >= N, timeout=30) |
time.sleep(5) |
unique_email("user") / temp_user_email fixture |
hardcoded pytests@gmail.com |
logger.debug("got %s", resp.status_code) |
print(resp.content) |
start, end = time_window(minutes=15) |
hardcoded start_time = 1700629279639000 |
Anti-patterns that fail review
assert resp.status_code in [200, 400, 404, 500]— pick one. If the API behaves wrongly, file a bug; don't paper over it.pytest.skip(...)when the test discovers the bug it's meant to catch — use@pytest.mark.xfail(strict=True).- Commented-out test functions — delete them.
pages.*direct imports — that subtree is legacy. New tests usesupport.client.
Lint
rye run ruff check support/ tests/test_framework_selfcheck.py # strict mode
rye run ruff check --fix support/ # autofix
Existing legacy tests have a per-file ruff-ignore (so print etc. don't fail
the whole suite); new tests under support/ and any new well-formed test
file should pass strict ruff.
Test markers
| Marker | Meaning |
|---|---|
framework |
Self-tests for support/ framework. Run with -m framework. |
order(N) |
Run order pin (legacy — avoid in new tests). |
Troubleshooting
KeyError: 'ZO_BASE_URL'on collection —conftest.pyreads env vars at import. Export them before running pytest (even--collect-only).test_workflow.pyfails with SMTP error —ZO_SMTP_ENABLED=truenot set. CI does this; locally you'd need to wire an SMTP env.- Test_organisations.py warning about
yield—test_create_organizationuses pre-pytest-4 yield-style and is silently ignored. Scheduled for rewrite in Phase 4.
Revamp in progress
This suite is being incrementally reworked. See:
../../tests/ui-testing/MD_Files/api tests revamp/API_TESTS_REVAMP.md— the plan../../tests/ui-testing/MD_Files/api tests revamp/oss_rework_progress/— phase-by-phase progress