feature/country-report #24
Conversation
CHANGELOG.md
Outdated
| **4 total changes • 0 possible breaking changes | ||
| | Table/Column | Change Type | Old Name | New Name | Notes | | ||
| |---------------------------------------------------|-------------|----------|-------------------------------------------|-------------------------------------------------------------------| | ||
| | tiktok_ads__campaign_country_report | New Model | | | New table that represents the daily performance of a campaign at the country/geographic region level. | |
There was a problem hiding this comment.
Do you think we should we add links to these models in the DAG?
There was a problem hiding this comment.
Agreed - added
CHANGELOG.md
Outdated
| | Table/Column | Change Type | Old Name | New Name | Notes | | ||
| |---------------------------------------------------|-------------|----------|-------------------------------------------|-------------------------------------------------------------------| | ||
| | tiktok_ads__campaign_country_report | New Model | | | New table that represents the daily performance of a campaign at the country/geographic region level. | | ||
| | stg_tiktok_ads__campaign_country_report_tmp | New Model | | | Temp model added for `campaign_country_report`. | |
There was a problem hiding this comment.
Should we differentiate between Staging and Transform Models here? "New Staging Model" vs. "New Transformation Model" for example.
There was a problem hiding this comment.
Agreed - added
CHANGELOG.md
Outdated
|
|
||
| ## Feature Updates | ||
| - Added the `tiktok_ads__campaign_country_report` end model and upstream staging models. See above for schema change details and new models added. | ||
| - For dbt Core users: If you do not sync this table or would like disable these new models you can disable the models by setting the `tiktok_ads__using_campaign_country_report` variable to `false` in your `dbt_project.yml` file (`true` by default). |
There was a problem hiding this comment.
Should we add a link to the README?
There was a problem hiding this comment.
Agreed - added
CHANGELOG.md
Outdated
| - Added the `tiktok_ads__campaign_country_report` end model and upstream staging models. See above for schema change details and new models added. | ||
| - For dbt Core users: If you do not sync this table or would like disable these new models you can disable the models by setting the `tiktok_ads__using_campaign_country_report` variable to `false` in your `dbt_project.yml` file (`true` by default). | ||
| - Included the `tiktok_ads__campaign_country_report_passthrough_metrics` passthrough variable in the above mentioned new staging models. Refer to the [README](https://github.com/fivetran/dbt_tiktok_ads/tree/main?tab=readme-ov-file#passing-through-additional-metrics) for more details. | ||
| - Introduced the above mentioned new columns to the `stg_tiktok_ads__campaign_history` model. |
There was a problem hiding this comment.
Is this repetitive with line 11 of the Schema Changes table?
There was a problem hiding this comment.
Agreed - added
README.md
Outdated
| @@ -89,6 +90,13 @@ vars: | |||
|
|
|||
| To connect your multiple schema/database sources to the package models, follow the steps outlined in the [Union Data Defined Sources Configuration](https://github.com/fivetran/dbt_fivetran_utils/tree/releases/v0.4.latest#union_data-source) section of the Fivetran Utils documentation for the union_data macro. This will ensure a proper configuration and correct visualization of connections in the DAG. | |||
|
|
|||
| #### Disable Country Reports | |||
| This package leverages the `campaign_country_report` table to help report on ad and campaign performance by country. However, if you are not actively syncing this report from your TikTok Ads connection, you may disable the transformations for the `campaign_country_report` by adding the following variable configuration to your root `dbt_project.yml` file: | |||
There was a problem hiding this comment.
| This package leverages the `campaign_country_report` table to help report on ad and campaign performance by country. However, if you are not actively syncing this report from your TikTok Ads connection, you may disable the transformations for the `campaign_country_report` by adding the following variable configuration to your root `dbt_project.yml` file: | |
| This package leverages the `campaign_country_report` table to help report on ad and campaign performance by country and region. However, if you are not actively syncing this report from your TikTok Ads connection, you may disable the transformations for the `campaign_country_report` by adding the following variable configuration to your root `dbt_project.yml` file: |
There was a problem hiding this comment.
See same comment from the source. It's not entirely accurate to claim and region here. Instead we'll clarify country/geographic region level.
dbt_project.yml
Outdated
| @@ -15,6 +15,10 @@ vars: | |||
| ad_report_hourly: "{{ ref('stg_tiktok_ads__ad_report_hourly') }}" | |||
| ad_history: "{{ ref('stg_tiktok_ads__ad_history') }}" | |||
| advertiser: "{{ ref('stg_tiktok_ads__advertiser') }}" | |||
| ad_country_report: "{{ ref('stg_tiktok_ads__ad_country_report') }}" | |||
There was a problem hiding this comment.
This appears to no longer being built, so it can be removed.
There was a problem hiding this comment.
Correct, this was initially included and then removed. Removing the artifact.
dbt_project.yml
Outdated
| tiktok_ads__ad_group_hourly_passthrough_metrics: [] | ||
| tiktok_ads__ad_hourly_passthrough_metrics: [] | ||
| tiktok_ads__campaign_hourly_passthrough_metrics: [] | ||
| tiktok_ads__campaign_country_report_passthrough_metrics: [] | ||
| tiktok_ads__ad_country_report_passthrough_metrics: [] |
There was a problem hiding this comment.
This macro seems to no longer being built, so it can be removed.
There was a problem hiding this comment.
Correct, this was initially included and then removed. Removing the artifact.
|
|
||
| select | ||
| country_report.source_relation, | ||
| cast(country_report.stat_time_day as date) as date_day, |
There was a problem hiding this comment.
Is there a reason we cast to date_day in the transformation model rather than the staging model?
There was a problem hiding this comment.
No reason, but we should cast at staging model. Made the update in the source package and removing this cast here.
models/tiktok_ads.yml
Outdated
| description: Total number of clicks. | ||
| - name: impressions | ||
| description: Total number of impressions. | ||
| - name: spend | ||
| description: Total spend in the advertiser’s currency. | ||
| - name: conversion | ||
| description: Total number of tracked conversions. | ||
| - name: conversion_rate | ||
| description: Conversion rate as a percentage. | ||
| - name: cost_per_conversion | ||
| description: Average cost per conversion. | ||
| - name: daily_cpc | ||
| description: Cost per click. | ||
| - name: daily_cpm | ||
| description: Cost per mille (1,000 impressions). | ||
| - name: daily_ctr | ||
| description: Click-through rate. | ||
| - name: real_time_conversion | ||
| description: Total number of real-time conversions reported. |
There was a problem hiding this comment.
I think you could replicate the yml descriptions for the metrics for what is shown on the other reports to add more details.
A few examples from the tiktok_ads__campaign report:
- name: impressions description: The number of impressions that occurred on the day of the record. - name: clicks description: The number of clicks that occurred on the day of the record. - name: spend description: The amount of spend that occurred on the day of the record. - name: reach description: The number of unique users who saw your ads at least once. This metric is estimated. - name: conversion description: > The number of times your ad achieved an outcome, based on the secondary goal you selected. As one campaign may have a number of different secondary goals, this statistic is not supported for campaigns. Please go to ad groups or ads to view. (The total count is calculated based on the time each ad impression occurred.)
There was a problem hiding this comment.
Agreed. Updated to match other metric defs from existing models
|
|
||
| - name: tiktok_ads__campaign_country_report | ||
| description: Each record in this table represents the daily performance of a campaign at the country/geographic region level. |
There was a problem hiding this comment.
If this model is representing country and geographic regions, should we rename the model to be more generic, like tiktok_ads__campaign_geographic_report? Or is the plurality of data for country_code usually countries?
There was a problem hiding this comment.
Yeah the plurality of data for country_code is typically countries with a few outliers based on TikTok Ads API docs. We opted to call out the geographic regions in the description to highlight that there are some outliers, but they are not the majority. Additionally, since there are only a few outliers and not complete support for regions, I didn't want to name the model as such.
Lastly, I would prefer the name of the model match the source name as closely as possible.
| end_count_unique_days != staging_count_unique_days or | ||
| end_total_impressions != staging_total_impressions or | ||
| end_total_clicks != staging_total_clicks or | ||
| end_total_spend != staging_total_spend or |
There was a problem hiding this comment.
With monetary floats, I've always noticed that the numbers can deviate by an insigifcant amount to throw errors that would not be logged in the financial reports. Perhaps this would be better:
| end_total_spend != staging_total_spend or | |
| abs(end_total_spend - staging_total_spend) > 0.01 or |
There was a problem hiding this comment.
Good approach and helps to future proof any false positives. Added.
| (sum(country_report.clicks)/nullif(sum(country_report.impressions),0))*100 as daily_ctr, | ||
| sum(country_report.real_time_conversion) as real_time_conversion | ||
|
|
||
| {{ tiktok_ads_persist_pass_through_columns(pass_through_variable='tiktok_ads__campaign_country_report_passthrough_metrics', identifier='country_report', transform='sum', coalesce_with=0, exclude_fields=['real_time_conversion','total_purchase_value','total_sales_lead_value']) }} |
There was a problem hiding this comment.
I'm not sure we need the exclude_fields here since this is a net new model and backwards compatibility isn't required. What are your thoughts?
There was a problem hiding this comment.
(I also don't think total_purchase_value' and 'total_sales_lead_value are even present in the campaign_country_report)
There was a problem hiding this comment.
Agreed, removed the exclude fields. This was an artifact of initial model build and emulating other models in this package.
There was a problem hiding this comment.
@fivetran-joemarkiewicz Great work so far! A few comments, questions and suggestions before approval.
Co-authored-by: Avinash Kunnath <[email protected]>
CHANGELOG.md
Outdated
|
|
||
| [PR #24](https://github.com/fivetran/dbt_tiktok_ads/pull/24) includes the following updates: | ||
| ## Schema Changes | ||
| **4 total changes • 0 possible breaking changes |
There was a problem hiding this comment.
I presume we want this bolded, so:
| **4 total changes • 0 possible breaking changes | |
| **4 total changes • 0 possible breaking changes** |
There was a problem hiding this comment.
Update applied.
| sum(country_report.impressions) as impressions, | ||
| sum(country_report.spend) as spend, | ||
| sum(country_report.conversion) as conversion, | ||
| sum(country_report.conversion_rate) as conversion_rate, |
There was a problem hiding this comment.
It doesn't make sense to sum rates so I don't think conversion_rate and cost_per_conversion should be included. I'm not sure there's a way to derive it either given that they come as is in the staging model
There was a problem hiding this comment.
Agreed - made the relevant updates.
PR Overview
Package version introduced in this PR:
v0.8.0This PR addresses the following Issue/Feature(s): Internal ticket
Summary of changes:
Introduces the new
tiktok_ads__campaign_country_report.Submission Checklist
Changelog