Skip to content

dev-tool: Add option to enable extended livekit logs#3924

Open
BillCarsonFr wants to merge 4 commits into
livekitfrom
valere/devx/livekit_logs
Open

dev-tool: Add option to enable extended livekit logs#3924
BillCarsonFr wants to merge 4 commits into
livekitfrom
valere/devx/livekit_logs

Conversation

@BillCarsonFr
Copy link
Copy Markdown
Member

Content

Adds a new devtool option to get more logs from livekit (info -> trace) (default is no)

image image image

Motivation and context

Screenshots / GIFs

Tests

  • Step 1
  • Step 2
  • Step ...

Checklist

  • I have read through CONTRIBUTING.md.
  • Pull request includes screenshots or videos if containing UI changes
  • Tests written for new code (and old code if feasible).
  • Linter and other CI checks pass.
  • I have licensed the changes to Element by completing the Contributor License Agreement (CLA)

},
[setEnableExtendedLivekitLogs],
)}
/>{" "}
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.

Suggested change
/>{" "}
/>

Comment thread src/LivekitLogLevelSync.tsx Outdated
Comment on lines +14 to +22
export const LivekitLogLevelSync: FC = () => {
const [extendedLivekitLogs] = useSetting(enableExtendedLivekitLogs);

useEffect(() => {
setLogLevel(extendedLivekitLogs ? "trace" : "info");
}, [extendedLivekitLogs]);

return <></>;
};
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.

This React component would make more sense as a simple enableExtendedLivekitLogs.value$.subscribe(...) callback, since it has nothing to do with the UI… though not a big deal

Copy link
Copy Markdown
Contributor

@toger5 toger5 left a comment

Choose a reason for hiding this comment

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

Test coverage needs an eye.
Seems like you need two more lines. The DeveloperSettingsTab might be easy enough

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

Labels

PR-Developer-Experience Release note category. A PR that does not change EC but improves working with the repository.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants