context call sites #4164
context call sites #4164
Conversation
| else: | ||
| logger.debug(msg) | ||
| fire_event(MacroEventDebug(msg)) |
There was a problem hiding this comment.
Just popping in to say that this does get called a lot, for lots of different reasons—this is what's called any time a message is logged from within the Jinja context. That could be for materializations, wrapped by other macros in packages, to offer visibility in complex operations, or any custom user-space code.
I think MacroEvent is a fine name; log() is called as a macro, though it can be called from outside macros (models, hooks, etc). I also think it's fair that this is not going to be nearly as structured/consistent as built-in dbt logging.
Eventually, we might think about offering a more structured version of the log macro, e.g. in which users can pass a key-value dictionary and an f-string-like message. That's for later, this is good for now.
There was a problem hiding this comment.
is it worth a tech-debt ticket?
There was a problem hiding this comment.
Maybe! I wouldn't be surprised if there's contextual info freely available in the Jinja context, which we can pass along with the user-provided message to make it that much richer. E.g. the "parent" macro calling this log() macro. But I don't actually know that
There was a problem hiding this comment.
Opened an issue to keep thinking about this: #4184
There was a problem hiding this comment.
LGTM! If we run into a lot of events with this pattern of just passing a message, we may want to make a little escape hatch so we don't need a whole type every time, but I do consider it an anti-pattern to just blindly pass along things without at least adding context.
* updated context dir to new structured logging
* updated context dir to new structured logging
* updated context dir to new structured logging
* updated context dir to new structured logging
Description
Modifying call sites for structured logging in core/dbt/context
Checklist
CHANGELOG.mdand added information about my change