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?

catriona-m · 2024-12-13T11:18:46Z

internal/services/dynatrace/dynatrace_tag_rules_resource_test.go

+	return pointer.To(true), nil
+}
+
+func (r TagRulesResource) basic(data acceptance.TestData) string {


The basic test is typically creating the resource with the minimal required properties, could we change this test to only the required properties and have a separate complete test which includes all the properties?

jiaweitao001 added 12 commits November 12, 2024 14:13

New resources: azurerm_dynatrace_monitors, azurerm_dynatrace_tag_rules

98e2e8f

Adding documentation

d501e48

Add subcategory

6c9301b

Renaming parameters

1c8d590

Fix acc tests

d379b76

Renaming variables

b5eaf49

fix schema

7df2f50

fix imports

76c7545

retrigger teamcity run

ed1eafb

New resource: dynatrace_tag_rules

2c250aa

New resource: azurerm_dynatrace_tag_rules

9db94f8

prevent acc test auto trigger

b761eff

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 labels Nov 12, 2024

fix vendor

1f7ffb1

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

fix acc tests

00c66c4

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

fix go generate

35d2f44

github-actions bot removed the waiting-response label 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 the waiting-response label Nov 26, 2024

jiaweitao001 added 2 commits November 27, 2024 05:46

address comments, change attribute types

9164bee

fix indent

cb2e1f1

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

sort imports

1007af6

github-actions bot removed the waiting-response label Nov 27, 2024

catriona-m requested changes Dec 2, 2024

View reviewed changes

github-actions bot added the waiting-response label Dec 2, 2024

prevent acc tests auto triggering

55584b9

github-actions bot removed the waiting-response label Dec 5, 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 the waiting-response label Dec 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_dynatrace_tag_rules` #27985

New Resource: `azurerm_dynatrace_tag_rules` #27985

jiaweitao001 commented Nov 12, 2024

divyansh3131 commented Nov 26, 2024

catriona-m left a comment

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m Nov 26, 2024

jiaweitao001 Nov 27, 2024

catriona-m left a comment

jiaweitao001 commented Dec 5, 2024

catriona-m commented Dec 11, 2024

jiaweitao001 commented Dec 12, 2024

catriona-m left a comment

catriona-m Dec 13, 2024

catriona-m Dec 13, 2024

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

New Resource: azurerm_dynatrace_tag_rules #27985

Are you sure you want to change the base?

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

New Resource: `azurerm_dynatrace_tag_rules` #27985

New Resource: `azurerm_dynatrace_tag_rules` #27985