Skip to content

fix(tracing): avoid crash on non-hex trace_context IDs#1737

Open
wei-juncheng wants to merge 1 commit into
langfuse:mainfrom
wei-juncheng:fix/non-hex-trace-context-ids
Open

fix(tracing): avoid crash on non-hex trace_context IDs#1737
wei-juncheng wants to merge 1 commit into
langfuse:mainfrom
wei-juncheng:fix/non-hex-trace-context-ids

Conversation

@wei-juncheng

@wei-juncheng wei-juncheng commented Jul 4, 2026

Copy link
Copy Markdown

What does this PR do?

When a trace_context is passed with a trace_id (or parent_span_id) that is not
parseable as hexadecimal, _create_remote_parent_span raised
ValueError: invalid literal for int() with base 16 and the observation failed to
start — even though the preceding log line claimed the ID would be ignored.

The root cause is that the validity warning and the actual conversion were decoupled:

if not self._is_valid_trace_id(trace_id):
    langfuse_logger.warning("... Ignoring trace ID.")   # says "ignoring"

int_trace_id = int(trace_id, 16)                        # but still converts -> crash

So any invalid ID hit one of two broken paths:

  • Non-hex ID (e.g. "my-trace-123") → int(trace_id, 16) throws, crashing span creation.
  • Parseable-but-non-standard ID (e.g. a 31-char hex string) → conversion succeeds and
    the ID is used, but the log falsely says it was "ignored".

This PR makes the conversion authoritative:

  • Wraps int(..., 16) in try/except (ValueError, TypeError).
  • Falls back to RandomIdGenerator only when the ID is genuinely unparseable.
  • Keeps parseable IDs working as before.
  • Rewords the warning for parseable non-standard IDs so it no longer says the ID is ignored.
  • Handles parent_span_id with the same behavior.
  • Updates the LangChain CallbackHandler docstring example to use a valid 32-character lowercase hex trace ID.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Refactor
  • Documentation update
  • Tooling, CI, or repo maintenance

Verification

List the main commands you ran:

uv run --frozen pytest tests/unit/test_otel.py
uv run --frozen ruff check langfuse/_client/client.py tests/unit/test_otel.py
uv run --frozen ruff format --check langfuse/_client/client.py tests/unit/test_otel.py
uv run --frozen mypy langfuse/_client/client.py --no-error-summary

Added regression coverage for non-hex trace_id, non-hex parent_span_id, and parseable non-standard trace_id behavior.
Also updated the LangChain callback docstring example so it no longer suggests a non-hex custom trace ID.

Checklist

  • I self-reviewed the diff using code_review.md.
  • I added or updated tests for behavior changes.
  • I updated docs, examples, or .env.template if needed.
  • I did not hand-edit generated files; if generated files changed, I used the upstream regeneration path.
  • I did not commit secrets or credentials.

Greptile Summary

Fixes a crash in _create_remote_parent_span where non-hexadecimal trace_id or parent_span_id values caused an unhandled ValueError from int(..., 16) even though the preceding warning log claimed the ID would be ignored. The fix wraps both conversions in try/except (ValueError, TypeError) and falls back to RandomIdGenerator only for genuinely unparseable values, while preserving (and correctly warning about) parseable-but-non-standard IDs.

  • langfuse/_client/client.py: _create_remote_parent_span now catches parse failures for both trace_id and parent_span_id, falling back to randomly-generated IDs and emitting accurate warning messages.
  • tests/unit/test_otel.py: Three regression tests added covering non-hex trace ID, non-hex parent span ID, and parseable non-standard trace ID scenarios.
  • langfuse/langchain/CallbackHandler.py: The docstring update promised in the PR description (replacing "my-trace-id" with a valid 32-char hex example) was not included in the diff.

Confidence Score: 4/5

The client.py fix is correct and well-tested; the only gap is that the LangChain CallbackHandler docstring still shows a non-hex example, which the PR description explicitly said would be updated.

The two-file change is a targeted, straightforward bug fix with solid regression tests. The CallbackHandler.py docstring at line 167 still uses "my-trace-id", meaning users who copy that example will now hit the new warning path and get a random trace ID instead — the exact misbehavior this PR was meant to prevent for that audience.

langfuse/langchain/CallbackHandler.py — the docstring example at line 167 was not updated as described.

Reviews (1): Last reviewed commit: "fix(tracing): avoid crash on non-hex tra..." | Re-trigger Greptile

@CLAassistant

CLAassistant commented Jul 4, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@wei-juncheng wei-juncheng marked this pull request as ready for review July 4, 2026 04:47

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

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