fix: Tag filters only allow single values per key #32775

cjr125 · 2022-08-23T12:33:42Z

What does this PR do?

When filtering tags from the AWS Metricbeat module, it is not possible to specify more than one value associated to a particular key. This PR allows for that.

Why is it important?

A practical example of this would be filtering for more than one development environment (among others) on a single instance of Metricbeat.

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.

How to test this PR locally

Try loading the following AWS module config into Metricbeat:

aws.yml: |-
    - module: aws
      period: 1m
      regions: us-east-2
      metricsets:
        - cloudwatch
      metrics:
        - namespace: AWS/ApiGateway
          resource_type: apigateway
        - namespace: AWS/Lambda
          resource_type: lambda
      tags_filter:
        - key: "amwell:environment-name"
          value: "dev-next"
        - key: "amwell:environment-name"
          value: "stg01"

mergify · 2022-08-23T12:34:17Z

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-08-23T12:38:32Z

💚 Build Succeeded

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-09-07T00:54:23.829+0000
Duration: 94 min 35 sec

Test stats 🧪

Test	Results
Failed	0
Passed	4072
Skipped	1019
Total	5091

💚 Flaky test report

Tests succeeded.

🤖 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!)

tommyers-elastic · 2022-08-23T14:12:20Z

thanks for your contribution! this seems like a reasonable enhancement to me. do you have thoughts @ravikesarwani ?

before we can merge we need a few things sorting:

please add appropriate unit tests
please update the documentation

thanks!

cjr125 · 2022-08-23T19:04:28Z

thanks for your contribution! this seems like a reasonable enhancement to me. do you have thoughts @ravikesarwani ?

before we can merge we need a few things sorting:
* please add appropriate unit tests

* please update the documentation
thanks!

The more I think about this, the more i realize that this might be a bigger problem. There is no way to distinguish between AND and OR logic with the filters I think is the real problem. Any advice?

mergify · 2022-08-25T18:06:11Z

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/multiValueTagFilter upstream/review/multiValueTagFilter
git merge upstream/main
git push upstream review/multiValueTagFilter

x-pack/metricbeat/module/aws/aws_test.go

cla-checker-service · 2022-09-02T07:32:33Z

💚 CLA has been signed

CHANGELOG.next.asciidoc

aspacca · 2022-09-06T01:31:23Z

@cjr125 it seems you force pushed a previous commit without the latest changes

aspacca

force push without approved changes

cjr125 · 2022-09-06T01:40:22Z

force push without approved changes

fixed

mergify · 2022-09-06T01:40:30Z

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

CHANGELOG.next.asciidoc

When filtering tags from the AWS Metricbeat module, it is not possible to specify more than one value associated to a particular key. A practical example of this would be filtering for more than one development environment (among others) on a single instance of Metricbeat.

aspacca · 2022-09-07T00:50:35Z

@cjr125 all good on my side

can you please sign the CLA?

* fix: Tag filters only allow single values per key When filtering tags from the AWS Metricbeat module, it is not possible to specify more than one value associated to a particular key. A practical example of this would be filtering for more than one development environment (among others) on a single instance of Metricbeat. * Expand Unit Test Coverage * Updated changelog (cherry picked from commit 3e07726)

* fix: Tag filters only allow single values per key When filtering tags from the AWS Metricbeat module, it is not possible to specify more than one value associated to a particular key. A practical example of this would be filtering for more than one development environment (among others) on a single instance of Metricbeat. * Expand Unit Test Coverage * Updated changelog (cherry picked from commit 3e07726) Co-authored-by: cjr125 <croberts1@gmail.com>

* fix: Tag filters only allow single values per key When filtering tags from the AWS Metricbeat module, it is not possible to specify more than one value associated to a particular key. A practical example of this would be filtering for more than one development environment (among others) on a single instance of Metricbeat. * Expand Unit Test Coverage * Updated changelog

cjr125 requested a review from a team as a code owner August 23, 2022 12:33

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

mergify bot assigned cjr125 Aug 23, 2022

cjr125 force-pushed the review/multiValueTagFilter branch 4 times, most recently from 60e5ac2 to 290114b Compare August 23, 2022 18:49

cjr125 changed the title ~~fix: Tag filters only allow single values per key~~ draft: fix: Tag filters only allow single values per key Aug 24, 2022

cjr125 marked this pull request as draft August 24, 2022 05:09

cjr125 force-pushed the review/multiValueTagFilter branch 2 times, most recently from b26ff63 to e10634c Compare August 25, 2022 12:06

