Skip to content

fix: skip OTLP log exporter setup when recording is disabled#4892

Open
theomonnom wants to merge 3 commits intomainfrom
fix/recording-401-when-disabled
Open

fix: skip OTLP log exporter setup when recording is disabled#4892
theomonnom wants to merge 3 commits intomainfrom
fix/recording-401-when-disabled

Conversation

@theomonnom
Copy link
Member

No description provided.

@chenghao-mou chenghao-mou requested a review from a team February 18, 2026 23:20
or options.get("audio", True)
or options.get("transcript", True)
)
if not needs_cloud:
Copy link
Member

Choose a reason for hiding this comment

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

should this be: needs_cloud and not is_cloud(..)?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup you're right

Move the is_cloud check from the top-level guard into the combined
condition with needs_cloud, so the early return is:
  if not (needs_cloud and is_cloud(url))

This is cleaner and addresses the PR review feedback.
Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

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

lgtm

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

Comments