Skip to content

Add log output through copy of client sdk logging infrastructure#109

Merged
1egoman merged 5 commits intomainfrom
add-logger
Oct 8, 2025
Merged

Add log output through copy of client sdk logging infrastructure#109
1egoman merged 5 commits intomainfrom
add-logger

Conversation

@1egoman
Copy link
Copy Markdown
Contributor

@1egoman 1egoman commented Oct 8, 2025

Adds a copy of src/logger.ts from the client sdk to this package, and wires up all logs to go through this logger implementation rather than via console.

One important caveat: logs from the mediapipe webworker aren't going through this logging infrastructure right now. Poking around in a debugger, it does look like there's a way to swap out console.log at the emscripten level but mediapipe abstracts over that interface in a way where it doesn't really look like there's any way to inject some custom logging implementation without fully forking mediapipe, which IMO probably isn't worth it for this. Related: google-ai-edge/mediapipe#4991

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Oct 8, 2025

🦋 Changeset detected

Latest commit: f1d3fb2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@livekit/track-processors Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@1egoman 1egoman marked this pull request as ready for review October 8, 2025 14:03
@1egoman 1egoman requested a review from lukasIO October 8, 2025 14:04
@1egoman
Copy link
Copy Markdown
Contributor Author

1egoman commented Oct 8, 2025

Per discussion with @lukasIO, I updated this change to tap into the logging infrastructure from the main client sdk package rather than instantiate a full new logging stack.

Copy link
Copy Markdown

@xianshijing-lk xianshijing-lk left a comment

Choose a reason for hiding this comment

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

One small question. lgtm on the rest

Comment thread src/logger.ts
const logLevel = LogLevel[methodName as LogLevelString];
const needLog = logLevel >= configLevel && logLevel < LogLevel.silent;

return (msg, context?: [msg: string, context: object]) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

just want to double check since I am not familiar with the high level use cases yet, should context be a optional object ? or tuple that contains the msg and context ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll admit I copied this code from here, and I'm unfamiliar with the specific details of why it was done the way it was done.

Taking a guess though, in https://github.com/pimterry/loglevel#writing-plugins, it seems like maybe the context isn't always included / only injected sometimes based off the log message?

@1egoman 1egoman merged commit 64cc9da into main Oct 8, 2025
4 checks passed
@1egoman 1egoman deleted the add-logger branch October 8, 2025 15:39
@github-actions github-actions Bot mentioned this pull request Oct 8, 2025
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.

3 participants