This essentially implements Option 2 in https://github.com/dagster-io/dagster/discussions/4195.
Details
Diff Detail
- Repository
- R1 dagster
- Branch
- logging-capture (branched from master)
- Lint
Lint Passed - Unit
No Test Coverage
Event Timeline
does this work with a log line emitted by a solid that isn't defined inline (but loaded from a file/module); does this work with log lines emitted by libraries called by solids?
python_modules/dagster/dagster/core/execution/plan/python_logging.py | ||
---|---|---|
18 | typo |
does this work with a log line emitted by a solid that isn't defined inline (but loaded from a file/module); does this work with log lines emitted by libraries called by solids?
I can write a test to verify, but it should. The root logger is global.
One nice thing we could do here would be to make it easy to filter down log messages in the structured viewer to specific loggers. That could actually be a pretty nice feature.
One nice thing we could do here would be to make it easy to filter down log messages in the structured viewer to specific loggers. That could actually be a pretty nice feature.
Oo yeah, that would be cool.
if we did this we would want it behind an opt-in gating of some kind right?
In 0.11.0 yes, but IMO we'd want to have it on by default in 0.12.0. The goal is that hello world could be:
import logging @solid def hello(name: str): logging.info(f"Hello, {name}!")
sending to your queue, I am guessing we are not going to move on this for 0.12.0 given the current timing.
I am very very very hesitant about sending all python logging to the event DB by default.
I am cool with an easy explicit opt in, but worry about the impact this would have as an implicit default. Especially with our current poor tooling around cleaning up or understanding whats in the DB.