fivetran / dbt_fivetran_log

Data models for Fivetran's internal log connector built using dbt.
https://fivetran.github.io/dbt_fivetran_log/
Apache License 2.0
30 stars 24 forks source link

[Bug] unique_fivetran_platform__audit_user_activity_log_id not using log composite key #100

Closed kieronellis closed 8 months ago

kieronellis commented 9 months ago

Is there an existing issue for this?

Describe the issue

Fivetran platform's Log table is not unique on the id field but is a composite key of id and timestamp - see ERD image

Therefore testing uniqueness of log_id in fivetran_platform__audit_user_activity model does not make sense - i.e. uniqueness of combination of columns log_id and occurred_at would needed (or the field log_id needs to become a combination of id and timestamp from the table log).

Relevant error log or model output

10:00:35  Completed with 1 error and 0 warnings:
10:00:35  
10:00:35  Failure in test unique_fivetran_platform__audit_user_activity_log_id (models/fivetran_platform.yml)
10:00:35    Got 112 results, configured to fail if != 0
10:00:35  
10:00:35    compiled Code at target/compiled/fivetran_log/models/fivetran_platform.yml/unique_fivetran_platform__audit_user_activity_log_id.sql

Expected behavior

model test passes

dbt Project configurations

vars: fivetran_log: fivetran_platform_database: ___

Package versions

packages:

What database are you using dbt with?

bigquery

dbt Version

Core:

Plugins:

Additional Context

No response

Are you willing to open a PR to help address this issue?

fivetran-jamie commented 9 months ago

Hey there! you're totally right -- we actually implement the correct test on the staging log table, but it appears we forgot to persist that testing grain in the new fivetran_platform__audit_user_activity model.

I'm working on another PR and can easily fold this fix into the same upcoming release (should be out by mid next week) if that sounds good to you.

fivetran-jamie commented 9 months ago

@kieronellis I have a working branch if you'd like to test it out -- i was not able to reproduce the error with our own log data so would appreciate your stamp of approval 😄

packages:
  - git: https://github.com/fivetran/dbt_fivetran_log.git
    revision: bug/audit-user-activity-test-grain
    warn-unpinned: false
kieronellis commented 9 months ago

@fivetran-jamie I could but we are relying on this package being stable, and this will be entirely the case when the logs explorer is removed from the Fivetran UI in a future update, so me manually testing your changes is not sustainable.

Could you instead create test cases which can be used to validate the code in this package for both this release and also for all future releases.

fivetran-jamie commented 9 months ago

Ah yes I understand -- if you have a development schema or environment feel free to test there, but otherwise we are confident with the fix and can certainly stll move forward with implementing.

fivetran-jamie commented 8 months ago

this fix is live in v1.4.0 of the package!