New Resource: `azurerm_dynatrace_tag_rules` #27985

jiaweitao001 · 2024-11-12T08:08:13Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
I have written new tests for my resource or datasource changes & updated any relevent documentation.
I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
(For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

azurerm_resource - support for the thing1 property [GH-00000]

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Related Issue(s)

Fixes #0000

Note

This PR is related to #27432, the same as the previous one, it cannot be tested on TeamCity. Here is the test result from my end with service team's subscription:

=== RUN   TestAccDynatraceTagRules_basic
=== PAUSE TestAccDynatraceTagRules_basic
=== RUN   TestAccDynatraceTagRules_requiresImport
=== PAUSE TestAccDynatraceTagRules_requiresImport
=== CONT  TestAccDynatraceTagRules_basic
=== CONT  TestAccDynatraceTagRules_requiresImport
--- PASS: TestAccDynatraceTagRules_requiresImport (287.09s)
--- PASS: TestAccDynatraceTagRules_basic (288.82s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/dynatrace     288.856s

divyansh3131 · 2024-11-26T07:46:33Z

@katbyte Do we have any ETA by when can we expect this PR to be reviewed? It's already couple of weeks.

This PR is a follow-up for a dependent child RT related to the previously released monitors resource. It is crucial for our functionality, and without it, our service will have a broken experience. Could you please help prioritize it

catriona-m

Hi @jiaweitao001 I've had a look through this and left some comments inline, I can take another look once those are addressed. Thanks!

catriona-m · 2024-11-26T10:12:39Z

internal/services/arckubernetes/log

should this be removed?

Sure. Will do.

catriona-m · 2024-11-26T10:16:11Z

internal/services/dynatrace/dynatrace_tag_rules_resource.go

+			MaxItems: 1,
+			Elem: &pluginsdk.Resource{
+				Schema: map[string]*schema.Schema{
+					"send_aad_logs": {


Suggested change

"send_aad_logs": {

"send_azure_active_directory_logs": {

Will change.

catriona-m · 2024-11-26T10:16:48Z

internal/services/dynatrace/dynatrace_tag_rules_resource.go

+						Type:     pluginsdk.TypeString,
+						Optional: true,
+						ValidateFunc: validation.StringInSlice([]string{
+							"Enabled",
+							"Disabled",


should we consider making this a bool?

Will change.

catriona-m · 2024-11-26T10:17:01Z

internal/services/dynatrace/dynatrace_tag_rules_resource.go

+						}, false),
+					},
+
+					"send_activity_logs": {


should we consider making this a bool?

Will change.

catriona-m · 2024-11-26T10:17:08Z

internal/services/dynatrace/dynatrace_tag_rules_resource.go

+						}, false),
+					},
+
+					"send_subscription_logs": {


should we consider making this a bool?

catriona-m · 2024-11-26T10:30:07Z

internal/services/dynatrace/helper.go

+	if input.SendAadLogs != nil {
+		sendAadLogs = string(*input.SendAadLogs)
+	}


Suggested change

if input.SendAadLogs != nil {

sendAadLogs = string(*input.SendAadLogs)

}

sendAadLogs = pointer.From(input.SendAadLogs)

catriona-m · 2024-11-26T10:31:11Z

internal/services/dynatrace/helper.go

+	if input.SendSubscriptionLogs != nil {
+		sendSubscriptionLogs = string(*input.SendSubscriptionLogs)
+	}


Suggested change

if input.SendSubscriptionLogs != nil {

sendSubscriptionLogs = string(*input.SendSubscriptionLogs)

}

sendSubscriptionLogs = pointer.From(input.SendSubscriptionLogs)

catriona-m · 2024-11-26T10:39:10Z

internal/services/dynatrace/helper.go

+	var name string
+	var value string
+	var action string
+	tags := *input
+	v := tags[0]
+
+	if v.Name != nil {
+		name = *v.Name
+	}
+
+	if v.Value != nil {
+		value = *v.Value
+	}
+
+	if v.Action != nil {
+		action = string(*v.Action)
+	}
+
+	return []FilteringTag{
+		{
+			Name:   name,
+			Value:  value,
+			Action: action,
+		},
+	}


Suggested change

var name string

var value string

var action string

tags := *input

v := tags[0]

if v.Name != nil {

name = *v.Name

}

if v.Value != nil {

value = *v.Value

}

if v.Action != nil {

action = string(*v.Action)

}

return []FilteringTag{

{

Name: name,

Value: value,

Action: action,

},

}

tags := pointer.From(input)[0]

return []FilteringTag{

{

Name: pointer.From(tags.Name),

Value: pointer.From(tags.Value),

Action: string(pointer.From(tags.Action)),

},

}

catriona-m · 2024-11-26T10:40:09Z

internal/services/dynatrace/helper.go

+		return []MetricRule{}
+	}
+
+	var filteringTags []FilteringTag


Suggested change

var filteringTags []FilteringTag

filteringTags := make([]FilteringTag, 0)

Will change.

catriona-m · 2024-11-26T10:43:15Z

website/docs/r/dynatrace_tag_rules.html.markdown

+* `create` - (Defaults to 1 hour) Used when creating the Dynatrace tag rules.
+* `read` - (Defaults to 5 minutes) Used when retrieving the Dynatrace tag rules.
+* `update` - (Defaults to 1 hour) Used when updating the Dynatrace tag rules.
+* `delete` - (Defaults to 1 hour) Used when deleting the Dynatrace tag rules.


Suggested change

* `create` - (Defaults to 1 hour) Used when creating the Dynatrace tag rules.

* `read` - (Defaults to 5 minutes) Used when retrieving the Dynatrace tag rules.

* `update` - (Defaults to 1 hour) Used when updating the Dynatrace tag rules.

* `delete` - (Defaults to 1 hour) Used when deleting the Dynatrace tag rules.

* `create` - (Defaults to 30 minutes) Used when creating the Dynatrace tag rules.

* `read` - (Defaults to 5 minutes) Used when retrieving the Dynatrace tag rules.

* `delete` - (Defaults to 30 minutes) Used when deleting the Dynatrace tag rules.

catriona-m

Thanks for updating this @jiaweitao001. I ran the tests on this and it looks like there are some failures. Could you take a look at fixing these up and then we can have another look at this? Thanks!

------- Stdout: -------
=== RUN   TestAccDynatraceTagRules_basic
=== PAUSE TestAccDynatraceTagRules_basic
=== CONT  TestAccDynatraceTagRules_basic
    testcase.go:173: Step 1/3 error: Error running pre-apply plan: exit status 1
        Error: expected "user.0.first_name" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 50, in resource "azurerm_dynatrace_monitor" "test":
          50:     first_name   = ""
        Error: expected "user.0.last_name" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 51, in resource "azurerm_dynatrace_monitor" "test":
          51:     last_name    = ""
        Error: test: user.0.email, "" is not an valida email address
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 52, in resource "azurerm_dynatrace_monitor" "test":
          52:     email        = ""
        Error: expected "user.0.phone_number" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 53, in resource "azurerm_dynatrace_monitor" "test":
          53:     phone_number = ""
        Error: expected "user.0.country" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 54, in resource "azurerm_dynatrace_monitor" "test":
          54:     country      = ""
--- FAIL: TestAccDynatraceTagRules_basic (4.70s)
FAIL

jiaweitao001 · 2024-12-05T08:58:15Z

Thanks for updating this @jiaweitao001. I ran the tests on this and it looks like there are some failures. Could you take a look at fixing these up and then we can have another look at this? Thanks!

------- Stdout: -------
=== RUN   TestAccDynatraceTagRules_basic
=== PAUSE TestAccDynatraceTagRules_basic
=== CONT  TestAccDynatraceTagRules_basic
    testcase.go:173: Step 1/3 error: Error running pre-apply plan: exit status 1
        Error: expected "user.0.first_name" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 50, in resource "azurerm_dynatrace_monitor" "test":
          50:     first_name   = ""
        Error: expected "user.0.last_name" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 51, in resource "azurerm_dynatrace_monitor" "test":
          51:     last_name    = ""
        Error: test: user.0.email, "" is not an valida email address
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 52, in resource "azurerm_dynatrace_monitor" "test":
          52:     email        = ""
        Error: expected "user.0.phone_number" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 53, in resource "azurerm_dynatrace_monitor" "test":
          53:     phone_number = ""
        Error: expected "user.0.country" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 54, in resource "azurerm_dynatrace_monitor" "test":
          54:     country      = ""
--- FAIL: TestAccDynatraceTagRules_basic (4.70s)
FAIL

Hi @catriona-m , these acc tests should not be triggered automatically, I've add some logics to prevent it from happening. As mentioned in the description, because of some cost issue, I'll have to manually run the tests on the subs provided by the service team and paste the results here.

catriona-m · 2024-12-11T16:32:56Z

Thanks for updating this @jiaweitao001. I ran the tests on this and it looks like there are some failures. Could you take a look at fixing these up and then we can have another look at this? Thanks!

------- Stdout: -------
=== RUN   TestAccDynatraceTagRules_basic
=== PAUSE TestAccDynatraceTagRules_basic
=== CONT  TestAccDynatraceTagRules_basic
    testcase.go:173: Step 1/3 error: Error running pre-apply plan: exit status 1
        Error: expected "user.0.first_name" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 50, in resource "azurerm_dynatrace_monitor" "test":
          50:     first_name   = ""
        Error: expected "user.0.last_name" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 51, in resource "azurerm_dynatrace_monitor" "test":
          51:     last_name    = ""
        Error: test: user.0.email, "" is not an valida email address
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 52, in resource "azurerm_dynatrace_monitor" "test":
          52:     email        = ""
        Error: expected "user.0.phone_number" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 53, in resource "azurerm_dynatrace_monitor" "test":
          53:     phone_number = ""
        Error: expected "user.0.country" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 54, in resource "azurerm_dynatrace_monitor" "test":
          54:     country      = ""
--- FAIL: TestAccDynatraceTagRules_basic (4.70s)
FAIL

Hi @catriona-m , these acc tests should not be triggered automatically, I've add some logics to prevent it from happening. As mentioned in the description, because of some cost issue, I'll have to manually run the tests on the subs provided by the service team and paste the results here.

Thanks @jiaweitao001 - I'll take another look at this once the test results are available.

jiaweitao001 · 2024-12-12T01:57:52Z

Thanks for updating this @jiaweitao001. I ran the tests on this and it looks like there are some failures. Could you take a look at fixing these up and then we can have another look at this? Thanks!

------- Stdout: -------
=== RUN   TestAccDynatraceTagRules_basic
=== PAUSE TestAccDynatraceTagRules_basic
=== CONT  TestAccDynatraceTagRules_basic
    testcase.go:173: Step 1/3 error: Error running pre-apply plan: exit status 1
        Error: expected "user.0.first_name" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 50, in resource "azurerm_dynatrace_monitor" "test":
          50:     first_name   = ""
        Error: expected "user.0.last_name" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 51, in resource "azurerm_dynatrace_monitor" "test":
          51:     last_name    = ""
        Error: test: user.0.email, "" is not an valida email address
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 52, in resource "azurerm_dynatrace_monitor" "test":
          52:     email        = ""
        Error: expected "user.0.phone_number" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 53, in resource "azurerm_dynatrace_monitor" "test":
          53:     phone_number = ""
        Error: expected "user.0.country" to not be an empty string, got
          with azurerm_dynatrace_monitor.test,
          on terraform_plugin_test.tf line 54, in resource "azurerm_dynatrace_monitor" "test":
          54:     country      = ""
--- FAIL: TestAccDynatraceTagRules_basic (4.70s)
FAIL

Hi @catriona-m , these acc tests should not be triggered automatically, I've add some logics to prevent it from happening. As mentioned in the description, because of some cost issue, I'll have to manually run the tests on the subs provided by the service team and paste the results here.

Thanks @jiaweitao001 - I'll take another look at this once the test results are available.

Hi @catriona-m , these tests should not be available on your teamcity pipeline because of cost issue. We have agreed that acc tests related to this resource will be ran under the subs provided by the service team, same as a previous resource here: #27432 . You can find the tests results in the description above.

catriona-m

Thanks @jiaweitao001 - I had another look through a left a couple more comments. I also wanted to note that if it is not possible to update his resource, then all the properties should be marked as ForceNew.

catriona-m · 2024-12-13T11:15:41Z

internal/services/dynatrace/dynatrace_tag_rules_resource.go

+					"send_azure_active_directory_logs_enabled": {
+						Type:     pluginsdk.TypeBool,
+						Optional: true,
+					},
+
+					"send_activity_logs_enabled": {
+						Type:     pluginsdk.TypeBool,
+						Optional: true,
+					},
+
+					"send_subscription_logs_enabled": {
+						Type:     pluginsdk.TypeBool,
+						Optional: true,
+					},


do these have default values?

Yes, the default value is false. I'll update it.

internal/services/dynatrace/dynatrace_tag_rules_resource_test.go

catriona-m

Thanks for updating this @jiaweitao001. I just wanted to note that the comments on the test are still waiting to be addressed.

I also wondered if you could confirm whether or not it is possible to for this resource to be updated? If not, could we mark all the properties asForceNew. If it is possible to update, we would need to add an Update func for this.

Thanks!

jiaweitao001 · 2025-01-09T07:36:00Z

Thanks for updating this @jiaweitao001. I just wanted to note that the comments on the test are still waiting to be addressed.

I also wondered if you could confirm whether or not it is possible to for this resource to be updated? If not, could we mark all the properties asForceNew. If it is possible to update, we would need to add an Update func for this.

Thanks!

Hi @catriona-m , thanks for replying. I'll update the resource names in the acc tests.
For the ForceNew issue, I've marked all the properties on the top level as ForceNew, I'm not sure if it's necessary to mark all the way down to every single attribute?

catriona-m · 2025-01-09T11:10:25Z

Thanks for updating this @jiaweitao001. I just wanted to note that the comments on the test are still waiting to be addressed.
I also wondered if you could confirm whether or not it is possible to for this resource to be updated? If not, could we mark all the properties asForceNew. If it is possible to update, we would need to add an Update func for this.
Thanks!

Hi @catriona-m , thanks for replying. I'll update the resource names in the acc tests. For the ForceNew issue, I've marked all the properties on the top level as ForceNew, I'm not sure if it's necessary to mark all the way down to every single attribute?

I think we should mark them all if it's not possible to update, thanks @jiaweitao001

catriona-m

Thanks for updating @jiaweitao001, once the final few commenst are addressed this should be good! Thanks!

catriona-m · 2025-01-09T11:12:27Z

internal/services/dynatrace/dynatrace_tag_rules_resource_test.go

+%[1]s
+
+resource "azurerm_dynatrace_tag_rules" "test" {
+  name       = "acctestragrules%d"


Suggested change

name = "acctestragrules%d"

name = "acctesttagrules%d"

catriona-m · 2025-01-09T11:12:44Z

internal/services/dynatrace/dynatrace_tag_rules_resource_test.go

+  log_rule {
+    filtering_tag {
+      name   = "Environment"
+      value  = "Prod"
+      action = "Include"
+    }
+  }
+
+  metric_rule {
+    filtering_tag {
+      name   = "Environment"
+      value  = "Prod"
+      action = "Include"
+    }
+  }


since neither of these are not required, we should be able to create the resource in the basic test without them

Will remove.

catriona-m · 2025-01-09T11:12:56Z

internal/services/dynatrace/dynatrace_tag_rules_resource_test.go

+%[1]s
+
+resource "azurerm_dynatrace_tag_rules" "test" {
+  name       = "acctestragrules%d"


Suggested change

name = "acctestragrules%d"

name = "acctesttagrules%d"

catriona-m

Thanks @jiaweitao001 LGTM!

* Update CHANGELOG.md for #28233 * Update for #28215 * Update CHANGELOG.md for #28279 * Update CHANGELOG.md #28269 * Update CHANGELOG.md #27876 * Update CHANGELOG.md #28069 * Update CHANGELOG.md for #28312 * Update CHANGELOG.md for #28278 * Update CHANGELOG.md #28311 * Update CHANGELOG.md undo 28311 * Update CHANGELOG.md #27874 * Update CHANGELOG.md * Update CHANGELOG for #28352 * Update CHANGELOG.md for #28390 * Update CHANGELOG.md for #28398 * Update CHANGELOG.md for #28425 * Update CHANGELOG.md #28427 * Update CHANGELOG.md #28280 * Update CHANGELOG.md for #28319 * Update CHANGELOG.md #24801 * Update for #28360 #28216 #27830 #28404 #27401 #27122 #27931 #28442 * Update for #28379 * Update CHANGELOG.md for #28281 * Update for #28380 * Update for #27375 * Update for #25695 * Update CHANGELOG.md #27985 * Update CHANGELOG.md - update release date manually until can be scripted * Update CHANGELOG.md revert date change as script available * pre-release script updates --------- Co-authored-by: stephybun <[email protected]> Co-authored-by: catriona-m <[email protected]> Co-authored-by: Wyatt Fry <[email protected]> Co-authored-by: sreallymatt <[email protected]> Co-authored-by: Matthew Frahry <[email protected]> Co-authored-by: kt <[email protected]>

jiaweitao001 requested review from katbyte and a team as code owners November 12, 2024 08:08

github-actions bot added documentation dependencies service/service-connector service/dynatrace size/XL waiting-response and removed waiting-response labels Nov 12, 2024

rcskosir added the new-resource label Nov 12, 2024

catriona-m self-assigned this Nov 26, 2024

catriona-m requested changes Nov 26, 2024

View reviewed changes

github-actions bot added waiting-response and removed waiting-response labels Nov 26, 2024

catriona-m requested changes Dec 2, 2024

View reviewed changes

github-actions bot added waiting-response and removed waiting-response labels Dec 2, 2024

catriona-m added the waiting-response label Dec 11, 2024

github-actions bot removed the waiting-response label Dec 12, 2024

catriona-m requested changes Dec 13, 2024

View reviewed changes

github-actions bot added waiting-response and removed waiting-response labels Dec 13, 2024

jiaweitao001 force-pushed the dynatrace branch from 449647a to a66c159 Compare December 20, 2024 06:14

github-actions bot removed the waiting-response label Dec 20, 2024

jiaweitao001 requested a review from catriona-m January 6, 2025 07:02

catriona-m requested changes Jan 8, 2025

View reviewed changes

github-actions bot added the waiting-response label Jan 8, 2025

github-actions bot removed the waiting-response label Jan 9, 2025

catriona-m requested changes Jan 9, 2025

View reviewed changes

github-actions bot added waiting-response and removed waiting-response labels Jan 9, 2025

New resource: azurerm_dynatrace_tag_rules

5ac3b19

jiaweitao001 force-pushed the dynatrace branch from d9d97b4 to 5ac3b19 Compare January 10, 2025 02:41

github-actions bot added the waiting-response label Jan 10, 2025

update vendors

13409ec

github-actions bot added waiting-response and removed waiting-response labels Jan 10, 2025

catriona-m approved these changes Jan 10, 2025

View reviewed changes

catriona-m merged commit 9959dbc into hashicorp:main Jan 10, 2025
33 checks passed

github-actions bot added this to the v4.15.0 milestone Jan 10, 2025

catriona-m added a commit that referenced this pull request Jan 10, 2025

Update CHANGELOG.md #27985

22c63a3

jackofallops pushed a commit that referenced this pull request Jan 10, 2025

Update CHANGELOG.md #27985

89b3aab

	var filteringTags []FilteringTag
	filteringTags := make([]FilteringTag, 0)

New Resource: azurerm_dynatrace_tag_rules #27985

New Resource: azurerm_dynatrace_tag_rules #27985

Conversation

jiaweitao001 commented Nov 12, 2024

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

divyansh3131 commented Nov 26, 2024

catriona-m left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catriona-m left a comment

Choose a reason for hiding this comment

jiaweitao001 commented Dec 5, 2024

catriona-m commented Dec 11, 2024

jiaweitao001 commented Dec 12, 2024

catriona-m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catriona-m left a comment

Choose a reason for hiding this comment

jiaweitao001 commented Jan 9, 2025

catriona-m commented Jan 9, 2025

catriona-m left a 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

catriona-m left a comment

Choose a reason for hiding this comment

New Resource: `azurerm_dynatrace_tag_rules` #27985

New Resource: `azurerm_dynatrace_tag_rules` #27985