fix: Fix parsing for dimension names from CloudWatch tag identifier values #33040

cjr125 · 2022-09-09T16:01:31Z

Bug fix

What does this PR do?

Fix parsing for dimension names from CloudWatch tag identifier values

Why is it important?

This change is necessary for filtering CloudWatch metric tags with dimensions attached to identifier values

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

Approach to remove dimension names from identifier values is correct

mergify · 2022-09-09T16:01:57Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @cjr125? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2022-09-09T16:08:57Z

❕ Build Aborted

The PR is not allowed to run in the CI yet

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-10-11T22:16:46.801+0000
Duration: 5 min 36 sec

Steps errors

Expand to view the steps failures

`Load a resource file from a library`

Took 0 min 0 sec . View more details here
Description: approval-list/elastic/beats.yml

`Error signal`

Took 0 min 0 sec . View more details here
Description: githubApiCall: The REST API call https://api.github.com/orgs/elastic/members/cjr125 return the message : java.lang.Exception: httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/cjr125 : httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/cjr125 : Code: 404Error: {"message":"User does not exist or is not a member of the organization","documentation_url":"https://docs.github.com/rest/reference/orgs#check-organization-membership-for-a-user"}

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

mergify · 2022-09-20T21:24:44Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b review/addCloudFront upstream/review/addCloudFront
git merge upstream/main
git push upstream review/addCloudFront

aspacca · 2022-09-21T01:00:35Z

@cjr125 there's no need to add a new metricset to metrcibeat

you can collect the metrics you need just adding the following to your modules.d/aws.yml:

- module: aws
  period: 5m
  metricsets:
    - cloudwatch
  metrics:
      - namespace: AWS/CloudFront
        resource_type: cloudfront
        statistic: ["Average"]
        name:
          - 4xxErrorRate
          - 401ErrorRate
          - 403ErrorRate
          - 404ErrorRate
          - 5xxErrorRate
          - 502ErrorRate
          - 503ErrorRate
          - 504ErrorRate
          - CacheHitRate
          - TotalErrorRate
          - ExecutionTime
      - namespace: AWS/CloudFront
        resource_type: cloudfront
        statistic: ["Sum"]
        name:
          - BytesDownloaded
          - BytesUploaded
          - Requests
          - Invocations
          - ValidationErrors
          - ExecutionErrors
          - FunctionThrottles
      - namespace: AWS/CloudFront
        resource_type: cloudfront
        statistic: ["p95"] # or others relevant pNN
        name:
          - OriginLatency

cla-checker-service · 2022-10-06T12:16:43Z

💚 CLA has been signed

cjr125 · 2022-10-06T12:23:25Z

@cjr125 there's no need to add a new metricset to metrcibeat

you can collect the metrics you need just adding the following to your modules.d/aws.yml:

- module: aws
  period: 5m
  metricsets:
    - cloudwatch
  metrics:
      - namespace: AWS/CloudFront
        resource_type: cloudfront
        statistic: ["Average"]
        name:
          - 4xxErrorRate
          - 401ErrorRate
          - 403ErrorRate
          - 404ErrorRate
          - 5xxErrorRate
          - 502ErrorRate
          - 503ErrorRate
          - 504ErrorRate
          - CacheHitRate
          - TotalErrorRate
          - ExecutionTime
      - namespace: AWS/CloudFront
        resource_type: cloudfront
        statistic: ["Sum"]
        name:
          - BytesDownloaded
          - BytesUploaded
          - Requests
          - Invocations
          - ValidationErrors
          - ExecutionErrors
          - FunctionThrottles
      - namespace: AWS/CloudFront
        resource_type: cloudfront
        statistic: ["p95"] # or others relevant pNN
        name:
          - OriginLatency

@aspacca I do actually need part of this code to fix a problem with using tags_filter on CloudFront metrics in CloudWatch. The identifier values include dimension names which break the standard tag filtering logic. please see the updates (I left the commit history)

