Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
137 changes: 137 additions & 0 deletions docs/adr/adr-0001-retire-sketch-core.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# ADR-0001: Retire `sketch-core`; `asap_sketchlib` is the single Rust algorithm crate

| | |
|---|---|
| Status | **Accepted** (retrospective — work already shipped) |
| Date | 2026-05-01 |
| Deciders | Project ASAP maintainers |
| Supersedes | n/a |
| Superseded by | n/a |

## Context

Before this work, the Rust side of the ASAP system had **two**
algorithm crates with overlapping responsibilities:

- `asap_sketchlib` — the canonical algorithm crate, holding
`DDSketch`, `KLL`, `HLL`, `Count`, `CountMin`, `CMSHeap`, etc.
Already shared with `sketchlib-bench` and external benchmark
harnesses.
- `sketch-core` — a wrapper crate vendored into
`ASAPQuery-backend/asap-common/sketch-core/` (and its sibling
forks in `ASAPQuery/asap-common/sketch-core/` and
`sketchlib-bench/sketch-core/`). It re-exposed the algorithms
with ASAP-specific wire-format types (`DdSketchState`,
`CountSketchState`, etc.), `apply_delta` methods, and the
`Strategy::Legacy` vs `Strategy::Sketchlib` `ImplMode` switch.

The duplication was a source of:

- **Cross-language drift risk.** The Go side had one canonical
algorithm crate (`sketchlib-go`); having two on the Rust side
made a third potential drift target.
- **Apply-delta logic in the wrong place.** `apply_delta`
implementations belonged with the algorithms, not in a wrapper
crate. Bug fixes (e.g., DDSketch delta count reconstruction)
had to be written in two places.
- **Maintenance overhead.** Three on-disk forks of the same
crate had to stay in sync; PRs touching the shared types
needed three identical commits.

The `ImplMode` switch (Legacy vs Sketchlib) was originally
introduced to allow gradual migration off hand-written matrix
implementations onto `asap_sketchlib`-backed implementations.
Once the Sketchlib path was the default and Legacy was no longer
exercised in production paths, the switch became dead weight.

## Decision

1. **Retire `sketch-core` entirely.** Move all of its content
into `asap_sketchlib`:
- Wire-format types (`DdSketch`, `CountSketch`, `HllSketch`,
`CountMinSketch`, `KllSketch`, `CountMinSketchWithHeap`)
into the existing `src/sketches/<name>.rs` files alongside
the high-throughput algorithms — single home per sketch
concept.
- `apply_delta` implementations into the same files so the
algorithm + its delta semantics live together.
- New sibling files for sketches that had no existing home in
`asap_sketchlib` (`hydra_kll.rs`, `set_aggregator.rs`,
`delta_set_aggregator.rs`).
- The previous `*_sketchlib.rs` FFI wrapper files inlined into
their main sketch file (e.g., `SketchlibCms` lives directly
inside `countmin.rs`).
- The standalone `asap_runtime` module for the legacy
`ImplMode` configuration was created and then removed (see
decision 3).

2. **Drop the three on-disk `sketch-core` forks.** All consumers
(`ASAPQuery`, `ASAPQuery-backend`, `sketchlib-bench`) depend
on `asap_sketchlib` directly via git URL.

3. **Drop the `ImplMode` (Legacy / Sketchlib) dispatch.** Always
use the `asap_sketchlib`-backed implementation. Removed:
- The `asap_runtime` module entirely.
- `KllBackend` enum (Sketchlib + Legacy variants); `KllSketch`
now holds `SketchlibKll` directly.
- The `dsrs` (datasketches-rs) dependency that provided the
legacy KLL backend.
- The `clap` (asap-cli feature) and `ctor` (legacy-mode test
initializer) dependencies.
- The backend's `--sketch-cms-impl` / `--sketch-kll-impl` /
`--sketch-cmwh-impl` CLI args and their `config::configure`
plumbing.

