New data source: `azurerm_dynatrace_monitor` #28381

jiaweitao001 · 2024-12-25T07:25:35Z

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

If this PR changes meaningfully during the course of review please update the title and description as required.

jackofallops

Hi @jiaweitao001 - Thanks for the PR. Some comments to take a look at below if you could please?

jackofallops · 2025-01-13T12:14:58Z

internal/services/dynatrace/dynatrace_monitors_data_source.go

+		"environment_properties": {
+			Type:     pluginsdk.TypeList,
+			Computed: true,
+			Elem: &pluginsdk.Resource{
+				Schema: map[string]*pluginsdk.Schema{
+					"environment_info": {
+						Type:     pluginsdk.TypeList,
+						Computed: true,
+						Elem: &pluginsdk.Resource{
+							Schema: map[string]*pluginsdk.Schema{
+								"environment_id": {
+									Type:     pluginsdk.TypeString,
+									Computed: true,
+								},
+							},
+						},
+					},
+				},
+			},
+		},


Is there a reason we're doing 2 levels of nesting here for a single value? Can this just be flattened to the top level?

Also, this property doesn't appear to be supported by the related resource, should it even be included here?

We'd better not flatten this because according to the service team, there are a bunch of other attributes under environment_properties and environment_info, but they do not want them on TF right now. We will have to onboard them once they request in the future.

This property is not supported by the related resource is because it's an optional + computed attribute and its value comes from the API response only, that's why we will have to onboard the data source first. Then I'll add it to the related resource.

jackofallops · 2025-01-13T12:17:09Z

internal/services/dynatrace/dynatrace_monitors_data_source.go

+					Identity:                      identityProps,
+					EnvironmentProperties:         FlattenDynatraceEnvironmentProperties(props.DynatraceEnvironmentProperties),
+					PlanData:                      FlattenDynatracePlanData(props.PlanData),
+					UserInfo:                      FlattenDynatraceUserInfo(metadata.ResourceData.Get("user").([]interface{})),


This is computed only, so we cannot "Get" it here, if this is not returned by the API, it cannot be included in the Data Source as it will always be empty.

jackofallops · 2025-01-13T12:19:08Z

internal/services/dynatrace/dynatrace_monitors_resource_test.go

+    first_name   = "Alice"
+    last_name    = "Bobab"
+    email        = "[email protected]"
+    phone_number = "123456"
+    country      = "westus"


Why are we now supplying data directly here? This looks like it might actually be a real email (or could be in future?) Is this a mistake from local testing that should be removed?

Right, it was for local testing, will remove them.

jackofallops · 2025-01-13T12:19:40Z

website/docs/d/dynatrace_monitors.html.markdown

+
+---
+
+A `user` block supports the following:


and here

Suggested change

A `user` block supports the following:

A `user` block exports the following:

jackofallops · 2025-01-13T12:19:47Z

website/docs/d/dynatrace_monitors.html.markdown

+
+---
+
+A `plan` block supports the following:


same here

Suggested change

A `plan` block supports the following:

A `plan` block exports the following:

jackofallops · 2025-01-13T12:20:33Z

website/docs/d/dynatrace_monitors.html.markdown

+
+---
+
+A `identity` block supports the following:


Since this block is computed only, it's exports rather than supports.

Suggested change

A `identity` block supports the following:

An `identity` block exports the following:

jackofallops · 2025-01-13T12:21:05Z

website/docs/d/dynatrace_monitors.html.markdown

+
+A `identity` block supports the following:
+
+* `type` - The type of identity used for the resource. Only possible value is `SystemAssigned`.


Since this is computed only, we don't specify possible values.

Suggested change

* `type` - The type of identity used for the resource. Only possible value is `SystemAssigned`.

* `type` - The type of identity used for the resource.

New data source: azurerm_dynatrace_monitor

30066ee

jiaweitao001 requested a review from a team as a code owner December 25, 2024 07:25

github-actions bot added documentation service/dynatrace size/XL waiting-response labels Dec 25, 2024

Remove deprecated method

36e729f

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

rcskosir added the new-data-source label Jan 8, 2025

jackofallops requested changes Jan 13, 2025

View reviewed changes

github-actions bot assigned jackofallops Jan 13, 2025

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

address comments

2e1095c

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New data source: `azurerm_dynatrace_monitor` #28381

New data source: `azurerm_dynatrace_monitor` #28381

jiaweitao001 commented Dec 25, 2024

jackofallops left a comment

jackofallops Jan 13, 2025

jiaweitao001 Jan 14, 2025

jackofallops Jan 13, 2025

jiaweitao001 Jan 14, 2025

jackofallops Jan 13, 2025

jiaweitao001 Jan 14, 2025

jackofallops Jan 13, 2025

jiaweitao001 Jan 14, 2025

jackofallops Jan 13, 2025

jiaweitao001 Jan 14, 2025

jackofallops Jan 13, 2025

jiaweitao001 Jan 14, 2025

jackofallops Jan 13, 2025

jiaweitao001 Jan 14, 2025

	A `user` block supports the following:
	A `user` block exports the following:

	A `plan` block supports the following:
	A `plan` block exports the following:

	A `identity` block supports the following:
	An `identity` block exports the following:


		A `identity` block supports the following:

		* `type` - The type of identity used for the resource. Only possible value is `SystemAssigned`.

New data source: azurerm_dynatrace_monitor #28381

Are you sure you want to change the base?

New data source: azurerm_dynatrace_monitor #28381

Conversation

jiaweitao001 commented Dec 25, 2024

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

jackofallops 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

New data source: `azurerm_dynatrace_monitor` #28381

New data source: `azurerm_dynatrace_monitor` #28381