Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[datadog_apm_retention_filter] Support updating default retention filters #2370

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

SalahEddineBC
Copy link
Contributor

@SalahEddineBC SalahEddineBC commented Apr 19, 2024

This PR fixes an issue where users are unable to define default retention filters as resources, this PR updates datadog_apm_retention_filter resource to allow defining those filters in terraform.

this PR depends on https://github.com/DataDog/datadog-api-spec/pull/2757 to be released

@SalahEddineBC SalahEddineBC requested review from a team as code owners April 19, 2024 09:05
@SalahEddineBC SalahEddineBC changed the title Support updating default retention filters [datadog_apm_retention_filter] Support updating default retention filters Apr 19, 2024
@SalahEddineBC SalahEddineBC force-pushed the salah.bachircherif/update-retention-filters-resource branch from c1d4793 to 8c6d657 Compare May 17, 2024 17:49
var id string
var filterName string
for _, rfa := range resp.Data {
if string(rfa.Attributes.GetFilterType()) == state.FilterType.ValueString() {
Copy link
Member

Choose a reason for hiding this comment

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

Why are we matching my filter type here? Can't we have multiple default pipelines with the same type?

func (r *ApmRetentionFilterResource) Create(ctx context.Context, request resource.CreateRequest, response *resource.CreateResponse) {
var state ApmRetentionFilterModel
response.Diagnostics.Append(request.Plan.Get(ctx, &state)...)
if response.Diagnostics.HasError() {
return
}
if state.FilterType.ValueString() != "spans-sampling-processor" {
Copy link
Member

Choose a reason for hiding this comment

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

Can default retention rules not have type spans-sampling-processor ? I see Synthetics Default rule has definition:

                {
                    "id": "",
                    "name": "Synthetics Default",
                    "enabled": false,
                    "meta": {
                        "description": "Get started with Synthetic Monitoring",
                        "tags": [
                            "dd.default:true",
                            "dd.feature:synthetics"
                        ]
                    },
                    "type": "spans-sampling-processor"
                }

@HantingZhang2 HantingZhang2 self-requested a review May 27, 2024 14:24
@SalahEddineBC SalahEddineBC marked this pull request as draft June 25, 2024 08:46
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.

2 participants