Skip to content

feat(langfuse): migrate observability integration - #10

Open
Yangtze-Seventh wants to merge 1 commit into
mainfrom
feat/langfuse-integration
Open

Yangtze-Seventh wants to merge 1 commit into
mainfrom
feat/langfuse-integration

Conversation

@Yangtze-Seventh

Copy link
Copy Markdown
Collaborator

Summary

  • migrate the complete EverOS × Langfuse integration from EverOS/examples/langfuse into plugins/langfuse
  • preserve the live OpenTelemetry demo, local trace recorder, 237-span/60-trace fixture, and replay utility
  • make replay fixture discovery independent of the current working directory
  • add pinned replay dependencies, offline contract tests, a full local replay round trip, and path-scoped CI
  • document Langfuse as an observability integration in the repository catalog

EverOS keeps its vendor-neutral OpenTelemetry runtime and Langfuse-compatible attributes/scores; only the ecosystem-facing example is moved.

Verification

  • ruff check demo.py replay.py record_trace.py tests
  • python -m py_compile demo.py replay.py record_trace.py
  • python replay.py --help
  • python record_trace.py --help
  • python -m unittest discover -s tests -v
  • end-to-end replay: 237 spans and 47 scores sent to and re-recorded by the local compatible sink
  • migrated demo.py, record_trace.py, recorded_trace.json, and .gitignore byte-for-byte compared with the EverOS source
  • staged secret-pattern scan: no credentials detected

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: replay must fail when Langfuse rejects trace or score uploads, and the branch conflict must be resolved.

I reviewed the full origin/main...HEAD diff and the surrounding replay/recorder/demo paths. The offline workflow checks pass, but the inline false-success path can leave a user with no imported data while returning exit status 0. Separately, GitHub currently reports this PR as CONFLICTING; merging current main produces a conflict in the integration table in README.md (the new dify_cloud and langfuse rows both occupy the same insertion point).

Verification: python -m py_compile demo.py replay.py record_trace.py, both CLI --help entry points, and python -m unittest discover -s tests -v all passed (3 tests) in a clean virtual environment with requirements.txt installed. I also checked the repository guidance (no AGENTS.md, CLAUDE.md, or CONTEXT*.md is present), changed-file callers, commit/path history, fixture topology, compatibility with current main, and that the tests were additive rather than weakened. No separate documented architecture constraints were present; the native-OTLP architecture and fixture/replay contract are internally consistent apart from the reported error handling.

Comment thread langfuse/replay.py
try:
_post_score(host, auth, payload)
sent += 1
except urllib.error.HTTPError as exc:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: Replay must return failure when Langfuse rejects uploads.

This catches every terminal score HTTP failure, prints it, and continues to the final “Open Langfuse” message with exit status 0. Trace export has the same externally visible result: BatchSpanProcessor logs exporter failure, but force_flush() only reports whether it timed out and its result is ignored before the unconditional “Replayed … to host” message. For example, a reachable but wrong LANGFUSE_HOST can pass _check_credentials with a non-401/403 response, reject the OTLP and score POSTs, and still make this command look successful to both a person and automation. Please track terminal trace/score failures and exit nonzero instead of claiming completion when the project received only part—or none—of the fixture.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants