feat: Adds support for send_collection_latency_metrics and send_database_metrics for Datadog integrations in mongodbatlas_third_party_integration
#3259
Conversation
Adds support for `sendCollectionLatencyMetrics` and `sendDatabaseMetrics` in the Datadog `third_party_integration` resource and data sources as they are supported by the atlas API Updates documentation and example configurations to reflect these new options.
maastha
changed the title
third_party_integrationsend_collection_latency_metrics and send_database_metrics for Datadog integrations in mongodbatlas_third_party_integration
|
APIx bot: a message has been sent to Docs Slack channel |
|
nit: can you also refer in the description the original PR? #3253 |
examples/mongodbatlas_third_party_integration/datadog/variables.tf
Outdated
Show resolved
Hide resolved
| ), | ||
| }, | ||
| { | ||
| Config: configDatadog(projectID, updatedAPIKey, "US", true, true), |
There was a problem hiding this comment.
nit: don't know if it makes sense to have true, false and false, true in 2 more steps, e.g. to make sure we're not crossing the values
There was a problem hiding this comment.
Potentially see the behavior when you remove the field from the config. Will it go back to the default value of false? Could add a default value in the schema if not
Or alternatively, warn in the docs about using an explicit false instead of removing the field
There was a problem hiding this comment.
Agree with Espen we can clarify behaviour in docs, seeing both attributes are O+C we will likely be in this case
warn in the docs about using an explicit false instead of removing the field
There was a problem hiding this comment.
Or alternatively, warn in the docs about using an explicit false instead of removing the field
Since this is a general behavior of O+C attributes, I think we should have a generic documentation note in all resources (best practices in a way). Similar to how we have a note for advanced_configuration in cluster resources, I think we can have something like below:
**NOTE:** For certain attributes with default values, it's recommended to explicitly set them back to their default instead of removing them from the configuration. For example, if `send_collection_latency_metrics` is set to `true` and you want to revert to the default (`false`), set it to `false` rather than removing `it.`
I don't think we should add this recommendation for each attribute separately.
| "password": integrationSchema.Password, | ||
| "service_discovery": integration.ServiceDiscovery, | ||
| "enabled": integration.Enabled, | ||
| "id": integration.Id, |
There was a problem hiding this comment.
[nit] Feels very verbose this function. Seems like we could do this inline
internal/service/thirdpartyintegration/data_source_third_party_integration.go
Outdated
Show resolved
Hide resolved
| Computed: true, | ||
| Optional: true, |
There was a problem hiding this comment.
Always returned by the API right?
…tabase_metrics` for Datadog integrations in `mongodbatlas_third_party_integration` (mongodb#3259)
|
Hi @maastha, thank you for merging this feature so quickly — I really need this feature! I did notice that I might not have been added as a co-author in 919a5e0, as mentioned in the guidelines?. It would’ve been great to appear as contributor, but no worries. Also, I’d really appreciate it if you could comment when this is released. Thanks! |
Description
Adds support for
sendCollectionLatencyMetricsandsendDatabaseMetricsin the Datadogthird_party_integrationresource and data sources as they are supported by the atlas APIUpdates documentation and example configurations to reflect these new options.
Cherry-picked from #3253
Link to any related issue(s): CLOUDP-276352
Type of change:
Required Checklist:
Further comments