Adding Advisor and Resource Graph recommendations exports to Hubs #1130

helderpinto · 2024-11-16T12:07:23Z

🛠️ Description

This is the first PR in a series of PRs that will add Advisor and Resource Graph recommendations exports to Hubs. This PR includes the initial Advisor/ARG recommendations export and the schema for recommendations. It is compatible with Data Explorer ingestion. For this PR, we also fixed Western Europe timezones.

The upcoming PRs will address documentation, reporting, and granting permissions to the ADF identity at a higher scope, for the recommendations exports to succeed. Currently, Reader permissions have to be granted manually at the subscription or management group level.

📋 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)

src/templates/finops-hub/modules/azuretimezones.bicep

src/templates/finops-hub/modules/scripts/IngestionSetup.kql

src/templates/finops-hub/modules/dataFactory.bicep

Co-authored-by: Michael Flanakin <flanakin@users.noreply.github.com>

…b.com/helderpinto/finops-toolkit into helderpinto/dev/recommendationsexport

Co-authored-by: Michael Flanakin <flanakin@users.noreply.github.com>

…b.com/helderpinto/finops-toolkit into helderpinto/dev/recommendationsexport

helderpinto · 2024-12-26T12:17:14Z

@flanakin, the ADF pipeline was refactored to become query type-agnostic. It is now prepared to deal with other query types in the future. There are still some unresolved conversations. Feel free to ping me offline or let's set up a meeting to discuss this further.

flanakin · 2025-02-10T21:27:05Z

src/templates/finops-hub/modules/dataFactory.bicep

@@ -799,6 +863,24 @@ resource dataset_ftkReleaseFile 'Microsoft.DataFactory/factories/datasets@2018-0
  }
 }

