Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give wiggle room for selecting SLA policy names if timestamps are off by milliseconds #174

Merged
merged 4 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
# dbt_zendesk v0.18.1
[PR #174](https://github.com/fivetran/dbt_zendesk/pull/174) includes the following changes:

## Bug Fix
- Addressed an issue in which some records in `zendesk__sla_policies` might erroneously have a null `sla_policy_name` due to system-generated millisecond-long gaps in timestamps. The package now compares timestamps to the nearest `second` when selecting valid SLA policy names in [int_zendesk__sla_policy_applied](https://github.com/fivetran/dbt_zendesk/blob/main/models/sla_policy/int_zendesk__sla_policy_applied.sql).

## Under the Hood
- Updated `consistency_sla_policies` data validation test to account for the above change.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably combine the two sentences here into one, I think they were both to account for the above changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

combined

- Updated logic in `sla_count_match` data validation test.

# dbt_zendesk v0.18.0
[PR #171](https://github.com/fivetran/dbt_zendesk/pull/171) includes the following changes:

Expand Down
9 changes: 3 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ vars:
```

### (Optional) Step 5: Additional configurations
<details open><summary>Expand/Collapse details</summary>

#### Enabling the unstructured document model for NLP
This package includes the `zendesk__document` model, which processes and segments Zendesk text data for vectorization, making it suitable for NLP workflows. The model outputs structured chunks of text with associated document IDs, segment indices, and token counts. For definitions and more information, refer to [zendesk__document](https://fivetran.github.io/dbt_zendesk/#!/model/model.zendesk.zendesk__document) in our dbt docs.
Expand Down Expand Up @@ -171,9 +172,6 @@ This package will create a row in `zendesk__ticket_field_history` for each day t
```yml
# dbt_project.yml

...
config-version: 2

vars:
zendesk:
ticket_field_history_extension_months: integer_number_of_months # default = 0
Expand All @@ -184,9 +182,6 @@ Conversely, you may want to only track the past X years of ticket field history.
```yml
# dbt_project.yml

...
config-version: 2

vars:
zendesk:
ticket_field_history_timeframe_years: integer_number_of_years # default = 50 (everything)
Expand Down Expand Up @@ -219,6 +214,8 @@ vars:
zendesk_<default_source_table_name>_identifier: your_table_name
```

</details>

### (Optional) Step 6: Orchestrate your models with Fivetran Transformations for dbt Core™
<details><summary>Expand for details</summary>
<br>
Expand Down
2 changes: 1 addition & 1 deletion dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: 'zendesk'
version: '0.18.0'
version: '0.18.1'

config-version: 2
require-dbt-version: [">=1.3.0", "<2.0.0"]
Expand Down
2 changes: 1 addition & 1 deletion docs/catalog.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/manifest.json

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion integration_tests/dbt_project.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
config-version: 2

name: 'zendesk_integration_tests'
version: '0.18.0'
version: '0.18.1'

profile: 'integration_tests'

Expand Down Expand Up @@ -38,6 +38,7 @@ vars:
# using_organization_tags: false
# fivetran_integrity_sla_metric_parity_exclusion_tickets: (56,80)
# fivetran_integrity_sla_first_reply_time_exclusion_tickets: (56,80)
# fivetran_consistency_sla_policies_exclusion_tickets: (55,58) # can remove after this release

models:
+schema: "zendesk_{{ var('directed_schema','dev') }}"
Expand Down
29 changes: 25 additions & 4 deletions integration_tests/tests/consistency/consistency_sla_policies.sql
fivetran-jamie marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ with prod as (
target,
in_business_hours,
sla_breach_at,
round(sla_elapsed_time, -1) as sla_elapsed_time, --round to the nearest tens
sla_elapsed_time,
is_active_sla,
is_sla_breach
from {{ target.schema }}_zendesk_prod.zendesk__sla_policies
Expand All @@ -28,7 +28,7 @@ dev as (
target,
in_business_hours,
sla_breach_at,
round(sla_elapsed_time, -1) as sla_elapsed_time, --round to the nearest tens
sla_elapsed_time,
is_active_sla,
is_sla_breach
from {{ target.schema }}_zendesk_dev.zendesk__sla_policies
Expand All @@ -48,7 +48,7 @@ dev_not_in_prod as (
select * from prod
),

final as (
combine as (
select
*,
'from prod' as source
Expand All @@ -60,8 +60,29 @@ final as (
*,
'from dev' as source
from dev_not_in_prod
),

final as (
select
*,
max(sla_elapsed_time) over (partition by ticket_id, metric, sla_applied_at) as max_sla_elapsed_time,
min(sla_elapsed_time) over (partition by ticket_id, metric, sla_applied_at) as min_sla_elapsed_time,

{#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder to remove before merging!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

This is necessary for upgrading to v0.18.1, as it introduces a fix for erronesouly null sla_policy_name values. The union all will consider these distinct rows as a result
Remove this and following where clause afterward
#}
sum(case when sla_policy_name is null then 1 else 0 end) over (partition by ticket_id, metric, sla_applied_at) = 1 as name_was_null_prior

from combine
{{ "where ticket_id not in " ~ var('fivetran_consistency_sla_policies_exclusion_tickets',[]) ~ "" if var('fivetran_consistency_sla_policies_exclusion_tickets',[]) }}
)

select *
from final
{{ "where ticket_id not in " ~ var('fivetran_consistency_sla_policies_exclusion_tickets',[]) ~ "" if var('fivetran_consistency_sla_policies_exclusion_tickets',[]) }}
where
{# Take differences in runtime into account #}
max_sla_elapsed_time - min_sla_elapsed_time > 2

{# Remove after v0.18.1 #}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder to remove before merging!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

and NOT name_was_null_prior
2 changes: 1 addition & 1 deletion integration_tests/tests/integrity/sla_count_match.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
with source as (
select
*,
case when field_name = 'first_reply_time' then row_number() over (partition by ticket_id, field_name order by valid_starting_at desc) else 1 end as latest_sla
case when field_name = 'first_reply_time' then row_number() over (partition by ticket_id, field_name order by valid_starting_at) else 1 end as latest_sla
from {{ ref('stg_zendesk__ticket_field_history') }}
),

Expand Down
4 changes: 2 additions & 2 deletions models/sla_policy/int_zendesk__sla_policy_applied.sql
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ with ticket_field_history as (
from sla_policy_applied
left join sla_policy_name
on sla_policy_name.ticket_id = sla_policy_applied.ticket_id
and sla_policy_applied.valid_starting_at >= sla_policy_name.valid_starting_at
and sla_policy_applied.valid_starting_at < coalesce(sla_policy_name.valid_ending_at, {{ dbt.current_timestamp() }})
and {{ dbt.date_trunc("second", "sla_policy_applied.valid_starting_at") }} >= {{ dbt.date_trunc("second", "sla_policy_name.valid_starting_at") }}
and {{ dbt.date_trunc("second", "sla_policy_applied.valid_starting_at") }} < coalesce({{ dbt.date_trunc("second", "sla_policy_name.valid_ending_at") }}, {{ dbt.current_timestamp() }})
where sla_policy_applied.latest_sla = 1
)

Expand Down