Increase service spend coverage #173
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fernandobrito
requested a deployment
to
Approve Integration Tests
GitHub Actions
Waiting
fernandobrito
temporarily deployed
to
Approve Integration Tests
GitHub Actions
Inactive
fernandobrito
commented
Feb 6, 2025
models/hourly_spend.sql
Outdated
Comment on lines
462
to
489
| materialized_view_spend_hourly as ( | ||
| select | ||
| hours.hour, | ||
| 'Materialized Views' as service, | ||
| null as storage_type, | ||
| null as warehouse_name, | ||
| null as database_name, | ||
| coalesce( | ||
| sum( | ||
| stg_metering_history.credits_used * daily_rates.effective_rate | ||
| ), | ||
| 0 | ||
| ) as spend, | ||
| spend as spend_net_cloud_services, | ||
| any_value(daily_rates.currency) as currency | ||
| from hours | ||
| left join {{ ref('stg_metering_history') }} as stg_metering_history on | ||
| hours.hour = convert_timezone( | ||
| 'UTC', stg_metering_history.start_time | ||
| ) | ||
| and stg_metering_history.service_type = 'MATERIALIZED_VIEW' | ||
| left join {{ ref('daily_rates') }} as daily_rates | ||
| on hours.hour::date = daily_rates.date | ||
| and daily_rates.service_type = 'MATERIALIZED_VIEW' | ||
| and daily_rates.usage_type = 'materialized views' | ||
| group by 1, 2, 3, 4 | ||
| ), | ||
|
|
There was a problem hiding this comment.
This change will "rename" this service from Materialized Views to Materialized View. If that's a problem, we should revert this part.
There was a problem hiding this comment.
As discussed we're going to keep the 'Materialized Views' name
There was a problem hiding this comment.
I updated the code to rename it from the new source to Materialized Views and tested it with real data
fernandobrito
deployed
to
Approve Integration Tests
GitHub Actions
Active
|
I was able to remove 2 more CTEs by renaming the service from |
NiallRees
approved these changes
Feb 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are new Snowflake services (such as SENSITIVE_DATA_CLASSIFICATION, DATA_QUALITY_MONITORING, SERVERLESS_ALERTS, TRUST_CENTER) which are not covered today by our
hourly_spendbecause we have 1 CTE per service.This PR adds a new CTE to catch any service that is in
metering_historyand currently not covered by ourhourly_spend.