feat(otel): add Prometheus + Loki consumer source

[BtXin]

Summary

Adds type: otel as a federated data source — Skardi pulls metrics and logs from Prometheus-compatible and Loki backends and exposes them through DataFusion as metrics / logs tables (with predicate pushdown) plus prom_query / prom_range / loki_query / loki_range escape-hatch table functions. Closes the OTEL consumer side of the design doc at openspec/changes/add-otel-data-source/.

Drafted for review of architecture + sharp-edge tradeoffs before final merge.

43 / 50 tasks complete; the 7 remaining are tracked as partial with inline scope notes pointing at the deferred follow-ups (most notably labels['k'] matcher recognition in the translator and aggregate pushdown — both need DataFusion map_extract Expr-shape validation against a running planner).

What lands

• Foundation — otel cargo feature on skardi + skardi-server + skardi-cli; DataSourceType::Otel variant; OtelError / OtelHttpClient / OtelSourceConfig / OtelAuth with env-only credentials and inline-secret rejection at config load
• Translator (otel/translator.rs) — SQL → backend-query IR with the v1 supported-predicate matrix (name=, ts range, line LIKE, LIMIT); unsupported predicates surface as OtelUnsupportedPushdown with a prom_query(...) / loki_query(...) hint baked into the error message
• Prometheus provider — predicate-pushdown metrics TableProvider + prom_query/prom_range escape-hatch UDTFs; Arrow batch construction with the row cap enforced; JSON parsing hand-rolled against the documented API so our OtelHttpClient (auth + extra_headers + timeout) stays in play
• Loki provider — predicate-pushdown logs TableProvider + loki_query/loki_range UDTFs; handles both streams and matrix response shapes; empty-selector queries reject at the table boundary with a clear pointer at loki_range
• Access mode — SQL validator wires metrics/logs table names as ReadOnly; INSERT / UPDATE / DELETE against them fail at config load with a table-named error
• Server integration — dashboard renders OTEL sources with a read-only badge + Prometheus @ <url> / Loki @ <url> subtitle; startup info logs per registered OTEL source
• Observability of the OTEL provider itself — skardi_otel_queries_total{source, backend, outcome} + skardi_otel_query_duration_seconds{source, backend}, with Outcome enum mapping each OtelError variant to a stable label string (pinned by unit tests so dashboards can't silently break)
• CLI — skardi sql --ctx ctx.yaml ... works against type: otel sources via the same registration path the server uses
• Demo — demo/otel_service_health/ with ctx.yaml + 3 pipelines (tier-1 metrics, tier-3 loki_range, tier-3 prom_query) + README documenting the v1 sharp edges
• CI — new check-otel fast-lane job in .github/workflows/ci.yml running otel-gated unit + integration tests in ~2 minutes per PR

Sharp edges shipping in v1 (intentional, documented)

1. labels['k'] matchers aren't pushed down yet. The translator can't yet recognize them; tier-1 queries referencing them fall through to UnsupportedPushdown pointing at the escape hatch. Tracked under tasks 3.5.2 / 3.5.3.
2. Aggregations on metrics happen at the DataFusion level, not in PromQL. Counter-vs-gauge semantic mismatch — for counter metrics, use prom_query('sum by(service)(rate(...[5m]))'). Tracked under task 4.2; documented in design.md Decision 4 "v1 sharp edge".
3. Pipeline param substitution has known rough edges for timestamps ({param} → NULL breaks INTERVAL shapes at load-time validation) and for params inside SQL string literals (JSON strings get single-quoted, producing nested quotes inside prom_query('...{...}...')). Demo pipelines hardcode these values. Fix is in the pipeline substituter, separate from this change.
4. SELECT labels['k'] from prom_query results currently tickles a DataFusion projection bug. Workaround: project the whole labels Map and destructure client-side. Tracked separately.
5. Per-source rate limiting is not in v1. Row caps + time-window enforcement + per-request timeouts bound the blast radius of any single query. For strict-SLA upstreams, put a reverse-proxy rate limit in front of Skardi.

Test plan

• cargo test --workspace — 457 / 457 pass (default features; +1 over baseline from a new pipeline-brace-safety test)
• cargo test --workspace --features skardi/otel — all green: 537 skardi unit + 5 access_mode + 9 loki integration + 8 prom integration + 3 demo_smoke + 8 otel_config + 50 pre-existing server tests
• cargo fmt --all -- --check clean
• cargo clippy --workspace --features skardi/otel --all-targets — no new warnings in OTEL files
• cargo check --workspace clean; cargo check --workspace --features skardi/otel clean
• Manual: boot the demo against observability/docker-compose.yml (docker compose -f observability/docker-compose.yml up -d prometheus loki) + record a screencast of an agent calling /service-health/execute (task 10.4, the meta task this PR opening completes)

Reviewer notes

Suggested reading order if you only have 30 minutes:

1. openspec/changes/add-otel-data-source/proposal.md — why this exists, what's in / out of scope
2. openspec/changes/add-otel-data-source/design.md — 10 design decisions, especially Decision 4 (tier-1 vs tier-3) and Decision 11 (pipelines as the canonical surface)
3. docs/otel/README.md — operator-facing reference (config, schema, predicate matrix, sharp edges)
4. demo/otel_service_health/ — runnable demo
5. crates/skardi/src/sources/providers/otel/translator.rs — the architectural heart (where SQL meets the backend HTTP API)
6. openspec/changes/add-otel-data-source/tasks.md — the 43 ✓ + 7 partial scope notes show exactly what shipped vs deferred

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 82.10526% with 544 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...es/skardi/src/sources/providers/otel/prometheus.rs	68.29%	182 Missing ⚠️
crates/skardi/src/sources/providers/otel/loki.rs	79.52%	121 Missing ⚠️
...es/skardi/src/sources/providers/otel/translator.rs	91.18%	88 Missing ⚠️
crates/server/src/config.rs	4.34%	44 Missing ⚠️
crates/cli/src/main.rs	0.00%	26 Missing ⚠️
crates/skardi/src/sources/providers/otel/http.rs	79.24%	22 Missing ⚠️
crates/server/src/gui.rs	0.00%	19 Missing ⚠️
...rates/skardi/src/sources/providers/otel/metrics.rs	85.98%	15 Missing ⚠️
crates/skardi/src/sources/providers/otel/time.rs	92.56%	11 Missing ⚠️
crates/skardi/src/sources/providers/otel/mod.rs	97.00%	10 Missing ⚠️
... and 3 more

📢 Thoughts on this report? Let us know!

[BtXin]

Code review — feat(otel): add Prometheus + Loki consumer source

Reviewed against add-otel-data-source. Spec ↔ code correspondence is unusually high; tests are genuinely solid for v1. Findings below — most are polish, but #1 and #2 are merge-blockers IMO.

Strengths

• Honest task list. The 7 partial items in tasks.md carry inline scope notes; nothing is silently aspirational.
• Defense-in-depth on credentials. Hand-rolled Deserialize for OtelAuth at mod.rs:75-137 rejects token:/password:/username: at YAML parse with field-named errors.
• Single outbound HTTP chokepoint. http.rs — auth, extra_headers, per-request timeout converge here; future rate-limiter has one place to land.
• Pinned outcome labels. outcome_strings_are_stable in metrics.rs:147-158 prevents silent Grafana breakage when refactoring.
• Wire-level test assertions. otel_prometheus.rs:121 pins query=http_requests_total{}; otel_loki.rs:145 pins the full LogQL string.
• LIKE → regex correctly escapes metacharacters — translator.rs:737-744 (LIKE 'a.b%' must not match aXb…).
• Brace-collision pinned. inferencer.rs:1174 proves the {identifier} regex won't snag PromQL {service="api"}.

Issues (prioritized)

1. prometheus-http-query declared but never used — merge-blocker

Cargo.toml:27 + Cargo.toml:59 pull it in (with rustls-tls); zero use prometheus_http_query anywhere. Doc comment at prometheus.rs:27-30 explicitly justifies hand-rolled JSON parsing. This contradicts Decision 3 in design.md and adds dead compile time + TLS stack to every --features otel build. Drop the dep and update Decision 3.

2. Translator errors have empty source_name — operator UX

translator.rs:494-498, 507, 519, 624, 641, 646 construct OtelError::Backend { source_name: String::new(), … }. These propagate untouched; the rendered message is otel source : expected a timestamp literal…``. Thread source_name: &str into `literal_as_timestamp` / `try_line_like` / `try_ts_predicate`, or rewrap in `translate_*_filters`. Violates the invariant that every `OtelError` variant carries the source name.

3. _DURATION_REEXPORT_REMINDER hack

translator.rs:677-679 — #[allow(dead_code)] const _DURATION_REEXPORT_REMINDER: Option<Duration> = None; silences an unused-import warning. The use std::time::Duration; at line 56 has no other use site. Delete both.

4. spec.selector == \"{}\" is a stringly-typed sentinel

loki.rs:627 detects "no stream-label predicate" via literal string comparison. Once 3.5.3 lands and real selectors get emitted, this silently breaks. Replace with LokiQuerySpec::has_stream_label: bool or selector: Option<String>.

5. HTTP 200 + status:\"error\" returns UpstreamStatus { status: 200 }

prometheus.rs:224-232, loki.rs:219-227. Confusing for operators chasing a 5xx that isn't there. Add a LogicalError variant or prefix the message with "logical error (HTTP 200)".

6. supports_filters_pushdown returns Exact for all filters

prometheus.rs:604-609, loki.rs:605-610. Deliberate per Decision 4 (fast-fail beats unbounded fan-out), but differs from tasks.md 4.1's text. Update the task spec to match. Consider an alert on outcome=\"unsupported_pushdown\" — agent SQL variation will hit this in prod.

7. Float-precision loss in Loki matrix timestamps

loki.rs:368: (ts_secs * 1e9) as i64 loses sub-µs precision past ~15 decimal digits. Benign for rate(…) over minutes, but consider ts_secs.trunc() * 1_000_000_000 + (ts_secs.fract() * 1e9) as i64.

8. Null arg errors instead of defaulting

prometheus.rs:833-838, loki.rs:843-848. The comment correctly identifies that pipeline param substitution emits NULL for unbound params, then returns plan_err!. Treating Null as None (so resolve_window applies defaults) would unblock the parameterized-window case the demo currently hardcodes around.

9. Multiple OTEL sources silently overwrite UDTFs

prometheus.rs:939-946, loki.rs:910-917, and the CLI mirrors this at cli/src/main.rs:961-1024. Two OTEL sources → second registration wins silently. Single-source-per-process is documented but not enforced. Add tracing::warn! or hard-error on duplicate registration.

10. "RFC-3339/ISO-8601" doc inaccuracy

prometheus.rs:856-864, loki.rs:835 call parse_from_rfc3339 (strict — no 2024-01-02T15:04:05 without offset). Either widen the parser or drop "ISO-8601" from the docs/errors.

11. extract_substring_like ignores SQL ESCAPE clause

translator.rs:220-231 — the DataFusion Like Expr exposes escape_char: Option<char> but it's ignored. Low likelihood, but LIKE 'a%b' ESCAPE '\\' would produce a wrong substring path. Either honor it or assert escape_char.is_none() and fall through to UnsupportedPushdown.

Test coverage

37+ tests across otel_prometheus.rs (8), otel_loki.rs (9), otel_access_mode.rs (5), otel_config.rs (8), otel_demo_smoke.rs (3), plus extensive unit coverage. Gaps (all flagged in the PR body as deferred):

• labels['k'] matcher pushdown (3.5.2 / 3.5.3) — no tests, no implementation. The v1 tier-1 SQL surface is materially less useful than the spec advertises until these land; demo top_error_logs.yaml has to use loki_range for this reason.
• Aggregate pushdown (4.2) — translator hard-errors on any aggregate.
• otel_demo_smoke asserts rows > 0 but doesn't pin upstream query params — tightening to query_param(…) matchers would catch translator regressions caught only at the wire today.

Security

• Inline-secret rejection at deserializer + OtelError::InlineCredentialRejected, tested e2e in otel_config.rs:127-150.
• Outbound TLS via rustls; bearer/basic auth never logged.
• extra_headers validated at startup so a bad header in ctx.yaml fails the server, not the first scan.
• max_result_rows (50k), max_window (24h), request_timeout (10s) bound blast radius. Per-source rate-limiting is a documented v1 non-goal.

Suggested next moves

The two easy wins (#1 and #3) as a single patch:

--- a/crates/skardi/Cargo.toml
-otel = [\"dep:reqwest\", \"dep:humantime-serde\", \"dep:prometheus-http-query\", \"dep:opentelemetry\"]
+otel = [\"dep:reqwest\", \"dep:humantime-serde\", \"dep:opentelemetry\"]
-prometheus-http-query = { version = \"0.8\", default-features = false, features = [\"rustls-tls\"], optional = true }

--- a/crates/skardi/src/sources/providers/otel/translator.rs
-use std::time::Duration;
-#[allow(dead_code)]
-const _DURATION_REEXPORT_REMINDER: Option<Duration> = None;

…and update design.md Decision 3 to drop the prometheus-http-query rationale (the actual rationale lives at prometheus.rs:27-30).

#2 (translator source_name plumbing) is a small but real correctness fix — error messages without the source name will be a recurring operator complaint.

Issues 4–11 are smaller polish; none block correctness for the v1 shape.

[BtXin]

Re: sharp edge #4 — the projection bug is broader than the PR body suggests

Ran the v1 against the bundled observability stack (observability/docker-compose.yml — real Prom + Loki, simple_backend as emitter, ~3 min of mixed-outcome traffic) and the documented workaround — "project the whole labels Map and destructure client-side" — doesn't actually hold. The bug surfaces in three independent shapes, only one of which is covered by sharp edge #4.

Repro A — tier-3 + labels['k'] (the documented case)

SELECT labels['pipeline'] AS pipeline, value
FROM prom_query('sum by (pipeline) (rate(pipeline_requests_total{status=\"error\"}[5m]))')

Internal error: Assertion failed: col.name() == matching_name: Input field name name does not match with the projection expression labels.

Repro B — tier-1 + labels['k'] (same bug, different surface)

SELECT name, labels['pipeline'] AS pipeline, value
FROM metrics
WHERE name = 'pipeline_requests_total'
LIMIT 5

Internal error: Assertion failed: col.name() == matching_name: Input field name ts does not match with the projection expression value.

So this isn't prom_query-specific — PromMetricsTable::scan exhibits the same column-ordering mismatch. Forward-implication: task 3.5.2 (WHERE labels['k']=… matcher recognition) will surface this bug for every tier-1 query the moment it lands.

Repro C — UNION ALL of two prom_query results, no labels[] access at all

SELECT 'error_rate' AS name, labels, ts, value
FROM prom_query('sum by (pipeline) (rate(pipeline_requests_total{status=\"error\"}[5m]))')
UNION ALL
SELECT 'p99_ms' AS name, labels, ts, value
FROM prom_query('histogram_quantile(0.99, sum by (pipeline, le) (rate(pipeline_latency_ms_milliseconds_bucket[5m])))')

Same assertion. Also fails with a 3-column shape ('metric' AS metric, labels, value) and with a WITH … LEFT JOIN CTE form.

Repro C is the "federation case" from design.md Decision 4 — joining two telemetry queries in one round-trip — and is the single shape Grafana MCP architecturally cannot do. Currently unreachable from a pipeline.

Evidence the root cause is in the provider, not DataFusion

Server log from the failing UNION ALL:

ERROR skardi::engine::datafusion: Failed to collect query results.
  Schema: Schema { fields: [
    Field { name: \"metric\", data_type: Utf8 },
    Field { name: \"labels\", data_type: Map<...>, ... },
    Field { name: \"value\",  data_type: Float64, nullable: true },
  ] }
ERROR ... Input field name name does not match with the projection expression labels.

Plan-time schema has 3 columns [metric, labels, value] — but the assertion references field name name, which is column[0] of prom_query's declared 4-column schema [name, labels, ts, value]. So the projection was built against the function's declared schema but at execution it's being matched against a reshaped input — column indices line up against the wrong names. That's a schema-vs-RecordBatch divergence at the provider boundary, not a DataFusion bug.

The current error message (\"This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker...\") is misleading for operators chasing this.

Likely fix sites

• crates/skardi/src/sources/providers/otel/prometheus.rs — PromMetricsTable::scan() + PromQueryFunction's batch builder. Check that any projection-aware column pruning uses indices into the declared SchemaRef, and that the emitted RecordBatch columns come back in declared order (not source-iteration order).
• crates/skardi/src/sources/providers/otel/loki.rs — same pattern; I'd expect LokiLogsTable::scan() exhibits the same bug under e.g. SELECT line, labels FROM logs (not repro'd, but the shape is identical).

Impact on the v1 contract

Until this lands, the spec's "agent writes SQL across telemetry" promise narrows to SELECT name, labels, ts, value FROM <single prom_query/loki_range/metrics-no-labels-projection>. Any column-drop projection, any compound query, any labels['k'] access — and the agent gets a "DataFusion bug" pointing at an upstream tracker.

I have full request/response captures from both skardi-otel and grafana-mcp on the same five canonical questions (Q1–Q4 work on both, Q5 = this bug) if useful for a regression test — happy to share.

[abbccdda]

Is this still relevant? @BtXin