feat: Add post-parse model configuration telemetry - #1665
Conversation
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||
saishreeeee
changed the title
Drop kitchen-sink and kwargs-forwarding coverage that restated collector maps, and retain cases that can fail without a matching production change.
Merge knobs, Z-order, and constraints were inflating adoption for strategies and formats the macros ignore, and v2 catalog_database HMS routing was labeled Unity.
…emetry V2 never applies persist_constraints metadata, and CLUSTER BY AUTO is ignored when explicit liquid clustering columns are set.
Match get_model_constraints and get_column_constraints so telemetry does not union legacy meta with contract constraints at the same level.
Count MV contract constraints, streaming-table column NOT NULL and masks, V2 table/incremental masks, and drop V1 Python-table auto-liquid and row-filter false positives.
Dedicated CHECK is routed to alter_constraints, which the MV path never applies; keep NOT NULL, PK, FK, and CUSTOM.
Keep only contracts that can fail without rewriting the collector maps, and remove lookup-table and default-forwarding cases.
Stop predicting runtime applicability so POST_PARSE reports configured intent, including conflicts; keep resolution on effective_* fields.
| def _constraint_configs(node: Any) -> set[models.ModelConfig]: | ||
| constraint_names = _constraint_type_keys(getattr(node, "constraints", None)) | ||
| meta = getattr(node, "meta", None) or {} | ||
| constraint_names.update(_constraint_type_keys(_value(meta, "constraints"))) |
There was a problem hiding this comment.
Please restore the list/tuple check for meta.constraints. dbt accepts arbitrary metadata such as meta: {constraints: true}; this now raises TypeError and silently drops the entire POST_PARSE event. Confirmed against a real parsed manifest; the previous head succeeds. Keep declaration counting unconditional, but ignore unsupported legacy metadata shapes and add a regression test.
| assert config.is_eligible_command() is eligible | ||
|
|
||
|
|
||
| class TestTransportEligibility: |
There was a problem hiding this comment.
Please retain compact tests for default-off consent, explicit opt-in, and eligible/ineligible commands. The hook tests mock is_enabled_for_invocation, so they don’t cover this boundary.
| } | ||
|
|
||
|
|
||
| def _config_usage(node: Any, config: Any) -> set[models.ModelConfig]: |
There was a problem hiding this comment.
Could we add a short docstring to _config_usage explaining this intentional boundary? Suggested wording: “POST_PARSE measures resolved configuration intent, so declarations are counted independently of runtime applicability.” Keep effective_* resolution separate. This should help future maintainers avoid reintroducing execution gates.
Arbitrary meta.constraints values were iterable and dropped the whole event.
2 participants
Description
Adds aggregate model-configuration statistics to the existing opt-in POST_PARSE telemetry event. The payload now captures:
The values are aggregated counts and do not include model names or configuration values. The post-parse hook supplies catalog resolution through the adapter so effective catalog and storage choices are recorded consistently.
Testing
Ran dbt build with telemetry opted in and manually verified the POST_PARSE and POST_RUN events landed in the dbt telemetry table
Checklist
CHANGELOG.mdand added information about my change to the "dbt-databricks next" section.dbt-databricks-pr-readyproject skill for this PR and addressed its merge-readiness feedback