aspacca · 2022-10-11T01:21:03Z

The identifier values include dimension names which break the standard tag filtering logic. please see the updates (I left the commit history)

can you explain your use case with an example?
From what I see it looks like a breaking change, and if so we cannot merge without proper protocol

mergify · 2022-10-11T03:21:16Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b review/addCloudFront upstream/review/addCloudFront
git merge upstream/main
git push upstream review/addCloudFront

cjr125 · 2022-10-11T13:35:57Z

The identifier values include dimension names which break the standard tag filtering logic. please see the updates (I left the commit history)

can you explain your use case with an example? From what I see it looks like a breaking change, and if so we cannot merge without proper protocol

When loading metrics using the following config, I am able to print the following:

  aws.yml: |-
    - module: aws
      period: 1m
      metricsets:
        - cloudwatch
      metrics:
        - namespace: AWS/CloudFront
          resource_type: cloudfront
          statistic: ["Average"]
          name:
            - 4xxErrorRate
            - 401ErrorRate
            - 403ErrorRate
            - 404ErrorRate
            - 5xxErrorRate
            - 502ErrorRate
            - 503ErrorRate
            - 504ErrorRate
            - CacheHitRate
            - TotalErrorRate
            - ExecutionTime
        - namespace: AWS/CloudFront
          resource_type: cloudfront
          statistic: ["Sum"]
          name:
            - BytesDownloaded
            - BytesUploaded
            - Requests
            - Invocations
            - ValidationErrors
            - ExecutionErrors
            - FunctionThrottles
        - namespace: AWS/CloudFront
          resource_type: cloudfront
          statistic: ["p95"] # or others relevant pNN
          name:
            - OriginLatency
      tags_filter:
        - key: "amwell:environment-name"
          value: "dev-next"

// remove dimension names from identifier value if applicable
if strings.Contains(identifierValue, dimensionSeparator) {
	fmt.Println("identifierValue:", identifierValue)
	identifierSeparators := strings.Split(identifierValue, dimensionSeparator)
	identifierValue = identifierSeparators[len(identifierSeparators)-1]
}

identifierValue: Global,E5TGO8HHLKEA8

This reverts commit 9f4b35cae8443d60ce3252b55f83d90701534da9.

…alues

aspacca · 2022-10-12T00:13:48Z

identifierValue: Global,E5TGO8HHLKEA8

you should use this full value in the tags_filter

with the other PR you've created you can add multiple values for the same tag filter

cjr125 · 2022-10-12T07:25:53Z

identifierValue: Global,E5TGO8HHLKEA8

you should use this full value in the tags_filter

with the other PR you've created you can add multiple values for the same tag filter

I'm not sure I follow what you are suggesting. These identifier values are what come back when I try to apply the filter for a specific application environment which we need to monitor. I need the filter to select all CloudFront distributions tagged with this environment name. For other namespaces, the identifier values do not contain dimension names (Global in this case) and the filtering works properly. In the case of the CloudFront namespace, however, the dimension name must be trimmed off in order to capture the desired identifier value. Is there some other way this problem can be approached?

mergify · 2022-10-25T09:56:15Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b review/addCloudFront upstream/review/addCloudFront
git merge upstream/main
git push upstream review/addCloudFront

kaiyan-sheng · 2022-10-28T21:12:55Z

@cjr125 Im trying to fix this issue in #33472, please feel free to test/review it.

cjr125 requested a review from a team as a code owner September 9, 2022 16:01

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 9, 2022

mergify bot assigned cjr125 Sep 9, 2022

cjr125 force-pushed the review/addCloudFront branch 2 times, most recently from f6bca84 to dbb8ce4 Compare September 9, 2022 16:07

cjr125 force-pushed the review/addCloudFront branch 5 times, most recently from 8960f81 to 2c546ef Compare September 13, 2022 11:56

