beats
beats copied to clipboard
fix: Fix parsing for dimension names from CloudWatch tag identifier values
- 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
- [x] My code follows the style guidelines of this project
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] I have made corresponding change to the default configuration files
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] I have added an entry in
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.
Author's Checklist
- [ ] Approach to remove dimension names from identifier values is correct
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.0is the label to automatically backport to the8./dbranch./dis the digit
:grey_exclamation: 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"}
:robot: 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. -
runelasticsearch-ci/docs: Re-trigger the docs validation. (use unformatted text in the comment!)
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 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 has been signed
@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)
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
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
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
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
identifierValue: Global,E5TGO8HHLKEA8
you should use this full value in the
tags_filterwith 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?
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 Im trying to fix this issue in https://github.com/elastic/beats/pull/33472, please feel free to test/review it.