fix(aws): Log Error On Skipped Multiplexer #8799

bbernays · 2023-03-07T16:55:34Z

Summary

Currently for the AWS plugin, if you specify a region and a table/resource that is not available in that region the user will have to have logs set to trace in order to get information that a specific region does not support the resource. This PR will:

Have a single log message for all of the regions that are skipped on a per table basis (so a service that has multiple resources will have multiple log messages)
If a resource is going to be fully skipped then it will generate a error log message, but if it has at least a single region then it will just generate a warning log message

erezrokah · 2023-03-07T16:57:50Z

Currently for the AWS plugin, if you specify a region and a table/resource that is not available in that region the user will have to have logs set to trace in order to get information that a specific region does not support the resource. This PR will make change the log level from trace to warn

How does this look like on our playground account? I remember there are quite a few tables that are not supported in all regions so I'm worried this will flood our logs with many warnings

disq

This has the downside of printing warnings whenever a user uses table: ["*"].

I think that's OK

erezrokah · 2023-03-07T18:17:30Z

I got this

With the following config:

kind: source
spec:
  name: aws
  version: v7.3.2
  destinations: [postgresql]
  path: localhost:7777
  registry: grpc
  tables: ["aws_*_last_accessed_details"]

This seems a bit too much no? If you do all iam resources you'd get even more warnings

Maybe we can only log if someone explicitly sets a region?

candiduslynx · 2023-03-07T18:19:29Z

Are we able to check that there's at least 1 match for account-region-service that we will be fetching resources for prior to printing the warn message?

bbernays · 2023-03-07T18:21:17Z

Are we able to check that there's at least 1 match for account-region-service that we will be fetching resources for prior to printing the warn message?

Not that I am aware of...

erezrokah · 2023-03-07T18:43:14Z

