Skip to content

log to trace example fix python#10

Open
Nightingalelyy wants to merge 1 commit intorespanai:mainfrom
Nightingalelyy:DEV-5811-Fix-logs-to-trace-example-python
Open

log to trace example fix python#10
Nightingalelyy wants to merge 1 commit intorespanai:mainfrom
Nightingalelyy:DEV-5811-Fix-logs-to-trace-example-python

Conversation

@Nightingalelyy
Copy link
Contributor

No description provided.

Copy link
Contributor

@Raymond1415926 Raymond1415926 left a comment

Choose a reason for hiding this comment

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

Convention Review (BE golden rules: DRY, types/consts, reusable utils)

Verdict: Approve

PR #10 — Python fix (small, focused)

What's good:

  • infer_log_type() is a clean, well-documented helper with clear heuristic ordering
  • Uses Counter for sanity-check output — simple and appropriate
  • Defensive: if not processed_log.get("log_type") preserves existing values

Minor nit (non-blocking):

  • json.load(open(file_name)) doesn't close the file handle explicitly. with open(file_name) as f: json.load(f) is the idiomatic pattern.

Copy link
Contributor

@Raymond1415926 Raymond1415926 left a comment

Choose a reason for hiding this comment

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

Review: DEV-5811 Fix logs-to-trace Python example

Summary: Adds infer_log_type() function to utils.py that infers span types (chat/generation/task/tool) from span name heuristics. Updates generate_trace_data() to set log_type when missing. Adds a sanity-check print in main.py showing log_type distribution.

Verdict: APPROVED

The fix addresses the core issue: the example was sending spans without log_type, causing everything to render as 'chat' in the UI. The heuristic approach is reasonable for example code, and the fallback to 'generation' is safe.

Good to merge.

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