+resource dataset_resourcegraph 'Microsoft.DataFactory/factories/datasets@2018-06-01' = {
+  name: 'resourcegraph'


nit: Do we want to standardize on camelcasing? I think we have some inconsistent casing right now and some of that can't be fixed but do we have a strong preference on which way we go?

Suggested change

name: 'resourcegraph'

name: 'resourceGraph'

flanakin · 2025-02-10T21:28:59Z

src/templates/finops-hub/modules/dataFactory.bicep

-        frequency: 'Hour'
-        interval: 24
-        startTime: '2023-01-01T01:01:00'
+        frequency: 'Day'


I would keep this as hourly so customers can easily change it to be every 12 hours or whatever frequency they prefer.

flanakin · 2025-02-10T21:29:33Z

src/templates/finops-hub/modules/dataFactory.bicep

+      {
+        // Get Config


Undo all of these. We want them to be on the same line as the { so they are visible when collapsed.

flanakin · 2025-02-10T21:34:26Z

src/templates/finops-hub/modules/dataFactory.bicep

+            type: 'DatasetReference'
+            parameters: {
+              fileName: 'settings.json'
+              folderPath: '${configContainerName}/queries'


Thinking out loud: We will need configuration for CM exports as well. Should we follow this same pattern? And if so, is "queries" the right folder name? Should we use something more generic, like "datasets" or maybe something else? I'm not too picky. Just asking to be forward-looking.

flanakin · 2025-02-11T08:26:52Z

src/templates/finops-hub/modules/dataFactory.bicep

+        typeProperties: {
+          variableName: 'blobExportTimestamp'
+          value: {
+            value: '@concat(utcNow(\'yyyy\'),\'/\',utcNow(\'MM\'),\'/\',utcNow(\'dd\'),\'/\')'


Do we need to account for data that we don't want to duplicate by day? Do we anticipate hourly, monthly, or even no historical scenarios? Happy to handle that later. I just want to make sure we have a plan for how we'll handle it that doesn't require moving data around. That tends to be painful.

flanakin · 2025-02-11T09:14:20Z

src/templates/finops-hub/modules/scripts/IngestionSetup.kql

@@ -1519,18 +1635,41 @@ Recommendations_transform_v1_0()

 // Recommendations_final_v1_0 table
 .create-merge table Recommendations_final_v1_0 (


This table shouldn't change

flanakin · 2025-02-11T09:16:17Z

src/templates/finops-hub/modules/storage.bicep

  'reservationdetails_2023-03-01': loadTextContent('../schemas/reservationdetails_2023-03-01.json')
  'reservationrecommendations_2023-05-01_ea': loadTextContent('../schemas/reservationrecommendations_2023-05-01_ea.json')
  'reservationrecommendations_2023-05-01_mca': loadTextContent('../schemas/reservationrecommendations_2023-05-01_mca.json')
  'reservationtransactions_2023-05-01_ea': loadTextContent('../schemas/reservationtransactions_2023-05-01_ea.json')
  'reservationtransactions_2023-05-01_mca': loadTextContent('../schemas/reservationtransactions_2023-05-01_mca.json')
 }
+var queryFiles = {
+  'HubsRecommendations-AdvisorCost': loadTextContent('./scripts/HubsRecommendations-AdvisorCost.json')


Shouldn't these be in the queries folder?

Suggested change

'HubsRecommendations-AdvisorCost': loadTextContent('./scripts/HubsRecommendations-AdvisorCost.json')

'HubsRecommendations-AdvisorCost': loadTextContent('./queries/HubsRecommendations-AdvisorCost.json')

flanakin · 2025-02-11T09:19:15Z

src/templates/finops-hub/schemas/recommendations_1.0.json

+        "mappings": [
+            {
+                "source": {
+                    "path": "[['ResourceId']"


Is this path correct? It doesn't look right with the extra [. Same for the others.

flanakin · 2025-02-11T09:22:05Z

src/templates/finops-hub/schemas/recommendations_1.0.json

+            },
+            {
+                "source": {
+                    "path": "[['x_RecommendationImpact']"


Is the impact actually useful for customers? Can we stand by the impact assessment for any of these recommendations? I've never trusted it, but I also haven't had a conversation with customers about it.

flanakin · 2025-02-11T09:23:08Z

src/templates/finops-hub/schemas/recommendations_1.0.json

+            },
+            {
+                "source": {
+                    "path": "[['x_RecommendationMaturityLevel']"


What values does this have? Who assesses maturity? Is it standardized?

microsoft-github-policy-service · 2025-02-11T09:24:55Z

@helderpinto: 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.

Helder Pinto added 7 commits November 8, 2024 23:55

Advisor recommendations export initial

91a592a

Added recommendations schema

b767eda

Fixed West Europe timezones

0495762

Fixed missing variables

ef16646

Generalized ARG queries exports

666f6b4

Recommendations queries added to hub config

3d0c6aa

Included hubs add-ons in ADF queries

a2717b6

helderpinto added the Tool: FinOps hubs Data pipeline solution label Nov 16, 2024

helderpinto requested a review from MSBrett as a code owner November 16, 2024 12:07

microsoft-github-policy-service bot requested review from arthurclares and flanakin November 16, 2024 12:07

microsoft-github-policy-service bot added the Needs: Review 👀 PR that is ready to be reviewed label Nov 16, 2024

microsoft-github-policy-service bot assigned arthurclares, flanakin and MSBrett Nov 16, 2024

flanakin added this to the 2024-12 - December milestone Nov 16, 2024

flanakin modified the milestones: 2024-12 - December, 2025-01 - January Dec 14, 2024

flanakin requested changes Dec 19, 2024

View reviewed changes

microsoft-github-policy-service bot added Needs: Attention 👋 Issue or PR needs to be reviewed by the author or it will be closed due to no activity and removed Needs: Review 👀 PR that is ready to be reviewed labels Dec 19, 2024

microsoft-github-policy-service bot assigned helderpinto Dec 19, 2024

Changed to reference dataset_ingestion name

9247663

Co-authored-by: Michael Flanakin <flanakin@users.noreply.github.com>

microsoft-github-policy-service bot requested a review from flanakin December 20, 2024 14:46

microsoft-github-policy-service bot added Needs: Review 👀 PR that is ready to be reviewed and removed Needs: Attention 👋 Issue or PR needs to be reviewed by the author or it will be closed due to no activity labels Dec 20, 2024

Helder Pinto added 2 commits December 20, 2024 15:07

Addressing review comments

b6ff34c

Merge branch 'helderpinto/dev/recommendationsexport' of https://githu…

d6bb7a6

…b.com/helderpinto/finops-toolkit into helderpinto/dev/recommendationsexport

helderpinto changed the base branch from dev to features/recs December 20, 2024 15:09

Merge branch 'features/recs' into helderpinto/dev/recommendationsexport

306723b

Helder Pinto and others added 6 commits December 24, 2024 15:18

Refactored ARG exports pipeline to become query type agnostic

074f8c6

Renamed AdditionalInfo to RecommendationDetails

b302735

Co-authored-by: Michael Flanakin <flanakin@users.noreply.github.com>

Addresses PR review comments

e216791

Merge branch 'helderpinto/dev/recommendationsexport' of https://githu…

1dba5f8

…b.com/helderpinto/finops-toolkit into helderpinto/dev/recommendationsexport

Adresses PR review comment

20e7da4

Addresses PR review comments

bddeeff

Generalized query error handling

a3aa381

microsoft-github-policy-service bot requested review from ro100e and RolandKrummenacher January 5, 2025 21:13

microsoft-github-policy-service bot assigned ro100e and RolandKrummenacher Jan 5, 2025

Helder Pinto added 2 commits January 12, 2025 21:51

Updated Recommendations table version to 1.0r2

4ad0507

Fixed HubSetup and changed queries schedule

b88a60a

flanakin requested changes Feb 11, 2025

View reviewed changes

microsoft-github-policy-service bot added Needs: Attention 👋 Issue or PR needs to be reviewed by the author or it will be closed due to no activity and removed Needs: Review 👀 PR that is ready to be reviewed labels Feb 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Advisor and Resource Graph recommendations exports to Hubs #1130

Adding Advisor and Resource Graph recommendations exports to Hubs #1130

helderpinto commented Nov 16, 2024

helderpinto commented Dec 26, 2024

flanakin Feb 10, 2025

flanakin Feb 10, 2025

flanakin Feb 10, 2025

flanakin Feb 10, 2025

flanakin Feb 11, 2025

flanakin Feb 11, 2025

flanakin Feb 11, 2025

flanakin Feb 11, 2025

flanakin Feb 11, 2025

flanakin Feb 11, 2025

microsoft-github-policy-service bot commented Feb 11, 2025

		@@ -1519,18 +1635,41 @@ Recommendations_transform_v1_0()

		// Recommendations_final_v1_0 table
		.create-merge table Recommendations_final_v1_0 (

	'HubsRecommendations-AdvisorCost': loadTextContent('./scripts/HubsRecommendations-AdvisorCost.json')
	'HubsRecommendations-AdvisorCost': loadTextContent('./queries/HubsRecommendations-AdvisorCost.json')

Adding Advisor and Resource Graph recommendations exports to Hubs #1130

Are you sure you want to change the base?

Adding Advisor and Resource Graph recommendations exports to Hubs #1130

Conversation

helderpinto commented Nov 16, 2024

🛠️ Description

📋 Checklist

🔬 How did you test this change?

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

📑 Did you update docs/changelog.md?

📖 Did you update documentation?

helderpinto commented Dec 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

microsoft-github-policy-service bot commented Feb 11, 2025

📑 Did you update `docs/changelog.md`?