r/aws_cloudwatch_metric_stream: New resource #19429

YakDriver · 2021-05-18T19:35:32Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #18870 (supercedes)
Closes #18525

Output from acceptance testing (us-west-2):

--- PASS: TestAccAWSCloudWatchMetricStream_noName (21.20s)
--- PASS: TestAccAWSCloudWatchMetricStream_excludeFilters (21.32s)
--- PASS: TestAccAWSCloudWatchMetricStream_includeFilters (21.39s)
--- PASS: TestAccAWSCloudWatchMetricStream_namePrefix (21.39s)
--- PASS: TestAccAWSCloudWatchMetricStream_tags (21.39s)
--- PASS: TestAccAWSCloudWatchMetricStream_update (84.21s)
--- PASS: TestAccAWSCloudWatchMetricStream_basic (86.79s)
--- PASS: TestAccAWSCloudWatchMetricStream_updateName (157.46s)

Output from acceptance testing (GovCloud):

There does not seem to be support on GovCloud but rather than fail, tests timeout...
=== CONT  TestAccAWSCloudWatchMetricStream_basic
    provider_test.go:1092: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: putting metric_stream failed: RequestCanceled: request context canceled
        caused by: context deadline exceeded
        
          with aws_cloudwatch_metric_stream.test,
          on terraform_plugin_test.tf line 4, in resource "aws_cloudwatch_metric_stream" "test":
           4: resource "aws_cloudwatch_metric_stream" "test" {


--- SKIP: TestAccAWSCloudWatchMetricStream_namePrefix (70.36s)
--- SKIP: TestAccAWSCloudWatchMetricStream_update (70.46s)
--- SKIP: TestAccAWSCloudWatchMetricStream_noName (70.48s)
--- SKIP: TestAccAWSCloudWatchMetricStream_excludeFilters (70.52s)
--- SKIP: TestAccAWSCloudWatchMetricStream_tags (70.56s)
--- SKIP: TestAccAWSCloudWatchMetricStream_includeFilters (70.57s)
--- SKIP: TestAccAWSCloudWatchMetricStream_updateName (119.56s)
--- SKIP: TestAccAWSCloudWatchMetricStream_basic (124.57s)

Co-authored-by: Dirk Avery <[email protected]>

ewbankkit · 2021-05-18T19:59:10Z

aws/resource_aws_cloudwatch_metric_stream.go

+	var name string
+	if v, ok := d.GetOk("name"); ok {
+		name = v.(string)
+	} else if v, ok := d.GetOk("name_prefix"); ok {
+		name = resource.PrefixedUniqueId(v.(string))
+	} else {
+		name = resource.UniqueId()
+	}


Should follow the recommended Resource Name Generation Support pattern.

ewbankkit · 2021-05-18T20:00:12Z

aws/resource_aws_cloudwatch_metric_stream.go

+	log.Printf("[DEBUG] Putting CloudWatch MetricStream: %#v", params)
+	_, err := conn.PutMetricStreamWithContext(ctx, &params)
+	if err != nil {
+		return diag.FromErr(fmt.Errorf("Putting metric_stream failed: %s", err))


Suggested change

return diag.FromErr(fmt.Errorf("Putting metric_stream failed: %s", err))

return diag.FromErr(fmt.Errorf("putting metric_stream failed: %w", err))

ewbankkit · 2021-05-18T20:00:36Z

aws/resource_aws_cloudwatch_metric_stream.go

+	}
+
+	if err != nil {
+		return diag.FromErr(fmt.Errorf("Reading metric_stream failed: %s", err))


Suggested change

return diag.FromErr(fmt.Errorf("Reading metric_stream failed: %s", err))

return diag.FromErr(fmt.Errorf("reading metric_stream failed: %w", err))

YakDriver · 2021-05-18T22:15:16Z

Tags are a little strange for CloudWatch Metric Stream.

cloudwatch.TagResource() does not support tagging a metric stream. According to the docs, "Currently, the only CloudWatch resources that can be tagged are alarms and Contributor Insights rules." Presumably, this means "tagged with TagResource()."
cloudwatch.PutMetricStreamInput{} does have a Tags field.
cloudwatch.GetMetricStreamOutput{} does not have a Tags field.

So, tags can go in, but then you can never, have-I-ever see them again. 🤔

gcacace · 2021-05-18T23:17:21Z

Tags are a little strange for CloudWatch Metric Stream.

cloudwatch.TagResource() does not support tagging a metric stream. According to the docs, "Currently, the only CloudWatch resources that can be tagged are alarms and Contributor Insights rules." Presumably, this means "tagged with TagResource()."

cloudwatch.PutMetricStreamInput{} does have a Tags field.

cloudwatch.GetMetricStreamOutput{} does not have a Tags field.

So, tags can go in, but then you can never, have-I-ever see them again. 🤔

https://docs.aws.amazon.com/sdk-for-go/api/service/cloudwatch/#CloudWatch.ListTagsForResource

This should give you back tags for metric streams (an integration test should demonstrate that). I suspect the API documentation for both TagResource and ListResourceTags need to be updated.

magnetikonline · 2021-05-18T23:24:08Z

Great work team - keen to see this land. 👍

YakDriver · 2021-05-19T00:09:15Z

This should give you back tags for metric streams (an integration test should demonstrate that). I suspect the API documentation for both TagResource and ListResourceTags need to be updated.

~~Unfortunately, it does not.~~ You are correct! I was using the resource ID rather than the ARN to get the tags.

But, running into an issue with GovCloud. It doesn't fail, just times out.

ghost · 2021-05-20T01:40:22Z

This has been released in version 3.42.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

…am resource See: hashicorp/terraform-provider-aws#19429 (which superseded hashicorp/terraform-provider-aws#18870). Which means we can remove all the clunky run-your-own-forked-provider junk. Yay!

github-actions · 2021-06-19T02:13:55Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

gcacace and others added 3 commits May 18, 2021 15:34

Add support for aws_cloudwatch_metric_stream

8ceee9c

Apply suggestions from code review

aeefba4

Co-authored-by: Dirk Avery <[email protected]>

Addressed feedback from PR

4c96b08

YakDriver requested a review from a team as a code owner May 18, 2021 19:35

YakDriver mentioned this pull request May 18, 2021

Add support for aws_cloudwatch_metric_stream #18870

Closed

ewbankkit reviewed May 18, 2021

View reviewed changes

YakDriver added 6 commits May 18, 2021 19:47

internal/r/cloudwatch: Add waiter for metric stream

ea35b56

r/cloudwatch_metric_stream: Minor clean up

6d6357f

test/r/cloudwatch_metric_stream: Test clean up, add attribs

19f34fe

docs/r/cloudwatch_metric_stream: Update tags

2c3c643

tests/r/cloudwatch_metric_stream: Rework ID refresh

844a46d

docs/r/cloudwatch_metric_stream: Fix note

0c5ddf1

YakDriver added 3 commits May 18, 2021 20:31

docs/r/cloudwatch_metric_stream: Remove warning note

5c0e07d

r/cloudwatch_metric_stream: Fix tags

61fffd6

tests/r/cloudwatch_metric_stream: Fix tags fix

064f76e

YakDriver added 2 commits May 18, 2021 20:42

tests/r/cloudwatch_metric_stream: Remove unused struct var

3abd085

r/cloudwatch_metric_stream: Adjust timeouts, GovCloud skip

3c1c2e2

YakDriver added this to the v3.41.0 milestone May 19, 2021

YakDriver merged commit 9e042e9 into main May 19, 2021

YakDriver deleted the f-cloudwatch_metric_stream branch May 19, 2021 01:55

YakDriver modified the milestones: v3.41.0, v3.42.0 May 19, 2021

github-actions bot pushed a commit that referenced this pull request May 19, 2021

Update CHANGELOG.md for #19429

1a0c839

ismith mentioned this pull request May 21, 2021

terraform-provider-aws 3.42.0 supports the aws_cloudwatch_metric_stream resource honeycombio/terraform-aws-honeycomb-cloudwatch-metric-stream#7

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jun 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/aws_cloudwatch_metric_stream: New resource #19429

r/aws_cloudwatch_metric_stream: New resource #19429

YakDriver commented May 18, 2021 •

edited

Loading

ewbankkit May 18, 2021

ewbankkit May 18, 2021

ewbankkit May 18, 2021

YakDriver commented May 18, 2021 •

edited

Loading

gcacace commented May 18, 2021

magnetikonline commented May 18, 2021

YakDriver commented May 19, 2021 •

edited

Loading

ghost commented May 20, 2021

github-actions bot commented Jun 19, 2021

	return diag.FromErr(fmt.Errorf("Putting metric_stream failed: %s", err))
	return diag.FromErr(fmt.Errorf("putting metric_stream failed: %w", err))

	return diag.FromErr(fmt.Errorf("Reading metric_stream failed: %s", err))
	return diag.FromErr(fmt.Errorf("reading metric_stream failed: %w", err))

r/aws_cloudwatch_metric_stream: New resource #19429

r/aws_cloudwatch_metric_stream: New resource #19429

Conversation

YakDriver commented May 18, 2021 • edited Loading

Community Note

ewbankkit May 18, 2021

Choose a reason for hiding this comment

ewbankkit May 18, 2021

Choose a reason for hiding this comment

ewbankkit May 18, 2021

Choose a reason for hiding this comment

YakDriver commented May 18, 2021 • edited Loading

gcacace commented May 18, 2021

magnetikonline commented May 18, 2021

YakDriver commented May 19, 2021 • edited Loading

ghost commented May 20, 2021

github-actions bot commented Jun 19, 2021

YakDriver commented May 18, 2021 •

edited

Loading

YakDriver commented May 18, 2021 •

edited

Loading

YakDriver commented May 19, 2021 •

edited

Loading