cjr125 force-pushed the review/addCloudFront branch from 2c546ef to 03218ec Compare September 16, 2022 23:29

endorama added the Team:Cloud-Monitoring Label for the Cloud Monitoring team label Sep 19, 2022

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 19, 2022

cjr125 force-pushed the review/addCloudFront branch from 88cc90a to b574921 Compare September 20, 2022 21:53

cjr125 force-pushed the review/addCloudFront branch from c0b21d1 to 0ff090b Compare October 6, 2022 12:16

cjr125 force-pushed the review/addCloudFront branch from 0ff090b to 510295b Compare October 6, 2022 12:19

cjr125 changed the title ~~feat: Add AWS CloudFront Module to Metricbeat~~ fix: Fix parsing for dimension names from CloudWatch tag identifier values Oct 6, 2022

cjr125 force-pushed the review/addCloudFront branch from 5e410a9 to 342bc81 Compare October 11, 2022 22:14

cjr125 added 3 commits October 11, 2022 18:15

feat: Add AWS CloudFront Module to Metricbeat

d6e7df4

Revert "feat: Add AWS CloudFront Module to Metricbeat"

c2d167c

This reverts commit 9f4b35cae8443d60ce3252b55f83d90701534da9.

fix: Fix parsing for dimension names from CloudWatch tag identifier v…

af47c3d

…alues

cjr125 force-pushed the review/addCloudFront branch from 342bc81 to af47c3d Compare October 11, 2022 22:16

cjr125 mentioned this pull request Oct 28, 2022

[AWS] Add tags to events based on parsed identifier #33472

Merged

6 tasks

cjr125 closed this Oct 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fix parsing for dimension names from CloudWatch tag identifier values #33040

fix: Fix parsing for dimension names from CloudWatch tag identifier values #33040

cjr125 commented Sep 9, 2022 •

edited

Loading

mergify bot commented Sep 9, 2022

elasticmachine commented Sep 9, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

`Load a resource file from a library`

`Error signal`

mergify bot commented Sep 20, 2022

aspacca commented Sep 21, 2022

cla-checker-service bot commented Oct 6, 2022 •

edited

Loading

cjr125 commented Oct 6, 2022

aspacca commented Oct 11, 2022

mergify bot commented Oct 11, 2022

cjr125 commented Oct 11, 2022 •

edited

Loading

aspacca commented Oct 12, 2022

cjr125 commented Oct 12, 2022 •

edited

Loading

mergify bot commented Oct 25, 2022

kaiyan-sheng commented Oct 28, 2022

fix: Fix parsing for dimension names from CloudWatch tag identifier values #33040

fix: Fix parsing for dimension names from CloudWatch tag identifier values #33040

Conversation

cjr125 commented Sep 9, 2022 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

mergify bot commented Sep 9, 2022

elasticmachine commented Sep 9, 2022 • edited by jenkins-beats-ci bot Loading

❕ Build Aborted

Build stats

Steps errors

Load a resource file from a library

Error signal

🤖 GitHub comments

mergify bot commented Sep 20, 2022

aspacca commented Sep 21, 2022

cla-checker-service bot commented Oct 6, 2022 • edited Loading

cjr125 commented Oct 6, 2022

aspacca commented Oct 11, 2022

mergify bot commented Oct 11, 2022

cjr125 commented Oct 11, 2022 • edited Loading

aspacca commented Oct 12, 2022

cjr125 commented Oct 12, 2022 • edited Loading

mergify bot commented Oct 25, 2022

kaiyan-sheng commented Oct 28, 2022

cjr125 commented Sep 9, 2022 •

edited

Loading

elasticmachine commented Sep 9, 2022 •

edited by jenkins-beats-ci bot

Loading

`Load a resource file from a library`

`Error signal`

cla-checker-service bot commented Oct 6, 2022 •

edited

Loading

cjr125 commented Oct 11, 2022 •

edited

Loading

cjr125 commented Oct 12, 2022 •

edited

Loading