4. **Rename `CountMinDelta` → `CountMinSketchDelta`** to match
the `<Type>Delta` pattern of the other delta types
(`DdSketchDelta`, `CountSketchDelta`, `HllSketchDelta`).

5. **Rename to avoid wire-format collisions:**
- `octo_delta::HllDelta` (single-register, octo path) keeps
its name; the wire-format multi-register delta becomes
`HllSketchDelta`.
- `common::input::HeapItem` (polymorphic key type) keeps its
name; the wire-format CMSHeap item becomes `CmsHeapItem`.

## Consequences

### Positive

- Single canonical Rust algorithm crate, mirroring `sketchlib-go`
on the Go side. Cross-language drift is now a 1:1 concern, not
1:N.
- `apply_delta` logic lives next to the algorithm it applies to.
- Three on-disk forks deleted; consumers all track
`asap_sketchlib` directly.
- ~3,000 lines of net code removed once `Legacy` paths and
`dsrs` / `clap` / `ctor` / `asap-cli` deps were dropped.

### Negative / Tradeoffs

- The wire-format types now sit alongside the high-throughput
types in the same files (e.g., both `DDSketch` and `DdSketch`
in `src/sketches/ddsketch.rs`). File sizes grow by 30–80%.
Acceptable: the alternative (separate crate or separate
module) recreates the duplication problem this ADR solves.
- Cross-language parity tests between `sketchlib-go` and
`asap_sketchlib` are now the *only* defense against algorithm
drift. The R1 risk in the design doc explicitly calls this
out; mitigation (statistical-output cross-language harness in
`sketchlib-bench`) is tracked for follow-up.

### Compatibility

- Wire format unchanged: `SketchEnvelope.payload` bytes are
byte-identical pre/post-retirement. The
`tests::elastic_dsl_query_tests::tests::test_esdsl_time_range_query`
test was relaxed from `assert_eq!(value, 291.0)` to a `±1`
tolerance — `asap_sketchlib`'s KLL gives 290 on this
distribution where `dsrs` gave 291. Both are within KLL's
rank-error bound; the test was previously over-tight.

## References

