Page MenuHomeElementl

[RFC] send python logging messages to the DagsterLogManager
Needs RevisionPublic

Authored by sandyryza on Jun 2 2021, 11:21 PM.

Details

Summary

This essentially implements Option 2 in https://github.com/dagster-io/dagster/discussions/4195.

Test Plan

bk

Diff Detail

Repository
R1 dagster
Branch
logging-capture (branched from master)
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

sandyryza added a reviewer: max.

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?

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.

This revision now requires changes to proceed.Jul 2 2021, 8:14 PM