Skip to content

fix: count GetMetricData billed metrics per AWS pricing, not per query - #1919

Open
pujitha24 wants to merge 1 commit into
prometheus-community:masterfrom
pujitha24:auto/issue-1806
Open

fix: count GetMetricData billed metrics per AWS pricing, not per query#1919
pujitha24 wants to merge 1 commit into
prometheus-community:masterfrom
pujitha24:auto/issue-1806

Conversation

@pujitha24

Copy link
Copy Markdown

Motivation:
yace_cloudwatch_getmetricdata_metrics_requested_total is documented as
"how AWS bills" for the GetMetricData API, but it was implemented as a
raw count of MetricDataQuery entries in each API call. YACE emits one
MetricDataQuery per requested statistic of a metric, so a metric
configured with 5 statistics incremented this counter by 5, even
though AWS's GetMetricData pricing batches up to 5 statistics of the
same metric (namespace + name + dimensions) into a single billed
metric request. As a result this counter could overstate real
GetMetricData costs by up to 5x whenever more than one statistic was
requested per metric, making it unusable for the cost estimation it
exists for.

Approach:
Added billedGetMetricDataMetricsCount, which groups the CloudwatchData
requested in a GetMetricData call by metric identity (namespace, metric
name, sorted dimensions) and sums ceil(statistics / 5) per group,
matching AWS's documented batching. This only changes the value fed
into the CloudwatchGetMetricDataAPIMetricsCounter metric; it does not
change any CloudWatch API calls, exported metric data, or scrape
behavior.

Validation:

  • go build ./...
  • go test ./pkg/... (all packages pass, including new
    Test_billedGetMetricDataMetricsCount cases covering 1, exactly 5,
    and 6 statistics per metric, and distinct metrics/dimensions)
  • make lint (golangci-lint, 0 issues)
  • make build

This was validated at the unit-test level against AWS's documented
GetMetricData billing rule (up to 5 statistics per metric batched into
one billed metric request); it was not validated against a live AWS
bill, which this repo's CONTRIBUTE.md does not require for a change of
this kind.

Report: #1806
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com

Fixes #1806

Motivation:
yace_cloudwatch_getmetricdata_metrics_requested_total is documented as
"how AWS bills" for the GetMetricData API, but it was implemented as a
raw count of MetricDataQuery entries in each API call. YACE emits one
MetricDataQuery per requested statistic of a metric, so a metric
configured with 5 statistics incremented this counter by 5, even
though AWS's GetMetricData pricing batches up to 5 statistics of the
same metric (namespace + name + dimensions) into a single billed
metric request. As a result this counter could overstate real
GetMetricData costs by up to 5x whenever more than one statistic was
requested per metric, making it unusable for the cost estimation it
exists for.

Approach:
Added billedGetMetricDataMetricsCount, which groups the CloudwatchData
requested in a GetMetricData call by metric identity (namespace, metric
name, sorted dimensions) and sums ceil(statistics / 5) per group,
matching AWS's documented batching. This only changes the value fed
into the CloudwatchGetMetricDataAPIMetricsCounter metric; it does not
change any CloudWatch API calls, exported metric data, or scrape
behavior.

Validation:
- go build ./...
- go test ./pkg/... (all packages pass, including new
  Test_billedGetMetricDataMetricsCount cases covering 1, exactly 5,
  and 6 statistics per metric, and distinct metrics/dimensions)
- make lint (golangci-lint, 0 issues)
- make build

This was validated at the unit-test level against AWS's documented
GetMetricData billing rule (up to 5 statistics per metric batched into
one billed metric request); it was not validated against a live AWS
bill, which this repo's CONTRIBUTE.md does not require for a change of
this kind.

Report: prometheus-community#1806
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
@pujitha24

Copy link
Copy Markdown
Author

This has been sitting for a bit and is still rebased cleanly on master with DCO passing — happy to adjust anything if it would help move review along.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] YACE internal metrics does not reflect actual GetMetricData API usage costs

1 participant