Migration of cloud monitoring from observability-instances #3

iamar7 · 2025-03-14T10:34:56Z

Description

Cloud Monitoring & Metrics Router code from the observabilities-instances repo has been moved here and examples for this module created.

Release required?

No release
Patch release (x.x.X)
Minor release (x.X.x)
Major release (X.x.x)

Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

If relevant, a test for the change is included or updated with this PR.
If relevant, documentation for the change is included or updated with this PR.

For mergers

Use a conventional commit message to set the release level. Follow the guidelines.
Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
Use the Squash and merge option.

ocofaigh

I think Metrics routing should still be its own submodule. Otherwise we have no migration path for consumers migrating from old repo

version.tf

variables.tf

…erraform-ibm-cloud-monitoring into 12594-cm

iamar7 · 2025-03-21T13:47:52Z

/run pipeline

iamar7 · 2025-03-21T14:17:46Z

/run pipeline

iamar7 · 2025-03-24T09:13:03Z

Upgrade is failing which is expected

…form-ibm-cloud-monitoring into 12594-cm

iamar7 · 2025-03-24T09:21:36Z

/run pipeline

iamar7 · 2025-03-24T10:14:00Z

/run pipeline

iamar7 · 2025-03-24T11:16:19Z

/run pipeline

ocofaigh

Can you ensure all modules and examples are at required_version = ">= 1.9.0"

modules/metrics_routing/README.md

examples/advanced/README.md

This reverts commit 9d376c2.

iamar7 · 2025-03-26T18:19:03Z

/run pipeline

iamar7 · 2025-03-26T18:46:18Z

/run pipeline

examples/advanced/main.tf

…form-ibm-cloud-monitoring into 12594-cm

iamar7 · 2025-04-03T23:41:31Z

/run pipeline

ocofaigh

see comments

ocofaigh · 2025-04-04T17:12:53Z

README.md

-Use real values instead of "var.<var_name>" or other placeholder values
-unless real values don't help users know what to change.
-->
+To provision cloud monitoring instance


Hmm, the usage is showing metrics config too. I would suggest to just remove the sentence and use code comments in the example tf code itself to explain the usage

ocofaigh · 2025-04-04T17:16:00Z

outputs.tf

-# Outputs
-########################################################################################################################
+output "crn" {
+  value       = length(ibm_resource_instance.cloud_monitoring) > 0 ? ibm_resource_instance.cloud_monitoring.id : null


you can remove length function from all these inputs. there is no count around the instance creation.

outputs.tf

ocofaigh · 2025-04-04T18:11:34Z

modules/metrics_routing/variables.tf

+      id = string
+    })))
+  })
+  description = "Global settings for Metrics Routing"


We should mention that in order to configure metrics routing, the account must have a primary_metadata_region value set

Perhaps also mention the impact of setting private_api_endpoint_only to true (you are unable to see the settings in the UI)

modules/metrics_routing/variables.tf

ocofaigh · 2025-04-04T18:15:48Z

modules/metrics_routing/main.tf

