Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions services/intake/.clickhouse-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
26.3
3 changes: 3 additions & 0 deletions services/intake/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ environment instead of package-scoped `uv run --package ...` commands.

Prerequisite: Docker must be installed and running locally.

Intake is tested and profiled on ClickHouse 26.3 LTS. Other ClickHouse versions
may not be supported.

Start a local ClickHouse container for span and trace storage:

```bash
Expand Down
15 changes: 14 additions & 1 deletion services/intake/scripts/spans/run_clickhouse.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
set -euo pipefail

container_name="nmp-intake-clickhouse"
image="${CLICKHOUSE_IMAGE:-clickhouse/clickhouse-server:24.3}"
clickhouse_user="${CLICKHOUSE_USER:-default}"
clickhouse_password="${CLICKHOUSE_PASSWORD:-}"
script_dir="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
repo_root="$(cd -- "${script_dir}/../../../.." && pwd)"
clickhouse_version="$(tr -d '[:space:]' < "${script_dir}/../../.clickhouse-version")"
image="${CLICKHOUSE_IMAGE:-clickhouse/clickhouse-server:${clickhouse_version}}"
data_dir="${CLICKHOUSE_DATA_DIR:-${repo_root}/tmp/intake-clickhouse}"

ensure_host_dirs() {
Expand All @@ -21,13 +22,25 @@ ensure_tmp_dir() {
docker exec "${container_name}" sh -c "mkdir -p /var/lib/clickhouse/tmp && chown clickhouse:clickhouse /var/lib/clickhouse/tmp" >/dev/null
}

ensure_expected_image() {
local actual_image
actual_image="$(docker inspect --format "{{.Config.Image}}" "${container_name}")"
if [[ "${actual_image}" != "${image}" ]]; then
echo "${container_name} uses ${actual_image}, but Intake requires ${image}." >&2
echo "Remove the existing container after preserving any data you need, then rerun this script." >&2
exit 1
fi
}

if docker ps --filter "name=^/${container_name}$" --filter "status=running" --format "{{.Names}}" | grep -qx "${container_name}"; then
ensure_expected_image
ensure_tmp_dir
echo "${container_name} is already running"
exit 0
fi

if docker ps -a --filter "name=^/${container_name}$" --format "{{.Names}}" | grep -qx "${container_name}"; then
ensure_expected_image
ensure_host_dirs
docker start "${container_name}" >/dev/null
ensure_tmp_dir
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ async def list_sessions(
if needs_pre_metrics:
# Two-query path for cost/tokens sorts.
#
# ClickHouse 24.3 inlines CTEs (does not materialise them), so a single query that
# ClickHouse inlines regular CTEs (does not materialise them), so a single query that
# references `page_sessions` from multiple downstream CTEs (current_page_spans,
# session_metrics, session_scores, final SELECT) would re-execute the expensive
# all-session span aggregation once per reference. Splitting into two queries
Expand Down Expand Up @@ -340,7 +340,7 @@ def _metric_sort_page_ids_sql(
ORDER BY + LIMIT/OFFSET to return the ordered (workspace, session_id) pairs for
the requested page. Only IDs are returned — row hydration is a separate query.

Why separate: ClickHouse 24.3 inlines CTEs rather than materialising them, so a
Why separate: ClickHouse inlines regular CTEs rather than materialising them, so a
single query that references `page_sessions` from multiple CTEs would re-execute
the expensive all-session span aggregation once per reference. Returning IDs here
and hydrating in _hydrate_by_ids_sql ensures the aggregation runs exactly once.
Expand Down
11 changes: 10 additions & 1 deletion services/intake/tests/integration/spans/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from collections.abc import Callable
from datetime import datetime, timezone
from importlib.util import find_spec
from pathlib import Path
from typing import Any
from uuid import uuid4

Expand All @@ -23,6 +24,9 @@
)
from nmp.testing import create_test_client

_CLICKHOUSE_VERSION_FILE = Path(__file__).resolve().parents[3] / ".clickhouse-version"
CLICKHOUSE_VERSION = _CLICKHOUSE_VERSION_FILE.read_text(encoding="utf-8").strip()


def _run(coro: Any) -> Any:
return asyncio.run(coro)
Expand Down Expand Up @@ -55,14 +59,19 @@ def clickhouse_container():
from testcontainers.clickhouse import ClickHouseContainer

with ClickHouseContainer(
"clickhouse/clickhouse-server:24.3",
f"clickhouse/clickhouse-server:{CLICKHOUSE_VERSION}",
username="test",
password="test",
dbname="default",
) as container:
yield container


@pytest.fixture(scope="session")
def clickhouse_version() -> str:
return CLICKHOUSE_VERSION


@pytest.fixture(scope="session")
def clickhouse_settings(clickhouse_container) -> ClickHouseSettings:
return ClickHouseSettings(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@
from nmp.intake.spans.clickhouse_client import ClickHouseSpanClient, bootstrap_schema


def test_clickhouse_server_matches_supported_lts(
clickhouse_client: ClickHouseSpanClient,
clickhouse_version: str,
run_async,
) -> None:
result = run_async(clickhouse_client.query("SELECT version()"))

assert len(result.result_rows) == 1
assert str(result.result_rows[0][0]).startswith(f"{clickhouse_version}.")


def test_clickhouse_bootstrap_is_idempotent(clickhouse_client: ClickHouseSpanClient, run_async):
run_async(bootstrap_schema(clickhouse_client))
run_async(bootstrap_schema(clickhouse_client))
Expand Down
Loading