Skip to content

fix(observability/langfuse): surface placeholder & runtime ingestion failures (#60961) - #60974

Closed
imax500tr wants to merge 1 commit into
NousResearch:mainfrom
imax500tr:fix/60961-langfuse-placeholder
Closed

fix(observability/langfuse): surface placeholder & runtime ingestion failures (#60961)#60974
imax500tr wants to merge 1 commit into
NousResearch:mainfrom
imax500tr:fix/60961-langfuse-placeholder

Conversation

@imax500tr

Copy link
Copy Markdown

Summary

Fixes #60961 — the Langfuse observability plugin silently accepted placeholder/template credentials and swallowed runtime ingestion failures, so operators got zero traces with no signal in the logs.

  • Placeholder detection hardened (_validate_langfuse_key): previously only the pk-lf-/sk-lf- prefix was checked. Now a key that carries the right prefix but still contains a known placeholder fragment (e.g. sk-lf-..., sk-lf-***, pk-lf-placeholder, sk-lf-unset) is rejected with a clear one-shot warning naming the offending env var. Substring fragments stay conservative (real keys like sk-lf-...-abc are not flagged); whole-value templates (sk-lf-..., ***) are matched by exact equality.
  • Runtime failures surfaced: Langfuse's on_unexpected_error callback is now wired so the first ingestion failure (bad credentials / missing project) is logged as an ERROR naming the likely cause, instead of being swallowed. The warning fires once per process to avoid log floods. Older SDK versions that don't accept the callback fall back gracefully.

Test plan

  • Added TestIssue60961PlaceholderFragments (parametrized over pk-lf-..., sk-lf-..., sk-lf-***, pk-lf-placeholder, sk-lf-unset, pk-lf-example-key, …) asserting the guard rejects them.
  • Added TestRuntimeIngestionError asserting on_unexpected_error logs the first failure and fires only once.
  • Existing placeholder tests still pass; full tests/plugins/ suite (1752 tests) green.

🤖 Generated with Hermes

…failures (NousResearch#60961)

- Extend _validate_langfuse_key to catch placeholder fragments even when the
  key carries the correct pk-lf-/sk-lf- prefix (e.g. sk-lf-..., sk-lf-***,
  pk-lf-placeholder). Substring fragments stay conservative so real keys like
  sk-lf-...-abc are not flagged; whole-value templates (sk-lf-..., ***) are
  matched by exact equality.
- Wire Langfuse's on_unexpected_error callback so the first runtime ingestion
  failure (bad creds / missing project) is logged as an ERROR naming the cause,
  instead of being swallowed silently. The warning fires once per process to
  avoid log floods.

Fixes NousResearch#60961
@imax500tr imax500tr closed this Jul 8, 2026
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jul 8, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: fixes #60961. This extends the merged placeholder guard (#26320) which only checks the pk-lf-/sk-lf- prefix — a right-prefix-but-placeholder key (e.g. sk-lf-***, pk-lf-placeholder) still passes on main, and runtime ingestion failures are still swallowed. This PR adds fragment detection + wires on_unexpected_error, covering the residual gap. Related discussion threads: #51399, #57949, #29340.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Langfuse SDK plugin: placeholder API key silent failure

2 participants