plugins/source/aws/client/multiplexers.go

 			// Ensure that the region is always set by a region that has been initialized
 			if region == "" {
-				meta.(*Client).Logger().Trace().Str("accountID", accountID).Str("partition", partition).Msg("no valid regions have been specified for this account")
+				if !client.allRegions {


Can we do if client.allRegions instead of inverted logic (if !client.allRegions)?

Changed the name so its not inverted

plugins/source/aws/client/multiplexers.go

erezrokah · 2023-03-08T09:18:56Z

I think we'd still be getting duplicated log warnings, see:

Example config:

kind: source
spec:
  name: aws
  version: v7.3.2
  destinations: [postgresql]
  path: localhost:7777
  registry: grpc
  tables: ["aws_iam*"]
  spec:
    regions: 
      - eu-central-1

Maybe we should either print the warning once per service, or once per table that uses the service (so users know which table caused the warning). Also can we print what regions are supported and suggest to use those in the configuration? That should allow users to self serve and fix the issue themselves. WDYT?

bbernays · 2023-03-08T15:56:20Z

Here is the updated log message it includes the regions that are supported as well as the table name

erezrokah

Added a few non blocking comments. Thanks for following up @bbernays 🚀

plugins/source/aws/client/helpers.go

plugins/source/aws/client/multiplexers.go

plugins/source/aws/client/client.go

🤖 I have created a release *beep* *boop* --- ## [15.4.0](plugins-source-aws-v15.3.0...plugins-source-aws-v15.4.0) (2023-03-15) ### Features * **aws:** Add AWS EC2 AccountAttributes ([#8937](#8937)) ([90b9b3b](90b9b3b)) * **aws:** Add AWS Organizations Delegated Services ([#8938](#8938)) ([349cf0d](349cf0d)) * **aws:** Add Compute Optimizer Resource ([#8930](#8930)) ([da8cc77](da8cc77)) * **aws:** Add EC2 image launch permissions resource (`aws_ec2_image_launch_permissions`) ([#8689](#8689)) ([025786f](025786f)) * **aws:** Add Support for VPC Service Permissions ([#8808](#8808)) ([05d3342](05d3342)) ### Bug Fixes * **aws-resources-functions:** Save function configuration instead of failing on `AccessDenied` or `AccessDeniedException` errors in `aws_lambda_functions` ([#8870](#8870)) ([ec0b9fd](ec0b9fd)) * **aws:** Fix description for `aws_organizations_delegated_administrators` ([#8935](#8935)) ([2dc6675](2dc6675)) * **aws:** Fixed `aws_ecs_clusters` fetch to include cluster settings ([#9101](#9101)) ([6bc7933](6bc7933)) * **aws:** Log Error On Skipped Multiplexer ([#8799](#8799)) ([ea30c54](ea30c54)) * **deps:** Update module github.com/aws/aws-sdk-go-v2 to v1.17.6 ([#8882](#8882)) ([5fa0031](5fa0031)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/config to v1.18.16 ([#8883](#8883)) ([82ffe4d](82ffe4d)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/feature/ec2/imds to v1.12.24 ([#8885](#8885)) ([674fec4](674fec4)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/feature/s3/manager to v1.11.56 ([#8886](#8886)) ([8a3db4b](8a3db4b)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/internal/ini to v1.3.31 ([#8889](#8889)) ([f8fdb07](f8fdb07)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/internal/v4a to v1.0.22 ([#8890](#8890)) ([3c5b412](3c5b412)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/accessanalyzer to v1.19.6 ([#8891](#8891)) ([b90fb07](b90fb07)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/acm to v1.17.6 ([#8892](#8892)) ([65d5c27](65d5c27)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/amp to v1.16.5 ([#8893](#8893)) ([0adec38](0adec38)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/amplify to v1.13.5 ([#8894](#8894)) ([1f171cc](1f171cc)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/apigateway to v1.16.6 ([#8895](#8895)) ([dd84bc9](dd84bc9)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/apigatewayv2 to v1.13.6 ([#8896](#8896)) ([5699e4b](5699e4b)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/applicationautoscaling to v1.17.6 ([#8897](#8897)) ([1729bed](1729bed)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/cloudhsmv2 to v1.14.5 ([#8898](#8898)) ([f453645](f453645)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/cloudwatch to v1.25.5 ([#8899](#8899)) ([34cbb8d](34cbb8d)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs to v1.20.6 ([#8900](#8900)) ([a19edb4](a19edb4)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/codebuild to v1.20.6 ([#8901](#8901)) ([319ef28](319ef28)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/codepipeline to v1.14.5 ([#8907](#8907)) ([9751c0a](9751c0a)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/cognitoidentity to v1.15.5 ([#8908](#8908)) ([3bc3e3a](3bc3e3a)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/cognitoidentityprovider to v1.22.5 ([#8909](#8909)) ([f5dd4cc](f5dd4cc)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/configservice to v1.29.6 ([#8910](#8910)) ([6475fbc](6475fbc)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/dax to v1.12.5 ([#8911](#8911)) ([f7a6c1d](f7a6c1d)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v1.43.0 ([#8949](#8949)) ([31dfc63](31dfc63)) * Update endpoints ([#8942](#8942)) ([806e490](806e490)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Update multiplexers.go

9fd7f33

bbernays requested review from hermanschaaf and yevgenypats as code owners March 7, 2023 16:55

cq-bot added the aws label Mar 7, 2023

disq approved these changes Mar 7, 2023

View reviewed changes

candiduslynx approved these changes Mar 7, 2023

View reviewed changes

bbernays added 2 commits March 7, 2023 12:22

Update client.go

54a0527

Update multiplexers.go

8dcb3ca

erezrokah reviewed Mar 7, 2023

View reviewed changes

bbernays added 3 commits March 7, 2023 12:51

Update multiplexers.go

08b5800

Update multiplexers.go

a6ad061

Update client.go

5c65d72

candiduslynx approved these changes Mar 7, 2023

View reviewed changes

plugins/source/aws/client/multiplexers.go Outdated Show resolved Hide resolved

plugins/source/aws/client/multiplexers.go Outdated Show resolved Hide resolved

Update multiplexers.go

dac4a0b

bbernays changed the title ~~fix(aws): Warn On Skipped Multiplexer~~ fix(aws): Log Error On Skipped Multiplexer Mar 7, 2023

candiduslynx approved these changes Mar 7, 2023

View reviewed changes

plugins/source/aws/client/multiplexers.go Outdated Show resolved Hide resolved

Update multiplexers.go

91bf5eb

candiduslynx approved these changes Mar 8, 2023

View reviewed changes

bbernays added 3 commits March 8, 2023 09:21

Add table name to multiplexer

5b73b92

Delete vpc_endpoint_service_permissions.go

ef82edb

make log actionable

78e9eb2

erezrokah approved these changes Mar 8, 2023

View reviewed changes

bbernays added 2 commits March 8, 2023 10:44

Pr comments

8bcd800

Merge remote-tracking branch 'upstream/main' into warn-aws-multiplexer

03e4ae7

bbernays added 2 commits March 8, 2023 10:49

Update vpc_endpoint_service_permissions.go

51ba9f2

Update multiplexers.go

a3ff92c

erezrokah approved these changes Mar 8, 2023

View reviewed changes

Update helpers.go

8b03a97

bbernays added the automerge Automatically merge once required checks pass label Mar 8, 2023

Merge branch 'main' into warn-aws-multiplexer

3cc2f39

erezrokah merged commit ea30c54 into cloudquery:main Mar 8, 2023

bbernays deleted the warn-aws-multiplexer branch March 8, 2023 18:36

cq-bot mentioned this pull request Mar 8, 2023

chore(main): Release plugins-source-aws v15.4.0 #8836

Merged

fix(aws): Log Error On Skipped Multiplexer #8799

fix(aws): Log Error On Skipped Multiplexer #8799

Uh oh!

Conversation

bbernays commented Mar 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

erezrokah commented Mar 7, 2023

Uh oh!

disq left a comment

Choose a reason for hiding this comment

Uh oh!

erezrokah commented Mar 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

candiduslynx commented Mar 7, 2023

Uh oh!

bbernays commented Mar 7, 2023

Uh oh!

erezrokah Mar 7, 2023

Choose a reason for hiding this comment

Uh oh!

bbernays Mar 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erezrokah commented Mar 8, 2023

Uh oh!

bbernays commented Mar 8, 2023

Uh oh!

erezrokah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bbernays commented Mar 7, 2023 •

edited

Loading

erezrokah commented Mar 7, 2023 •

edited

Loading