+
+# metric routing to cloud monitoring s2s auth policy
+resource "ibm_iam_authorization_policy" "metrics_router_cloud_monitoring" {
+  for_each                    = { for target in var.metrics_router_targets : target.target_name => target if !target.skip_mrouter_sysdig_iam_auth_policy }


There should be a boolean to skip auth policy creation if required, same way we have in all other modules

skip_mrouter_sysdig_iam_auth_policy is the boolean to skip auth policy creation

I have renamed the variable to skip_metrics_router_auth_policy

modules/metrics_routing/main.tf

ocofaigh · 2025-04-04T18:18:54Z

examples/basic/main.tf

+##############################################################################

 #
 # Developer tips:


this can be removed its only needed in the initial template as a guide for developers creating a module example

ocofaigh · 2025-04-04T18:19:42Z

variables.tf

+
+variable "enable_platform_metrics" {
+  type        = bool
+  description = "Receive platform metrics in the provisioned IBM Cloud Monitoring instance."


Suggest to mention that only 1 instance in a given region can be enabled for platform metrics.

ocofaigh · 2025-04-04T18:26:42Z

examples/advanced/main.tf

+##############################################################################
+# IBM Cloud Metrics Routing
+#   - Cloud Monitoring target
+#   - Metrics Router route to the target


maybe explain more about the type of rule your creating in the example

iamar7 · 2025-04-09T08:44:23Z

/run pipeline

ocofaigh · 2025-04-09T12:14:25Z

modules/metrics_routing/main.tf

+  backup_metadata_region    = var.metrics_router_settings.backup_metadata_region
+  private_api_endpoint_only = var.metrics_router_settings.private_api_endpoint_only
+
+  # The create_before_destroy meta-argument makes sure that the new settings are applied first before the prior ones are removed.


@iamar7 I don't understand this comment? These settings can only have 1 value at any given time. So I don't understand how create_before_destroy could work in this scenario?

terraform-ibm-modules-ops · 2025-04-09T12:16:19Z

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Md Anam Raihan added 2 commits March 14, 2025 12:31

migrate cloud monitoring from observability

e03b66c

update test and codeowner

ebf0706

iamar7 requested review from daniel-butler-irl and ocofaigh as code owners March 14, 2025 10:34

iamar7 changed the title ~~migrate cloud monitoring from observability-instances~~ [WIP] migrate cloud monitoring from observability-instances Mar 14, 2025

Md Anam Raihan and others added 2 commits March 14, 2025 20:15

update code

f1984eb

Merge branch 'main' into 12594-cm

1a92ab1

ocofaigh requested changes Mar 18, 2025

View reviewed changes

version.tf Outdated Show resolved Hide resolved

version.tf Outdated Show resolved Hide resolved

variables.tf Outdated Show resolved Hide resolved

variables.tf Outdated Show resolved Hide resolved

variables.tf Outdated Show resolved Hide resolved

Md Anam Raihan added 2 commits March 21, 2025 19:16

add metrics router support

1d5d5ee

Merge branch '12594-cm' of https://github.com/terraform-ibm-modules/t…

96e15d5

…erraform-ibm-cloud-monitoring into 12594-cm

iamar7 self-assigned this Mar 21, 2025

resolves pc

b5d9f0c

iamar7 added 2 commits March 24, 2025 09:14

SKIP UPGRADE TEST

8902557

Merge branch 'main' of https://github.com/terraform-ibm-modules/terra…

bfa774a

…form-ibm-cloud-monitoring into 12594-cm

iamar7 changed the title ~~[WIP] migrate cloud monitoring from observability-instances~~ Migration of cloud monitoring from observability-instances Mar 24, 2025

Md Anam Raihan added 2 commits March 24, 2025 15:42

remove cloud monitoring provision variable

04834b5

remove cloud monitoring provision

8ba57ff

resolves pc

9d376c2

iamar7 marked this pull request as draft March 25, 2025 06:47

ocofaigh requested changes Mar 25, 2025

View reviewed changes

modules/metrics_routing/README.md Outdated Show resolved Hide resolved

examples/advanced/README.md Outdated Show resolved Hide resolved

Md Anam Raihan added 4 commits March 26, 2025 23:34

resolve comments

e87953a

Revert "resolves pc"

85dd8db

This reverts commit 9d376c2.

revert changes

1eedbca

resolve comments

32cce9e

update readme

ca432fd

ocofaigh reviewed Apr 2, 2025

View reviewed changes

examples/advanced/main.tf Outdated Show resolved Hide resolved

ocofaigh reviewed Apr 2, 2025

View reviewed changes

examples/advanced/main.tf Outdated Show resolved Hide resolved

Md Anam Raihan and others added 5 commits April 4, 2025 04:54

resolve comments

688d7da

resolve pc

19e617a

Merge branch 'main' of https://github.com/terraform-ibm-modules/terra…

1de2db4

…form-ibm-cloud-monitoring into 12594-cm

remove common-dev

1af430b

resolve pc

104efe0

iamar7 marked this pull request as ready for review April 4, 2025 00:17

ocofaigh requested changes Apr 4, 2025

View reviewed changes

Md Anam Raihan and others added 6 commits April 9, 2025 13:56

resolve comments

71c6668

resolve pc

1c89618

update cdev asset

03452c4

remove cdev

803fd13

Merge branch 'main' into 12594-cm

51cf35f

update readme

8e722f1

ocofaigh reviewed Apr 9, 2025

View reviewed changes

ocofaigh approved these changes Apr 9, 2025

View reviewed changes

ocofaigh merged commit 4083979 into main Apr 9, 2025
2 checks passed

ocofaigh deleted the 12594-cm branch April 9, 2025 12:15

terraform-ibm-modules-ops added the released label Apr 9, 2025

Migration of cloud monitoring from observability-instances #3

Migration of cloud monitoring from observability-instances #3

Uh oh!

Conversation

iamar7 commented Mar 14, 2025

Description

Release required?

Release notes content

Run the pipeline

Checklist for reviewers

For mergers

Uh oh!

ocofaigh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iamar7 commented Mar 21, 2025

Uh oh!

iamar7 commented Mar 21, 2025

Uh oh!

iamar7 commented Mar 24, 2025

Uh oh!

iamar7 commented Mar 24, 2025

Uh oh!

iamar7 commented Mar 24, 2025

Uh oh!

iamar7 commented Mar 24, 2025

Uh oh!

ocofaigh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

iamar7 commented Mar 26, 2025

Uh oh!

iamar7 commented Mar 26, 2025

Uh oh!

Uh oh!

Uh oh!

iamar7 commented Apr 3, 2025

Uh oh!

ocofaigh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamar7 commented Apr 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

terraform-ibm-modules-ops commented Apr 9, 2025

Uh oh!

Reviewers

ocofaigh left a comment •

edited

Loading