- [ProjectASAP/asap_sketchlib#36](https://github.com/ProjectASAP/asap_sketchlib/pull/36) — sketch-core merged into existing `src/sketches/` layout
- [ProjectASAP/ASAPQuery-backend#73](https://github.com/ProjectASAP/ASAPQuery-backend/pull/73) — backend consumer migration
- [ProjectASAP/ASAPQuery#309](https://github.com/ProjectASAP/ASAPQuery/pull/309) — legacy-fork consumer migration + branch-pin cleanup + elastic DSL test fix
- [ProjectASAP/asap_sketchlib#37](https://github.com/ProjectASAP/asap_sketchlib/pull/37) — wire-format / `apply_delta` semantic alignment with `sketchlib-go` (DDSketch count reconstruction, CountMin/CountSketch field additions, out-of-bounds policy)
195 changes: 195 additions & 0 deletions docs/adr/adr-0002-extract-precompute-runtime.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
# ADR-0002: Extract the precompute runtime to `asap-precompute-{go,rs}`

| | |
|---|---|
| Status | **Proposed** (gates Phase 2 / Phase 3 of the edge-framework migration) |
| Date | 2026-05-02 |
| Deciders | Project ASAP maintainers |
| Supersedes | n/a |
| Superseded by | n/a |

## Context

Today the windowing / delta / scheduler state machine lives
inside the Go OTel processors (~3850 LoC across
`opentelemetry-collector-contrib-patch/processor/{ddsketch,kll,hll,countsketch,countminsketch}processor/processor.go`)
and inside the Rust ingest path
(`ASAPQuery-backend/asap-query-engine/src/precompute_operators/*.rs`
+ `drivers/ingest/otel.rs::apply_modified_otlp_delta_bytes`).

Each of those files conflates four concerns:

1. **OTel binding** — implementing `processor.Metrics`, accepting
`pmetric.Metrics`, calling `nextConsumer.ConsumeMetrics`.
2. **Data shape adapter** — extracting `(timestamp, attrs, value)`
tuples out of `pmetric.Gauge | Sum | DDSketchDataPoint | …`.
3. **Runtime state machine** — `accumulateIntoWindow`,
`flushWindow`, snapshot caches, label matchers, scheduler.
4. **Output binding** — emitting `pmetric.Metrics` of the right
typed variant, calling `nextConsumer.ConsumeMetrics`.

(1, 2, 4) are host-specific. (3) is host-neutral. The framework
refactor (see `design-asap-edge-framework.md` §6) extracts (3) so
adapters provide (1, 2, 4) only.

## Decision

### What gets extracted

Two new artifacts, one per language:

- **`asap-precompute-go`** — Go module living under
`ASAPCollector/asap-precompute-go/` (subdirectory, not a
separate repo for now; promotion to a separate repo is a
Phase-7 / repo-rename concern).
- **`asap-precompute-rs`** — Rust crate living under
`ASAPCollector/asap-precompute-rs/` for build-time deployment
inside an OTel-shaped ingest, and dual-published from the same
source as a normal Rust crate that `ASAPQuery-backend` can
depend on for its backend-side ingest path. Concretely the crate
source lives in this repo and `ASAPQuery-backend` consumes it
via git URL, the same way it depends on `asap_sketchlib` today.

### Public API

The trait, types, and config surface are pinned by the design
doc (§5.1, §6.2, §6.3). The summary contract:

- `Observation` — host-neutral input. Includes timestamp, metric
name, labels, and a `value` (`Float | Hash | Bytes |
Envelope`).
- `Precompute` trait — generic over `SketchT: Sketch`.
- `observe(&Observation) -> Result<(), Overflow>`
- `observe_envelope(SketchEnvelope) -> Result<(), Error>`
- `tick(now_ms) -> Vec<SketchEnvelope>`
- `PrecomputeConfig` — exactly today's processor config knobs
(sketch type, window size, matchers, delta thresholds, sketch
params) plus two new fields needed once the runtime is no
longer behind OTel's pipeline backpressure: `max_series` +
`OnOverflow` (Drop | Block | EvictOldest).
- `Sketch` trait family — `Sketch + QuantileSketch +
CardinalitySketch + FrequencySketch` (split). Each in-process
sketch impls only the queries it supports.
- Crash recovery is intentionally **not** in the trait. A future
`PersistentPrecompute: Precompute` extension trait can add
`snapshot()` / `restore()` later.

### Behavior preservation

- Wire format unchanged. `SketchEnvelope.payload` bytes are
byte-identical pre/post-extraction.
- Window semantics unchanged. Tumbling-vs-sliding-vs-batch logic
is moved verbatim from each processor file into
`asap-precompute-go::window.go` (Go) /
`asap-precompute-rs::window.rs` (Rust).
- Snapshot cache invariants unchanged. The
`snapshots map[string][]byte` (Go) and
`IngestState.sketch_snapshots` (Rust) maps move into
`snapshot_cache.go` / `snapshot_cache.rs` with the same
per-series-key contract.
- Backwards-compat for the Go OTel processors during Phase 2:
each existing `processor/{ddsketch,kll,hll,countsketch,countminsketch}processor/processor.go`
reduces to a ~50-line shim that delegates to
`asap-precompute-go`. The shim's `ConsumeMetrics` signature,
metric output schema, and config keys remain identical;
config-file changes are not required for existing deployments.

### Performance contract

- **Go (Phase 2):** per-observation `Observe` latency p99 must
stay within 10% of the pre-refactor in-line implementation.
Verified via the existing fake-exporter b3-delta benchmark.
- **Rust (Phase 3):** entry point `observe_envelope` stays
bit-identical to today's per-accumulator
`apply_proto_delta_bytes`. No behavior drift on backend
PromQL output.

### Repo / module layout

```
ASAPCollector/
├── asap-precompute-go/
│ ├── go.mod // module github.com/ProjectASAP/asap-precompute-go
│ ├── observation.go // Observation type
│ ├── envelope.go // SketchEnvelope type (Go view of the proto)
│ ├── precompute.go // Precompute interface + impl
│ ├── window.go // tumbling / sliding / batch logic
│ ├── snapshot_cache.go // outbound + inbound snapshot caches; ComputeDelta
│ ├── matchers.go // LabelMatcher / aggregate_by / seriesKey
│ ├── config.go // PrecomputeConfig + AggregationMode + OnOverflow
│ ├── adapter.go // Adapter trait + helpers
│ └── controlchannel/ // ControlChannel trait + HttpPollChannel impl
├── asap-precompute-rs/
│ ├── Cargo.toml // crate name asap-precompute-rs
│ └── src/
│ ├── lib.rs
│ ├── observation.rs
│ ├── envelope.rs // re-exports asap_sketchlib::proto::sketchlib::SketchEnvelope
│ ├── precompute.rs // Precompute trait + generic impl
│ ├── window.rs
│ ├── snapshot_cache.rs
│ ├── matchers.rs
│ ├── config.rs
│ ├── adapter.rs
│ └── control_channel.rs
└── opentelemetry-collector-contrib-patch/processor/
├── ddsketchprocessor/processor.go # Phase 2: ~50 LoC shim delegating to asap-precompute-go
├── kllprocessor/processor.go # Phase 2: ~50 LoC shim
├── hllprocessor/processor.go # Phase 2: ~50 LoC shim
├── countsketchprocessor/processor.go # Phase 2: ~50 LoC shim
└── countminsketchprocessor/processor.go # Phase 2: ~50 LoC shim
```

### Sequencing

- Phase 2 (Go) and Phase 3 (Rust) can proceed in parallel
because they touch different repos. Both must merge before
Phase 4 (Telegraf adapter) starts, since Telegraf reuses
`asap-precompute-go` and the OTel adapter shim must be a
proven shape before generalizing it.

## Consequences

### Positive

- Each existing processor shrinks from ~700–950 LoC to ~50 LoC
shim, removing the same conflated-concern logic five times.
- Telegraf / OTAP / Vector adapters become viable — they reuse
`asap-precompute-{go,rs}` rather than re-implementing window /
snapshot / matcher logic.
- Backend ingest path becomes a thin adapter calling the same
Rust crate the agents would use, eliminating the agent /
backend duplication for delta-apply logic.
- Future Sketch trait additions (e.g., `observe_batch` for
columnar Arrow ingest) become single-crate changes.

### Negative

- Cross-repo dependency added: Go OTel patches now pull in
`github.com/ProjectASAP/asap-precompute-go`. The build
pipeline (`build_sketchcollector.sh`) needs to handle two
module sources.
- Test surface doubles temporarily during the migration: each
function moves through a "duplicated, behavior-verified, then
delete the original" sequence to catch divergence. Phase 2
exit criterion (b3-delta produces same value, p99 within 10%)
is the gate.

### Compatibility

- No wire-format changes.
- No config-file changes for existing OTel collector
deployments.
- Backend PromQL output preserved bit-for-bit (R4 mitigation).

## Phase-2 / Phase-3 execution plan

See [`docs/phase-2-execution-plan.md`](../phase-2-execution-plan.md)
for the file-by-file extraction map covering all 5 OTel
processors.

## References

- [`docs/design-asap-edge-framework.md`](../design-asap-edge-framework.md) §3, §6, §9 (Phases 2–3)
- ADR-0001 (sketch-core retirement — prerequisite that simplified the Rust algorithm crate before this extraction)
- ADR-0003 (adapter trait + control channel — defines what the OTel processor shims look like after extraction)
Loading