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

[AWS Cloudwatch] Fix Tags for APIGW #40755

Merged
merged 52 commits into from
Oct 3, 2024
Merged

[AWS Cloudwatch] Fix Tags for APIGW #40755

merged 52 commits into from
Oct 3, 2024

Conversation

gizas
Copy link
Contributor

@gizas gizas commented Sep 11, 2024

  • Enhancement

Proposed commit message

  • WHAT: Enhance cloudwatch metricset when APIGW is used as a namespace to be able to retrieve the tags configured in AWS
  • WHY: Currently the aws.tags retrieval was not working because we had no way to correlate tags of RestAPis with provided metrics of cloudwatch

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

  1. Install metricbeat with current configuration:
    - module: aws
      period: 10s
      regions:
        - us-east-1
      metricsets:
        - cloudwatch
      metrics:
        - namespace: "AWS/ApiGateway"
          resource_type: "apigateway:restapis"
          tags_filter: 
             - key: "ServiceOwner"
              value: "ole"
     
  1. Create RestAPi in AWS and configure some tags eg. ServiceOwner:ole

Related issues

Use cases

See analysis https://github.com/elastic/sdh-beats/issues/5103#issuecomment-2333853534

Screenshots

Successful usage
Screenshot 2024-09-11 at 5 21 24 PM

Logs

.version":"1.6.0"}}
{"log.level":"info","@timestamp":"2024-09-11T14:22:12.226Z","log.logger":"aws.cloudwatch","log.origin":{"function":"github.com/elastic/beats/v7/x-pack/metricbeat/module/aws/cloudwatch.(*MetricSet).Fetch","file.name":"cloudwatch/cloudwatch.go","file.line":210},"message":"PASSS InfoAPI response: map[PetStore:t8diprvp81 apm-typescript-test-dev:v7o6rbtm6i gizastest2:0yqjzpf5c7 gizastest3:w6lw8qolsl gizastest_private:x4fe7jy7o1 lucian-REST-PetStore:rsad95n9p5 testJavaApi:fooiifd0q6 tommyers-rest:p83fmmae0c]","service.name":"metricbeat","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2024-09-11T14:22:13.347Z","log.logger":"cloudwatch","log.origin":{"function":"github.com/elastic/beats/v7/x-pack/metricbeat/module/aws/cloudwatch.(*MetricSet).createEvents","file.name":"cloudwatch/cloudwatch.go","file.line":546},"message":"In region us-east-1, service /restapis/0yqjzpf5c7 tags match tags_filter","service.name":"metricbeat","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2024-09-11T14:22:13.347Z","log.logger":"cloudwatch","log.origin":{"function":"github.com/elastic/beats/v7/x-pack/metricbeat/module/aws/cloudwatch.(*MetricSet).createEvents","file.name":"cloudwatch/cloudwatch.go","file.line":546},"message":"In region us-east-1, service 0yqjzpf5c7 tags match tags_filter","service.name":"metricbeat","ecs.version":"1.6.0"}

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 11, 2024
@mergify mergify bot assigned gizas Sep 11, 2024
Copy link
Contributor

mergify bot commented Sep 11, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @gizas? 🙏.
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-8./d is the label to automatically backport to the 8./d branch. /d is the digit

Copy link
Contributor

mergify bot commented Sep 11, 2024

backport-8.x has been added to help with the transition to the new branch 8.x.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Sep 11, 2024
@gizas gizas added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label Sep 12, 2024
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 12, 2024
Copy link
Contributor

mergify bot commented Sep 12, 2024

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

@gizas gizas marked this pull request as ready for review September 12, 2024 15:35
@gizas gizas requested review from a team as code owners September 12, 2024 15:35
@elasticmachine
Copy link
Collaborator

Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services)

Copy link
Member

@cmacknz cmacknz left a comment

Choose a reason for hiding this comment

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

Approving go.mod changes.

@gizas gizas enabled auto-merge (squash) October 3, 2024 05:56
@gizas gizas merged commit 5aca19c into main Oct 3, 2024
139 of 142 checks passed
@gizas gizas deleted the awscloudwatchtags branch October 3, 2024 07:45
mergify bot pushed a commit that referenced this pull request Oct 3, 2024
* adding fix for aws tags of cloudwatch