cjr125 force-pushed the review/multiValueTagFilter branch 4 times, most recently from b65e355 to bb829af Compare August 28, 2022 22:32

cjr125 changed the title ~~draft: fix: Tag filters only allow single values per key~~ fix: Tag filters only allow single values per key Aug 28, 2022

cjr125 marked this pull request as ready for review August 28, 2022 22:48

cjr125 requested a review from a team as a code owner August 28, 2022 22:48

cjr125 requested review from fearful-symmetry and faec and removed request for a team August 28, 2022 22:48

cjr125 force-pushed the review/multiValueTagFilter branch 3 times, most recently from f6150c9 to a719f49 Compare August 31, 2022 18:25

cmacknz requested a review from a team August 31, 2022 23:49

cjr125 force-pushed the review/multiValueTagFilter branch from a719f49 to 83dd433 Compare September 1, 2022 02:30

aspacca reviewed Sep 1, 2022

View reviewed changes

x-pack/metricbeat/module/aws/aws_test.go Outdated Show resolved Hide resolved

cjr125 requested a review from a team as a code owner September 2, 2022 07:32

cjr125 force-pushed the review/multiValueTagFilter branch from 097a84d to d31cee4 Compare September 2, 2022 07:41

aspacca approved these changes Sep 6, 2022

View reviewed changes

cjr125 force-pushed the review/multiValueTagFilter branch from d31cee4 to 9b3343a Compare September 6, 2022 01:09

aspacca reviewed Sep 6, 2022

View reviewed changes

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved

aspacca suggested changes Sep 6, 2022

View reviewed changes

cjr125 force-pushed the review/multiValueTagFilter branch from 9b3343a to 9a2f093 Compare September 6, 2022 01:37

cjr125 closed this Sep 6, 2022

cjr125 reopened this Sep 6, 2022

cjr125 force-pushed the review/multiValueTagFilter branch from 9a2f093 to c2239e0 Compare September 6, 2022 13:22

aspacca suggested changes Sep 6, 2022

View reviewed changes

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved

cjr125 added 2 commits September 6, 2022 19:31

Expand Unit Test Coverage

8fa332d

cjr125 force-pushed the review/multiValueTagFilter branch from c2239e0 to 9c83f53 Compare September 6, 2022 23:35

aspacca approved these changes Sep 7, 2022

View reviewed changes

aspacca added the backport-v8.4.0 Automated backport with mergify label Sep 7, 2022

Updated changelog

384da88

cjr125 force-pushed the review/multiValueTagFilter branch from 9c83f53 to 384da88 Compare September 7, 2022 00:54

aspacca merged commit 3e07726 into elastic:main Sep 7, 2022

mergify bot mentioned this pull request Sep 7, 2022

[8.4](backport #32775) fix: Tag filters only allow single values per key #32999

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Tag filters only allow single values per key #32775

fix: Tag filters only allow single values per key #32775

cjr125 commented Aug 23, 2022 •

edited

Loading

mergify bot commented Aug 23, 2022

elasticmachine commented Aug 23, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

tommyers-elastic commented Aug 23, 2022

cjr125 commented Aug 23, 2022

mergify bot commented Aug 25, 2022

cla-checker-service bot commented Sep 2, 2022 •

edited

Loading

aspacca commented Sep 6, 2022

aspacca left a comment

cjr125 commented Sep 6, 2022

mergify bot commented Sep 6, 2022

aspacca commented Sep 7, 2022

fix: Tag filters only allow single values per key #32775

fix: Tag filters only allow single values per key #32775

Conversation

cjr125 commented Aug 23, 2022 • edited Loading

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

mergify bot commented Aug 23, 2022

elasticmachine commented Aug 23, 2022 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

tommyers-elastic commented Aug 23, 2022

cjr125 commented Aug 23, 2022

mergify bot commented Aug 25, 2022

cla-checker-service bot commented Sep 2, 2022 • edited Loading

aspacca commented Sep 6, 2022

aspacca left a comment

Choose a reason for hiding this comment

cjr125 commented Sep 6, 2022

mergify bot commented Sep 6, 2022

aspacca commented Sep 7, 2022

cjr125 commented Aug 23, 2022 •

edited

Loading

elasticmachine commented Aug 23, 2022 •

edited by jenkins-beats-ci bot

Loading

cla-checker-service bot commented Sep 2, 2022 •

edited

Loading