Transactional json logging #1806
Conversation
beckjake
force-pushed
the
feature/add-docs-generate-rpc
branch
from
02feb69 to
2031e23
Compare
beckjake
changed the base branch from
feature/add-docs-generate-rpc
to
dev/louisa-may-alcott
beckjake
force-pushed
the
feature/transactional-json-logging
branch
from
4b227a6 to
52ac98e
Compare
There was a problem hiding this comment.
This broadly looks very good to me. I think there are a couple more log lines we'll want to add here:
- at the beginning of the run, publish a log line that reports the number of resources that are going to be run in the invocation (eg. `"node_count": 17). This will help track completion percentage as the rest of the logs flow
- in the
Began running modellog line, do we have a node index? That will help order the nodes in a list (i imagine doing this by timestamp could be tricky, but doable if necessary)
And a couple of questions:
- how does the
exc_infofield get populated? - can we better structure the field values around success/error states? It looks like
model_statecan either bepassed, or a literal error message. If possible, it would be good to present a field which has a value in {success,error,skipped} that can be used in conjunction with a free-text field which describes the error (if one occurs).
Let me know if any of these things are more involved to implement than they appear at first blush. We have a lot of options here!
If you log a message with
Yeah, we can do this, I think. Part of the problem is that dbt's |
Add node count/total node to log lines Make model status structured
|
I'm going to resolve that error/status issue by just calling |
core/dbt/node_runners.py
Outdated
| if result.error: | ||
| return {'model_status': 'error', 'model_error': str(result.error)} | ||
| elif result.skip: | ||
| return {'model_status': 'skipped'} | ||
| elif result.fail: | ||
| return {'model_status': 'failed'} | ||
| elif result.warn: | ||
| return {'model_status': 'warn'} | ||
| else: | ||
| return {'model_status': 'passed'} |
There was a problem hiding this comment.
can we change all of these from model_status to node_status for consistency? Many of these results will be generated from nodes which are not models.
core/dbt/node_runners.py
Outdated
| @@ -436,6 +448,12 @@ def on_skip(self): | |||
| 'Freshness: nodes cannot be skipped!' | |||
| ) | |||
|
|
|||
| def get_result_status(self, result) -> Dict[str, str]: | |||
| if result.error: | |||
| return {'model_status': 'error', 'model_error': str(result.error)} | |||
There was a problem hiding this comment.
Let's also make this node_error instead of model_error
core/dbt/logger.py
Outdated
| ('schema', 'node_schema'), | ||
| ('database', 'node_database'), | ||
| ('name', 'node_name'), | ||
| ('original_file_path', 'node_path') |
There was a problem hiding this comment.
can we also include the resource type here?
s/model/node/ include resource_type
Fixes #1799
This is based on #1801, so once that is merged I'll rebase this against dev/louisa-may-alcott.