Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Disco] Propagate structlog/logging config to workers #16715

Merged

Conversation

Lunderberg
Copy link
Contributor

This is a follow-up to #16618, which propagates the structlog configuration to disco worker processes. For configurations that use structlog.stdlib to integrate structlog with the stdlib logging module, this integration must also be forwarded.

This is a follow-up to apache#16618, which propagates the `structlog`
configuration to disco worker processes.  For configurations that
use `structlog.stdlib` to integrate `structlog` with the stdlib
`logging` module, this integration must also be forwarded.
Copy link
Contributor

@slyubomirsky slyubomirsky left a comment

Choose a reason for hiding this comment

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

This seems straightforward enough, though I'm not aware of the wider context.

@Lunderberg
Copy link
Contributor Author

This seems straightforward enough, though I'm not aware of the wider context.

Thank you. For context, some applications use structlog to provide more flexible logging than python's stdlib logging. The structlog configuration determines what pre-processing is done for the log statements (e.g. appending contextual information to a log statement). When starting child processes using multiprocessing, it would be useful for the child processes to format/save their logs in the same manner as the parent, but this doesn't occur by default. In PR#16618, I added handling to forward the structlog configuration from the main process to the tvm.runtime.disco worker processes.

However, some configurations (example from structlog's documentation integrate structlog with the stdlib logging. The previous implementation only forwarded the configuration held by structlog, and didn't forward the configuration within the stdlib logging. This PR closes that gap.

@Lunderberg Lunderberg merged commit 1891b4d into apache:main Mar 27, 2024
19 checks passed
@Lunderberg Lunderberg deleted the disco_structlog_with_stdlib_logging branch March 27, 2024 14:11
thaisacs pushed a commit to thaisacs/tvm that referenced this pull request Apr 3, 2024
This is a follow-up to apache#16618, which propagates the `structlog`
configuration to disco worker processes.  For configurations that
use `structlog.stdlib` to integrate `structlog` with the stdlib
`logging` module, this integration must also be forwarded.
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