Repository navigation
Conversation
…tion history, add internal ops documentation
…tion history, add internal ops documentation
raphaelmansuy
left a comment
raphaelmansuy
left a comment
There was a problem hiding this comment.
Verdict: request changes — do not merge.
The Langfuse 3.1 OTLP-gap diagnosis is correct, the 404-only probe heuristic is right, and pushing models.toml via POST /api/public/models does not violate LAW-124-12. The native ingestion path as written is not a valid 3.1 compatibility shim, tests do not cover the mapping, and the PR is not merge-ready.
What is worth keeping
/api/public/otelarrived in Langfuse v3.22.0; 3.1 returns 404. Confirmed against Langfuse OTEL docs.- Probe only falls back on 404; 401 / network errors keep OTLP. That must stay.
- OTLP path for 3.22+ / 4.x is preserved.
- Dedicated OS thread for the blocking probe is the right nested-runtime fix.
Blockers
1. Ingestion event types are invalid on Langfuse 3.1 — the version this PR exists for
spans_to_batch emits format!("{obs_type}-create"). Langfuse 3.1.1 ingestion discriminant (ingestion.yml at v3.1.1) only allows:
trace-create | span-create | generation-create | event-create | score-create | sdk-log | legacy observation-create / observation-update
3.1 ObservationType is only SPAN | GENERATION | EVENT. There is no retriever-create, embedding-create, or chain-create.
EdgeQuake (LAW-124-13) tags spans as retriever / embedding / chain. Those become illegal envelope types. The is_generation heuristic then overrides typed observations: embedding spans carry a model → forced generation-create; retriever / ingest-chain typically do not → retriever-create / chain-create → 207 errors.
export() treats HTTP 207 as success and never reads errors[]. Partial batch failure is silent.
Required 3.1 mapping: generation → generation-create; everything else → span-create. Do not stringify arbitrary observation types into the envelope type.
Please add unit tests for spans_to_batch covering generation, retriever, embedding, chain, and a root HTTP span. Today there are only RFC3339 + endpoint-join tests.
2. Legacy ingestion is a compatibility bridge, not a replacement for upgrade
/api/public/ingestion is deprecated for traces. Cloud sunset is 2026-11-16; self-hosted v4 events_only rejects it. Internal docs now say upgrade is no longer required. Please rewrite as: 3.1 can receive traces via ingestion fallback; 3.225+ OTLP remains the supported path. Claiming “all Langfuse ≥ 2.x” is unproven (tested 3.1.1 / 3.225.5 / 4 only).
3. Product env contract is undocumented
EDGEQUAKE_LANGFUSE_API=auto|otlp|ingestion is missing from .env.example, docs/OBSERVABILITY.md, docs/operations/configuration.md, SPEC-124 (LAW-124-1 is still OTLP-only), and Helm / k8s product docs. Only internal French runbooks were updated. Also missing: LangfuseApi::from_env / probe 404-vs-other tests.
4. CI
- Must fix on this PR:
cargo fmt --checkfails onlangfuse_ingestion.rs,lib.rs,subscriber.rs. Do not mix streaming/workspace files into this PR. - Likely pre-existing, not a reject reason here: rustdoc
private-intra-doc-linksinedgequake-pdf;RUSTSEC-2026-0258(h2); E2E UIpdf.worker.min.mjsmissing. Confirm those onedgequake-main.
Workspace lib tests and SPEC-018 passed; they cannot catch the 3.1 envelope bug.
5. Hygiene
- Both commits are titled
fix(streaming, conversations): restore SSE token streaming…while this PR is Langfuse. Squash / retitle before merge. - Direct
reqwest 0.12sits besideopentelemetry-otlp’s own reqwest. Document why a second client is required, or reuse the OTLP stack. make langfuse-sync-prices FORCE=1still POSTs; Langfuse update isPUT /api/public/models/{id}. Implement PUT or drop--force.
Do not approve until
- 3.1-safe envelope mapping +
spans_to_batchunit tests (generation / retriever / embedding / chain / HTTP root). - 207
errors[]logged (truncated is fine). cargo fmt; commits retitled; no streaming files.- Product env docs + SPEC-124 note that ingestion is a 3.x compatibility fallback; upgrade to 3.225+ still recommended.
--forcevia PUT, or removed.
After you push, I will re-review langfuse_ingestion.rs + subscriber.rs only.
There was a problem hiding this comment.
Blocker. This heuristic plus format!("{obs_type}-create") (L333) is invalid on Langfuse 3.1.1, which is the version this fallback exists for.
3.1.1 ingestion.yml envelope types are only trace-create / span-create / generation-create / event-create (plus score/sdk-log and legacy observation-*). ObservationType is SPAN | GENERATION | EVENT.
LAW-124-13 tags retriever / embedding / chain. Embedding spans carry a model → forced generation-create. Retriever / chain typically do not → retriever-create / chain-create → 207 errors.
Required mapping for 3.1: generation → generation-create; everything else → span-create. Do not stringify arbitrary observation types into the envelope type.
Please unit-test spans_to_batch for generation, retriever, embedding, chain, and a root HTTP span.
| let code = resp.status().as_u16(); | ||
| if resp.status().is_success() || code == 207 { | ||
| Ok(()) | ||
| } else { |
There was a problem hiding this comment.
Blocker. HTTP 207 is already 2xx (is_success()), so || code == 207 is redundant — and this branch never reads errors[]. The ingestion API returns 207 with per-event failures for illegal types (retriever-create, etc.). Silent Ok(()) is how traces can look enabled while retrieval/ingest observations never land.
Please parse the JSON body and log/surface truncated errors[] even on 207.
| skipped += 1 | ||
| continue | ||
| try: | ||
| status, _ = request(api(args.base_url, "/api/public/models"), token, "POST", m) |
There was a problem hiding this comment.
--force does not overwrite. It only skips the GET of existing names (L148), then still POSTs. Langfuse updates via PUT /api/public/models/{id}. Duplicate POSTs will fail; the flag is misleading. Implement PUT or remove --force.
|
Status vs v0.26.2: this PR was not merged. The 3.1 OTLP-gap diagnosis was correct; the product cut that landed is v0.26.2 (#391), after the review blockers here (illegal 3.1 envelope types, silent 207, docs, fmt). What 0.26.2 actually doesOne env, two transports. Default
Settings/health expose requested Operator notes: docs/operations/langfuse-3.1.md · upgrade-to-0.26.2.md. Honest split: 3.1.x vs newer 3.x (not “3.2”)Langfuse added OTLP in v3.22.0, not 3.2. We do not special-case a 3.2 line. We did not certify “every Langfuse ≥ 2.x”. Live proof on this cut: isolated 3.1.1 ( Envelope mapping on the 3.1 bridge (the blocker on this PR): Ingestion is a compatibility bridge, not the long-term path. Cloud sunsets it 2026-11-16; self-hosted v4 What this PR asked for that is not “dual version support”
If this fork has no remaining unique work (price-sync |
3. Langfuse : support de toutes les versions ≥ 2.x
L'endpoint OTLP
/api/public/otel/v1/tracesn'existe qu'à partir de Langfuse3.22x — sur 3.1 il renvoie 404, donc aucune trace ne pouvait arriver, quelle
que soit la configuration réseau. Ajout d'un exportateur
SpanExporternatif(
langfuse_ingestion.rs) parlant/api/public/ingestion, présent depuis Langfuse v2.Le transport est choisi au démarrage via
EDGEQUAKE_LANGFUSE_API=auto|otlp|ingestion(défaut
auto, qui sonde et bascule sur un 404). Le chemin OTLP est inchangé.4. Coûts à $0.00 pour tout modèle récent
Langfuse calcule les coûts depuis son propre catalogue et EdgeQuake n'émet jamais
d'attribut de coût (
LAW-124-12— Langfuse reste la source unique de vérité). Lecatalogue livré avec 3.1 date de 2024 :
gpt-4omais pasgpt-5,gemini-1.5maispas
2.5,claude-3.5mais pas4.x, aucun Mistral. Ajout descripts/langfuse_sync_model_prices.py+ ciblemake langfuse-sync-pricesqui pousseles tarifs de
models.toml— sans contourner LAW-124-12.Fixes # (aucune issue ouverte — bugs remontés en usage direct)
Type of change
Checklist