Skip to content

Change the data type for ChargePeriodStart and ChargePeriodEnd to date to prevent issues with the runningtotal measures when more than one billing profile is present. #1626

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

MSBrett
Copy link
Contributor

@MSBrett MSBrett commented May 22, 2025

🛠️ Description

Change the data type for ChargePeriodStart and ChargePeriodEnd to date (was datetimezone) to prevent issues with the runningtotal measures when more than one billing profile is present. This issue seems to be related to some costs landing in with dates that don't align to the hour.

Fixes #1614

📷 Screenshots

📋 Checklist

🔬 How did you test this change?

  • 🤏 Lint tests
  • 🤞 PS -WhatIf / az validate
  • 👍 Manually deployed + verified
  • 💪 Unit tests
  • 🙌 Integration tests

🙋‍♀️ Do any of the following that apply?

  • 🚨 This is a breaking change.
  • 🤏 The change is less than 20 lines of code.

📑 Did you update docs/changelog.md?

  • ✅ Updated changelog (required for dev PRs)
  • ➡️ Will add log in a future PR (feature branch PRs only)
  • ❎ Log not needed (small/internal change)

📖 Did you update documentation?

  • ✅ Public docs in docs (required for dev)
  • ✅ Internal dev docs in src (required for dev)
  • ➡️ Will add docs in a future PR (feature branch PRs only)
  • ❎ Docs not needed (small/internal change)

@microsoft-github-policy-service microsoft-github-policy-service bot added Micro PR 🔬 Very small PR that should be especially easy for newcomers Needs: Review 👀 PR that is ready to be reviewed labels May 22, 2025
@MSBrett MSBrett assigned MSBrett and unassigned flanakin and arthurclares May 22, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request updates the data types for ChargePeriodStart and ChargePeriodEnd from datetimezone to date to resolve running total measure issues in Power BI reports. It revises the column definitions in the KQL table, updates the transformation logic in the query, and refreshes the related documentation and changelog.

  • Updated column definitions in the Costs.tmdl file to use date instead of datetimezone.
  • Added a transformation step to cast the ChargePeriod columns to date in the KQL query.
  • Updated documentation and changelog to reflect the new data type.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/power-bi/kql/Shared.Dataset/definition/tables/Costs.tmdl Updated column definitions and query transformation for ChargePeriodStart and ChargePeriodEnd.
docs/power-bi.md Updated documentation to detail the data type change for ChargePeriodStart.
docs-mslearn/toolkit/changelog.md Updated changelog with the data type change details.

@MSBrett MSBrett changed the title Msbrett/issue1614 Change the data type for ChargePeriodStart and ChargePeriodEnd to date to prevent issues with the runningtotal measures when more than one billing profile is present. May 22, 2025
@MSBrett MSBrett requested a review from Copilot May 22, 2025 16:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes the data type for ChargePeriodStart and ChargePeriodEnd in Power BI KQL reports from datetimezone to date to resolve issues with running total measures when multiple billing profiles exist. Key changes include updating the column properties in the Costs.tmdl file (format string, source provider type, and annotations), modifying the M-code transformation step to convert these columns to type date, and updating supporting documentation and changelog entries.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/power-bi/kql/Shared.Dataset/definition/tables/Costs.tmdl Updates the column definitions and transformation logic for ChargePeriodStart and ChargePeriodEnd.
docs/power-bi.md Adds documentation about the data model change regarding ChargePeriodStart column.
docs-mslearn/toolkit/changelog.md Adds log entry explaining the change of data types for ChargePeriodStart and ChargePeriodEnd.
Comments suppressed due to low confidence (2)

src/power-bi/kql/Shared.Dataset/definition/tables/Costs.tmdl:252

  • The column's dataType property remains set to 'dateTime' despite the underlying sourceProviderType and transformations switching to a date. For clarity and consistency, consider updating the dataType to 'date'.
column ChargePeriodEnd

src/power-bi/kql/Shared.Dataset/definition/tables/Costs.tmdl:1841

  • [nitpick] Consider using a naming convention without spaces for the transformation step (e.g., "ChangedType") to maintain consistency and improve maintainability.
#"Changed Type" = Table.TransformColumnTypes(Source,{{"ChargePeriodEnd", type date}, {"ChargePeriodStart", type date}}),

@MSBrett MSBrett marked this pull request as ready for review May 22, 2025 16:11
@MSBrett MSBrett enabled auto-merge (squash) May 22, 2025 16:36
@MSBrett MSBrett added Tool: Power BI Power BI reports and removed Micro PR 🔬 Very small PR that should be especially easy for newcomers labels May 22, 2025
@MSBrett MSBrett added Resolution: Automerge This pull request can be merged automatically and removed Needs: Review 👀 PR that is ready to be reviewed labels May 22, 2025
column ChargePeriodStart
dataType: dateTime
formatString: Mmm d, yyyy
sourceProviderType: datetimeoffset
formatString: Short Date
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert

lineageTag: 015b4c1a-f072-4c68-8f1b-d5e10c54b97e
summarizeBy: none
sourceColumn: ChargePeriodStart

annotation SummarizationSetBy = Automatic

annotation PBI_FormatHint = {"isCustom":true}
annotation UnderlyingDateTimeDataType = Date
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert

Comment on lines +200 to +215

## Data Model Update: ChargePeriodStart Column for KQL based reports

- **Column:** `ChargePeriodStart`
- **Old Data Type:** `datetimezone`
- **New Data Type:** `date`
- **Reason for Change:**
- To ensure correct grouping and aggregation by day in Power BI and downstream analytics tools.
- This change eliminates issues with multiple rows per day caused by time and timezone components.

**Note:**

- All queries, visuals, and documentation should now treat `ChargePeriodStart` as a `date` type.
- If you previously used DAX or Power Query workarounds to normalize this column, you can now use it directly for daily grouping.

<br>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert

@@ -251,25 +251,27 @@ table Costs

column ChargePeriodEnd
dataType: dateTime
formatString: General Date
sourceProviderType: datetimeoffset
formatString: Short Date
Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically, we should probably use the same format as start dates.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Attention 👋 Issue or PR needs to be reviewed by the author or it will be closed due to no activity label May 26, 2025

@@MSBrett: you have some new feedback!

Please review and resolve all comments and I'll let reviewers know by removing the Needs: Attention label. If I miss anything, just reply with #needs-review and I'll update the status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Attention 👋 Issue or PR needs to be reviewed by the author or it will be closed due to no activity Resolution: Automerge This pull request can be merged automatically Tool: Power BI Power BI reports
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Total savings number is incorrect
3 participants