* variables set to apigateway_max_results

* moving check inside apigw namespace

* setting max limit to 500

---------

Co-authored-by: kaiyan-sheng <[email protected]>
(cherry picked from commit 5aca19c)
gizas added a commit that referenced this pull request Oct 3, 2024
* [AWS Cloudwatch] Fix Tags for APIGW (#40755)

* adding fix for aws tags of cloudwatch

* variables set to apigateway_max_results

* moving check inside apigw namespace

* setting max limit to 500

---------

Co-authored-by: kaiyan-sheng <[email protected]>
(cherry picked from commit 5aca19c)

* fixing CHANGELOG.next.asciidoc

---------

Co-authored-by: Andrew Gizas <[email protected]>
@ycombinator
Copy link
Contributor

Since this PR was merged, we're seeing the https://buildkite.com/elastic/beats-packaging-pipeline consistently failing like so when it tries to run mage package on Agentbeat:

# github.com/elastic/beats/v7/x-pack/agentbeat
/usr/local/go/pkg/tool/linux_amd64/link: running aarch64-linux-gnu-gcc failed: exit status 1
/usr/lib/gcc-cross/aarch64-linux-gnu/6/../../../../aarch64-linux-gnu/bin/ld.gold: internal error in maybe_apply_stub, at ../../gold/aarch64.cc:5407
collect2: error: ld returned 1 exit status
Error: running "go build -o build/golang-crossbuild/agentbeat-linux-arm64 -buildmode pie -gcflags=all=-N -l -tags=agentbeat -ldflags -X github.com/elastic/beats/v7/libbeat/version.buildTime=2024-10-03T12:37:52Z -X github.com/elastic/beats/v7/libbeat/version.commit=698951e8c895eff9d03a8d0ececadba3b8b4c6bb" failed with exit code 1

We're still investigating how to fix this forward but, if we can't figure something out soon, we may just need to revert this PR to make CI green again and buy us more time to figure out the problem.

ycombinator added a commit that referenced this pull request Oct 3, 2024
ycombinator added a commit that referenced this pull request Oct 4, 2024
mergify bot pushed a commit that referenced this pull request Oct 4, 2024
@gizas gizas restored the awscloudwatchtags branch October 4, 2024 06:51
pierrehilbert pushed a commit that referenced this pull request Oct 4, 2024
This reverts commit 5aca19c.

(cherry picked from commit c8ab313)

Co-authored-by: Shaunak Kashyap <[email protected]>
@cmacknz
Copy link
Member

cmacknz commented Oct 4, 2024

This PR makes it so that Beats depends on both AWS SDK v1 and AWS SDK v2.

	github.com/aws/aws-sdk-go-v2 v1.30.5
...
	github.com/aws/aws-sdk-go v1.54.19

@cmacknz
Copy link
Member

cmacknz commented Oct 4, 2024

I suspect this is probably the source of the linker error it was introducing, hopefully it goes away if you only depend on aws-sdk-go-v2 (and we should forbid bringing in the v1 SDK with a lint rule to avoid this in the future).

@ycombinator
Copy link
Contributor

I'm able to make the linker error go away if the code doesn't rely on apigatewayv2.Client: #41129. Not suggesting that this PR is the solution, but it may help narrow down the cause of the linker error.

@cmacknz
Copy link
Member

cmacknz commented Oct 4, 2024

https://github.com/aws/aws-sdk-go is essentially deprecated and goes EOL on 7/31/2025, at which point we'll be forced to remove it as it will no longer receive security updates.

If one of them has to go it should be the v1 SDK.

@cmacknz
Copy link
Member

cmacknz commented Oct 16, 2024

We ended up getting the same build error with a GCP dependency bump, so it's not AWS specifically causing this: #41269

Copy link
Contributor

mergify bot commented Oct 23, 2024

⚠️ The sha of the head commit of this PR conflicts with #41388. Mergify cannot evaluate rules on this PR. ⚠️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.x Automated backport to the 8.x branch with mergify Team:obs-ds-hosted-services Label for the Observability Hosted Services team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metricbeat not collecting tags for AWS - Apigateway resource_type:restapis
8 participants