Skip to content

Conversation

kevinpombo-datadog
Copy link

@kevinpombo-datadog kevinpombo-datadog commented Oct 9, 2025

Updated monitors terraform to add support for setting draft status as an optional value with it defaulting to published when not set - https://datadoghq.atlassian.net/browse/MA-6686

@kevinpombo-datadog kevinpombo-datadog requested review from a team as code owners October 9, 2025 18:47
@kevinpombo-datadog kevinpombo-datadog requested a review from a team as a code owner October 9, 2025 19:12
@kevinpombo-datadog kevinpombo-datadog changed the title MA-6686 Update Monitors terraform to support draft monitors [MA-6686][feature] Update Monitors terraform to support draft monitors Oct 9, 2025
Copy link

@domalessi domalessi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion but approved!

@kevinpombo-datadog kevinpombo-datadog changed the title [MA-6686][feature] Update Monitors terraform to support draft monitors [MA-6686] Update Monitors terraform to support draft monitors Oct 9, 2025
Co-authored-by: domalessi <111786334+domalessi@users.noreply.github.com>
@kevinpombo-datadog kevinpombo-datadog changed the title [MA-6686] Update Monitors terraform to support draft monitors [feature] Update Monitors terraform to support draft monitors Oct 9, 2025
@nickpaw
Copy link

nickpaw commented Oct 10, 2025

Are we missing the read in updateMonitorState?

m.SetName(d.Get("name").(string))
m.SetMessage(d.Get("message").(string))
m.SetOptions(o)
m.SetDraftStatus(d.Get("draft_status").(datadogV1.MonitorDraftStatus))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the assertion correct? Following the monitor type enum, do we need to assert as a string and convert the same way?
m.SetDraftStatus(datadogV1.MonitorDraftStatus(d.Get("draft_status").(string)))

Also present below ln 826

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case SetDraftStatus is expecting an instance of datadogV1.MonitorDraftStatus

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, right, but does d.Get("draft_status") return an instance of datadogV1.MonitorDraftStatus or a string? Following the other enum example it looked like it returns a string and needed to be converted?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d.Get("draft_status") will return a string that we have to convert to the enum, this should be updated now

@kevinpombo-datadog
Copy link
Author

@nickpaw Updated updateMonitorState for both providers now leaving it as an optional field

ValidateDiagFunc: validators.ValidateEnumValue(datadogV1.NewMonitorOptionsNotificationPresetsFromValue),
},
"draft_status": {
Description: "Indicates whether the monitor is in a draft or published state. When set to `draft`, the monitor appears as Draft and does not send notifications. When set to `published`, the monitor is active, and it evaluates conditions and sends notifications as configured.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we note that the default value if not configured is published?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: true,
},
"draft_status": schema.StringAttribute{
Description: "The type of the monitor. The mapping from these types to the types found in the Datadog Web UI can be found in the Datadog API [documentation page](https://docs.datadoghq.com/api/v1/monitors/#create-a-monitor). Note: The monitor type cannot be changed after a monitor is created.",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description here looks like it was for monitor.type?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants