[AWS][Billing] Support Configurable Group By Types by BenB196 · Pull Request #38755 · elastic/beats

BenB196 · 2024-04-07T20:39:48Z

Enhancement

Proposed commit message

WHAT: Implements the ability to configure the AWS Billing modules "Group By" values. Previously these were hardcoded to DIMENSION and TAG. But AWS supports defining any combo of these values.
WHY: Expands the use-cases that the AWS Billing module can support.

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

Given that the PR makes some configuration changes, I've made an attempt at keeping things backwards compatible. Let me know if there is a better way of doing this.
- The default group by types align with the old, DIMENSION & TAG setup.
- The old config values group_by_dimension_keys & group_by_tag_keys have been marked as deprecated, and are only referenced if the new config values group_by_primary_keys & group_by_secondary_keys are unset.
The integration tests for the module are a bit light (what is there did pass), but I wasn't really able to understand how this framework works to expand it.
- I did a majority of the testing locally, and I'll cover the configs I used to validate this PR below in the "How to test this PR locally" section.
The majority of this PR can be described as two things, either:
- Simple find/replace to make naming more "generic".
- Updating the key mapping logic to be more "correct".
- (An additional change made in this PR was to update the supported values of supportedDimensionKeys as they were last touched ~4 years ago and were out-of-date)

How to test this PR locally

Ensure you have access to an AWS account and have the ability to configure an AWS access method that works with Metricbeat and that has all required permissions.
Build Metricbeat from main and run it with a config. This run will be used as the "baseline" for validating this PR.

Base Config used for Testing

metricbeat.modules:
- module: aws
  period: 24h
  metricsets:
    - billing
  cost_explorer_config:
    group_by_dimension_keys:
      - "AZ"
      - "INSTANCE_TYPE"
      - "SERVICE"
    group_by_tag_keys:
      - "aws:createdBy"

Build Metricbeat from this PR and run it with the following configs:

Base Config used for Testing (no changes to data should be observed)

metricbeat.modules:
- module: aws
  period: 24h
  metricsets:
    - billing
  cost_explorer_config:
    group_by_dimension_keys:
      - "AZ"
      - "INSTANCE_TYPE"
      - "SERVICE"
    group_by_tag_keys:
      - "aws:createdBy"

New Config with two dimension keys

metricbeat.modules:
- module: aws
  period: 24h
  metricsets:
    - billing
  cost_explorer_config:
    group_by_dimension_keys:
      - "AZ"
    group_by_primary_type: "DIMENSION"
    group_by_tag_keys:
      - "REGION"
    group_by_secondary_type: "DIMENSION"

New Config with two dimension keys (more keys)

metricbeat.modules:
- module: aws
  period: 24h
  metricsets:
    - billing
  cost_explorer_config:
    group_by_dimension_keys:
      - "AZ"
      - "INSTANCE_TYPE"
      - "USAGE_TYPE"
    group_by_primary_type: "DIMENSION"
    group_by_tag_keys:
      - "REGION"
      - "SERVICE"
    group_by_secondary_type: "DIMENSION"

New Config with two tag keys

metricbeat.modules:
- module: aws
  period: 24h
  metricsets:
    - billing
  cost_explorer_config:
    group_by_dimension_keys:
      - "aws:createdBy"
    group_by_primary_type: "TAG"
    group_by_tag_keys:
      - "Name"
    group_by_secondary_type: "TAG"

New Config using new config values, with two dimensions

metricbeat.modules:
- module: aws
  period: 24h
  metricsets:
    - billing
  cost_explorer_config:
    group_by_primary_keys: #<----
      - "SERVICE"
      - "REGION"
    group_by_primary_type: "DIMENSION"
    group_by_secondary_keys: #<----
      - "LINKED_ACCOUNT"
    group_by_secondary_type: "DIMENSION"

Related issues

Closes [Metricbeat] AWS Billing Module accept pairs of group_by values #34193

Use cases

Screenshots

Logs

mergify · 2024-04-07T20:40:27Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @BenB196? 🙏.
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 · 2024-04-07T20:45:07Z

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

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

Expand to view the summary

Build stats

Duration: 5 min 34 sec

🤖 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 · 2024-04-16T06:25:17Z

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 aws-billing-support-dynamic-group-by-types upstream/aws-billing-support-dynamic-group-by-types
git merge upstream/main
git push upstream aws-billing-support-dynamic-group-by-types

mergify · 2024-09-11T13:10:18Z

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 aws-billing-support-dynamic-group-by-types upstream/aws-billing-support-dynamic-group-by-types
git merge upstream/main
git push upstream aws-billing-support-dynamic-group-by-types

elasticmachine · 2024-09-16T13:11:06Z

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

elasticmachine · 2025-02-19T19:52:35Z

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

mergify · 2025-02-19T19:53:10Z

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 aws-billing-support-dynamic-group-by-types upstream/aws-billing-support-dynamic-group-by-types
git merge upstream/main
git push upstream aws-billing-support-dynamic-group-by-types

flexitrev · 2025-02-19T19:53:35Z

@elastic/obs-ds-hosted-services could you review this PR?

bturquet · 2025-02-21T09:04:20Z

I think @elastic/obs-infraobs-integrations might be involved here as they own the AWS Billing module
cc @lalit-satapathy can someone from your team help with the review ? thanks !

faec

Approving the code for data plane (subject to fixing the CI, which looks like it will just require a make check and a changelog update), but make sure you also get infraobs approval for the user-visible content of the feature.

lalit-satapathy · 2025-02-24T09:22:43Z

I think @elastic/obs-infraobs-integrations might be involved here as they own the AWS Billing module cc @lalit-satapathy can someone from your team help with the review ? thanks !

Adding @gpop63 to review.

gpop63

Let's get the CI to pass, probably need to run make update.

gpop63 · 2025-02-25T10:35:00Z

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

-								_, _ = event.RootFields.Put("aws.linked_account.name", name)
+
+					// index 0 is the key for the primary group_by
+					if index == 0 {


Are we sure these keys are always returned in consistent order? Wasn't the previous check safer?

gpop63 · 2025-02-25T10:52:04Z

x-pack/metricbeat/module/aws/billing/_meta/docs.asciidoc

+* *group_by_primary_type* The type of group for the primary keys, supports either `DIMENSION` or `TAG`. Default `DIMENSION`.
+* *group_by_primary_keys* A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_primary_type*.
+* *group_by_secondary_type* The type of group for the secondary keys, supports either `DIMENSION` or `TAG`. Default `TAG`.
+* *group_by_secondary_keys* A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_secondary_type*.


Let's keep the old settings as well and specify that they are deprecated in favor of the new settings.

Suggested change

* *group_by_primary_type* The type of group for the primary keys, supports either `DIMENSION` or `TAG`. Default `DIMENSION`.

* *group_by_primary_keys* A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_primary_type*.

* *group_by_secondary_type* The type of group for the secondary keys, supports either `DIMENSION` or `TAG`. Default `TAG`.

* *group_by_secondary_keys* A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_secondary_type*.

* *group_by_dimension_keys*: Deprecated, please use `group_by_primary_type` or `group_by_secondary_type`.

* *group_by_tag_keys*: Deprecated, please use `group_by_primary_type` or `group_by_secondary_type`.

* *group_by_primary_type*: The type of group for the primary keys, supports either `DIMENSION` or `TAG`. Default `DIMENSION`.

* *group_by_primary_keys*: A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_primary_type*.

* *group_by_secondary_type*: The type of group for the secondary keys, supports either `DIMENSION` or `TAG`. Default `TAG`.

* *group_by_secondary_keys*: A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_secondary_type*.

gpop63 · 2025-02-25T10:58:12Z

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

+	// handle bwc with old config
+	if len(config.CostExplorerConfig.GroupByPrimaryKeys) == 0 && len(config.CostExplorerConfig.GroupByDimensionKeys) > 0 {
+		config.CostExplorerConfig.GroupByPrimaryKeys = config.CostExplorerConfig.GroupByDimensionKeys
+	}
+	if len(config.CostExplorerConfig.GroupBySecondaryKeys) == 0 && len(config.CostExplorerConfig.GroupByTagKeys) > 0 {
+		config.CostExplorerConfig.GroupBySecondaryKeys = config.CostExplorerConfig.GroupByTagKeys
+	}


Will this fetch the exact same data from AWS as before?

gpop63 · 2025-02-25T11:01:48Z

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

 }

+// validateGroupByType checks if a string value for group_by type is a supported value.
+func validateGroupByType(groupByType costexplorertypes.GroupDefinitionType) (bool, error) {


Can't this function just return an error instead of (bool, error)?

gpop63 · 2025-02-25T11:13:56Z

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

+	if config.CostExplorerConfig.GroupByPrimaryType == "" {
+		config.CostExplorerConfig.GroupByPrimaryType = defaultGroupByPrimaryType
+	}
+	if config.CostExplorerConfig.GroupBySecondaryType == "" {
+		config.CostExplorerConfig.GroupBySecondaryType = defaultGroupBySecondaryType
+	}


Do we want to group the data by default? Shouldn't this be decided by the user?

BenB196 added 16 commits April 7, 2024 10:07

update dimension value validation method to support downstream updates

27dbd87

addition of group by type settings

f7560d6

unit tests for new functions

acfa47f

fix default settings for group by types

86b19ae

fix missed naming changes

321ed8a

fix bugs with metrics fields

7e1f3e7

simplify logic

d0cca7f

fix marshaling issue that caused memory leak

7bb3c2f

update and expand unit test

55bdd94

fix key mapping logic

e058108

better bwc handling

419469c

fix typo

70a91bd

fix typo

e1d5be2

fix linked accounts

776170a

update default config

0d49544

Update CHANGELOG.next.asciidoc

06971ca

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 7, 2024

Update CHANGELOG.next.asciidoc

9c0193e

mergify bot assigned BenB196 Apr 7, 2024

add back file

f63757c

Update docs.asciidoc

352a04b

BenB196 marked this pull request as ready for review April 7, 2024 21:09

BenB196 requested review from a team as code owners April 7, 2024 21:09

BenB196 requested review from belimawr and faec April 7, 2024 21:09

BenB196 added 2 commits April 7, 2024 17:38

fix additional finds

1bd5622

group by -> group_by

6316f8c

fix doc

5c47b8d

BenB196 mentioned this pull request Apr 8, 2024

[AWS] [Billing] Document GetCostAndUsage API limitation elastic/integrations#9290

Merged

4 tasks

Merge branch 'main' into aws-billing-support-dynamic-group-by-types

fbb8fd6

v1v added the backport-8.x Automated backport to the 8.x branch with mergify label Sep 11, 2024

pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Sep 16, 2024

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

BenB196 and others added 2 commits February 15, 2025 10:02

Merge branch 'main' into aws-billing-support-dynamic-group-by-types

d6e5321

update logic

4fecf31

flexitrev added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label Feb 19, 2025

bturquet requested a review from a team February 21, 2025 09:05

faec approved these changes Feb 21, 2025

View reviewed changes

gpop63 reviewed Feb 25, 2025

View reviewed changes

pierrehilbert marked this pull request as draft April 23, 2025 12:04

-* *group_by_primary_type* The type of group for the primary keys, supports either `DIMENSION` or `TAG`. Default `DIMENSION`.
-* *group_by_primary_keys* A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_primary_type*.
-* *group_by_secondary_type* The type of group for the secondary keys, supports either `DIMENSION` or `TAG`. Default `TAG`.
-* *group_by_secondary_keys* A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_secondary_type*.
+* *group_by_dimension_keys*: Deprecated, please use `group_by_primary_type` or `group_by_secondary_type`.
+* *group_by_tag_keys*: Deprecated, please use `group_by_primary_type` or `group_by_secondary_type`.
+* *group_by_primary_type*: The type of group for the primary keys, supports either `DIMENSION` or `TAG`. Default `DIMENSION`.
+* *group_by_primary_keys*: A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_primary_type*.
+* *group_by_secondary_type*: The type of group for the secondary keys, supports either `DIMENSION` or `TAG`. Default `TAG`.
+* *group_by_secondary_keys*: A list of keys used in Cost Explorer to group by. It can either by a list of dimension values or a list of tags, but needs to match the type set by *group_by_secondary_type*.

Conversation

BenB196 commented Apr 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed commit message

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Uh oh!

mergify bot commented Apr 7, 2024

Uh oh!

elasticmachine commented Apr 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❕ Build Aborted

Build stats

🤖 GitHub comments

Uh oh!

mergify bot commented Apr 16, 2024

Uh oh!

mergify bot commented Sep 11, 2024

Uh oh!

elasticmachine commented Sep 16, 2024

Uh oh!

elasticmachine commented Feb 19, 2025

Uh oh!

mergify bot commented Feb 19, 2025

Uh oh!

flexitrev commented Feb 19, 2025

Uh oh!

bturquet commented Feb 21, 2025

Uh oh!

faec left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lalit-satapathy commented Feb 24, 2025

Uh oh!

gpop63 left a comment

Choose a reason for hiding this comment

Uh oh!

gpop63 Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

gpop63 Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

gpop63 Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

gpop63 Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

gpop63 Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

BenB196 commented Apr 7, 2024 •

edited

Loading

elasticmachine commented Apr 7, 2024 •

edited

Loading

faec left a comment •

edited

Loading