Skip to content

Commit 24c3970

Browse files
fix(billing-cost-management-mcp-server): list ri-performance coverage group-by keys per service (#4833)
The ri-performance description listed one flat set of GetReservationCoverage group_by keys, copied from the API reference and botocore. The service checks the keys per service (from the SERVICE filter, EC2 by default), so the flat list was wrong in both directions: - INVOICING_ENTITY and OPERATING_SYSTEM are rejected for every service. - INSTANCE_TYPE_FAMILY is valid for EC2 but was missing. - CACHE_ENGINE, DATABASE_ENGINE and DEPLOYMENT_OPTION only work with their own service's filter; with the EC2 default they return a ValidationException. Also corrected: - A UUID or ARN SubscriptionId filter returns a ValidationException ("must contain only digits"), not an empty match. - The utilization SERVICE filter accepts several values; only coverage caps it, at 2. - Added MemoryDB, OpenSearch and Elasticsearch to the coverage service list. Every claim was checked against live GetReservationCoverage and GetReservationUtilization calls, whose errors list the accepted keys per service, e.g. "Only AZ, INSTANCE_TYPE, INSTANCE_TYPE_FAMILY, LINKED_ACCOUNT, PLATFORM, REGION, TENANCY is supported for groupBy with type DIMENSION for Amazon Elastic Compute Cloud - Compute". Tests: a contract test pins the documented per-service keys, plus tests for the invalid keys and the SubscriptionId format. Fixtures no longer group coverage by SERVICE or use "Amazon EC2" as a SERVICE value.
1 parent 2e65667 commit 24c3970

2 files changed

Lines changed: 116 additions & 24 deletions

File tree

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/ri_performance_tools.py‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,38 +56,38 @@
5656
- Helps identify RIs that could be modified or sold in the marketplace
5757
5858
GROUPING get_reservation_coverage (group_by):
59-
GetReservationCoverage groups ONLY by these attributes:
59+
GetReservationCoverage groups ONLY by these attributes. Keys marked "only" apply just to
60+
that service (from the SERVICE filter, EC2 when omitted); others return a ValidationException:
6061
- AZ: Availability Zone (e.g. us-east-1a)
61-
- CACHE_ENGINE: ElastiCache engine (e.g. Redis, Memcached)
62-
- DATABASE_ENGINE: RDS database engine (e.g. PostgreSQL, MySQL, Aurora MySQL)
63-
- DEPLOYMENT_OPTION: deployment mode (e.g. Single-AZ, Multi-AZ)
62+
- CACHE_ENGINE: ElastiCache engine (e.g. Redis, Memcached). ElastiCache and MemoryDB only
63+
- DATABASE_ENGINE: RDS database engine (e.g. PostgreSQL, MySQL, Aurora MySQL). RDS only
64+
- DEPLOYMENT_OPTION: deployment mode (e.g. Single-AZ, Multi-AZ). RDS only
6465
- INSTANCE_TYPE: instance type (e.g. m4.xlarge, cache.r6g.large)
65-
- INVOICING_ENTITY: the Amazon entity that issues the invoice
66+
- INSTANCE_TYPE_FAMILY: instance family (e.g. m5, t3). EC2 only
6667
- LINKED_ACCOUNT: member account id in the organization
67-
- OPERATING_SYSTEM: operating system (e.g. Linux, Windows, RHEL)
68-
- PLATFORM: operating system / platform
68+
- PLATFORM: operating system / platform. EC2 only
6969
- REGION: AWS Region (e.g. us-east-1)
70-
- TENANCY: instance tenancy. Exact, case-sensitive values: Shared, Dedicated, Host
71-
SERVICE is NOT a valid group_by attribute here — grouping coverage by SERVICE
72-
returns a ValidationException. SERVICE is a FILTER dimension instead: pass it in
70+
- TENANCY: instance tenancy. Exact, case-sensitive values: Shared, Dedicated, Host. EC2 only
71+
SERVICE, INVOICING_ENTITY and OPERATING_SYSTEM are NOT valid group_by attributes here —
72+
grouping coverage by them returns a ValidationException. SERVICE is a FILTER dimension instead: pass it in
7373
`filter` (one SERVICE value per call; if you omit a SERVICE filter, Cost Explorer
7474
defaults to EC2), and call once per service to compare across services.
7575
Reservation coverage applies only to reservable services. The valid SERVICE
7676
filter values are "Amazon Elastic Compute Cloud - Compute" (the default when
77-
omitted), "Amazon Relational Database Service", "Amazon ElastiCache", and
78-
"Amazon Redshift". Use the full Cost Explorer service display name, not
77+
omitted), "Amazon Relational Database Service", "Amazon ElastiCache",
78+
"Amazon MemoryDB", "Amazon Redshift", "Amazon OpenSearch Service", and
79+
"Amazon Elasticsearch Service". Use the full Cost Explorer service display name, not
7980
"EC2"/"RDS".
8081
8182
GROUPING get_reservation_utilization (group_by):
8283
GetReservationUtilization groups ONLY by SUBSCRIPTION_ID. No other attribute is
8384
valid. The SubscriptionId it returns (and that you filter on) is a NUMERIC id
8485
(e.g. "359809062"), not an ARN, instance id, or UUID — filtering SubscriptionId
85-
with an ARN/UUID returns no match.
86+
with an ARN/UUID returns a ValidationException ("must contain only digits").
8687
8788
FILTERING (both operations): the Cost Explorer Expression supports only AND among
8889
dimensions and nests one level deep; multiple values for one dimension are OR'd.
89-
The SERVICE filter takes a single value and defaults to Amazon Elastic Compute
90-
Cloud - Compute when omitted.
90+
The SERVICE filter defaults to Amazon Elastic Compute Cloud - Compute when omitted.
9191
9292
SCOPING LARGE QUERIES: a grouped or DAILY query over a wide window or many
9393
subscriptions can exceed the tool result size cap and return a ToolResultOversized
@@ -138,8 +138,8 @@ async def ri_performance(
138138
end_date: End date in YYYY-MM-DD format (exclusive). Defaults to today if not provided.
139139
granularity: Time granularity of the data (DAILY or MONTHLY). Defaults to DAILY. Ignored when group_by is provided (granularity and group_by are mutually exclusive).
140140
metrics: List of metrics to retrieve for coverage as a JSON string (e.g., '["HoursCoverage", "CostCoverage"]'). Defaults to all metrics.
141-
group_by: Optional grouping as a JSON string. For coverage: AZ, CACHE_ENGINE, DATABASE_ENGINE, DEPLOYMENT_OPTION, INSTANCE_TYPE, INVOICING_ENTITY, LINKED_ACCOUNT, OPERATING_SYSTEM, PLATFORM, REGION, or TENANCY (SERVICE is NOT valid — filter on it instead). For utilization: SUBSCRIPTION_ID only.
142-
filter: Optional filter as a JSON string (e.g., by service, region, or instance type). SERVICE takes a single value and defaults to EC2 when omitted; valid coverage SERVICE values are "Amazon Elastic Compute Cloud - Compute", "Amazon Relational Database Service", "Amazon ElastiCache", and "Amazon Redshift" (use the full display name, not "EC2"/"RDS"). SubscriptionId values are numeric, not ARNs/UUIDs. A wide grouped/DAILY query may exceed the result size cap (ToolResultOversized) — scope it with a filter, a shorter window, or MONTHLY granularity.
141+
group_by: Optional grouping as a JSON string. For coverage: AZ, CACHE_ENGINE, DATABASE_ENGINE, DEPLOYMENT_OPTION, INSTANCE_TYPE, INSTANCE_TYPE_FAMILY, LINKED_ACCOUNT, PLATFORM, REGION, or TENANCY, depending on the service (see the tool description) (SERVICE is NOT valid — filter on it instead). For utilization: SUBSCRIPTION_ID only.
142+
filter: Optional filter as a JSON string (e.g., by service, region, or instance type). SERVICE defaults to EC2 when omitted; valid coverage SERVICE values are "Amazon Elastic Compute Cloud - Compute", "Amazon Relational Database Service", "Amazon ElastiCache", "Amazon MemoryDB", "Amazon Redshift", "Amazon OpenSearch Service", and "Amazon Elasticsearch Service" (use the full display name, not "EC2"/"RDS"). SubscriptionId values are numeric, not ARNs/UUIDs. A wide grouped/DAILY query may exceed the result size cap (ToolResultOversized) — scope it with a filter, a shorter window, or MONTHLY granularity.
143143
sort_by: Optional sorting configuration as a JSON string with key and direction (ASCENDING or DESCENDING).
144144
max_items: Total number of groups to return across all pages. Omit it to fetch every
145145
page. Applies only to get_reservation_coverage with group_by set.

‎src/billing-cost-management-mcp-server/tests/tools/test_ri_performance_tools.py‎

Lines changed: 99 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import botocore.session
2626
import pytest
27+
import re
2728
from awslabs.billing_cost_management_mcp_server.tools.ri_performance_tools import (
2829
format_coverage_metrics,
2930
format_utilization_metrics,
@@ -33,6 +34,7 @@
3334
)
3435
from fastmcp import Context
3536
from fastmcp.tools import FunctionTool
37+
from typing import Dict
3638
from unittest.mock import AsyncMock, MagicMock, patch
3739

3840

@@ -186,7 +188,7 @@ def mock_ce_client():
186188
'Groups': [
187189
{
188190
'Attributes': {
189-
'SUBSCRIPTION_ID': '012345678901-ec2-us-east-1-t3.large',
191+
'SUBSCRIPTION_ID': '359809062',
190192
},
191193
'Utilization': {
192194
'UtilizationPercentage': '90.0',
@@ -197,7 +199,7 @@ def mock_ce_client():
197199
},
198200
{
199201
'Attributes': {
200-
'SUBSCRIPTION_ID': '012345678901-rds-us-east-1-db.m5.large',
202+
'SUBSCRIPTION_ID': '359809070',
201203
},
202204
'Utilization': {
203205
'UtilizationPercentage': '80.0',
@@ -481,8 +483,10 @@ async def test_get_reservation_coverage_with_options(
481483
)
482484

483485
mock_metrics = ['CoverageHours', 'CoverageCost']
484-
mock_group_by = [{'Type': 'DIMENSION', 'Key': 'SERVICE'}]
485-
mock_filter = {'Dimensions': {'Key': 'SERVICE', 'Values': ['Amazon EC2']}}
486+
mock_group_by = [{'Type': 'DIMENSION', 'Key': 'INSTANCE_TYPE'}]
487+
mock_filter = {
488+
'Dimensions': {'Key': 'SERVICE', 'Values': ['Amazon Elastic Compute Cloud - Compute']}
489+
}
486490
mock_sort_by = {'Key': 'CoverageHoursPercentage', 'SortOrder': 'DESCENDING'}
487491

488492
mock_parse_json.side_effect = [mock_metrics, mock_group_by, mock_filter, mock_sort_by]
@@ -640,7 +644,9 @@ async def test_get_reservation_utilization_with_options(
640644
)
641645

642646
mock_group_by = [{'Type': 'DIMENSION', 'Key': 'SUBSCRIPTION_ID'}]
643-
mock_filter = {'Dimensions': {'Key': 'SERVICE', 'Values': ['Amazon EC2']}}
647+
mock_filter = {
648+
'Dimensions': {'Key': 'SERVICE', 'Values': ['Amazon Elastic Compute Cloud - Compute']}
649+
}
644650
mock_sort_by = {'Key': 'UtilizationPercentage', 'SortOrder': 'DESCENDING'}
645651

646652
mock_parse_json.side_effect = [mock_group_by, mock_filter, mock_sort_by]
@@ -761,8 +767,8 @@ async def test_ri_performance_with_all_params(self, mock_context):
761767
"""Test ri_performance with all parameters."""
762768
# Setup
763769
metrics = '["CoverageHours", "CoverageCost"]'
764-
group_by = '[{"Type": "DIMENSION", "Key": "SERVICE"}]'
765-
filter_expr = '{"Dimensions": {"Key": "SERVICE", "Values": ["Amazon EC2"]}}'
770+
group_by = '[{"Type": "DIMENSION", "Key": "INSTANCE_TYPE"}]'
771+
filter_expr = '{"Dimensions": {"Key": "SERVICE", "Values": ["Amazon Elastic Compute Cloud - Compute"]}}'
766772
sort_by = '{"Key": "CoverageHoursPercentage", "SortOrder": "DESCENDING"}'
767773

768774
# Execute
@@ -1089,6 +1095,92 @@ async def test_description_says_when_total_is_returned():
10891095
assert 'covers the whole query; it is not repeated' not in description
10901096

10911097

1098+
# GetReservationCoverage GroupBy keys per SERVICE filter value, as the service validates
1099+
# them (Service.coverageGroupByDims) and confirmed by live calls. The API reference and
1100+
# botocore list one flat set that includes INVOICING_ENTITY and OPERATING_SYSTEM, which no
1101+
# service accepts, and omits INSTANCE_TYPE_FAMILY.
1102+
COVERAGE_GROUP_BY_BY_SERVICE = {
1103+
'Amazon Elastic Compute Cloud - Compute': {
1104+
'AZ',
1105+
'INSTANCE_TYPE',
1106+
'INSTANCE_TYPE_FAMILY',
1107+
'LINKED_ACCOUNT',
1108+
'PLATFORM',
1109+
'REGION',
1110+
'TENANCY',
1111+
},
1112+
'Amazon Relational Database Service': {
1113+
'AZ',
1114+
'INSTANCE_TYPE',
1115+
'LINKED_ACCOUNT',
1116+
'DATABASE_ENGINE',
1117+
'REGION',
1118+
'DEPLOYMENT_OPTION',
1119+
},
1120+
'Amazon ElastiCache': {'AZ', 'INSTANCE_TYPE', 'LINKED_ACCOUNT', 'CACHE_ENGINE', 'REGION'},
1121+
'Amazon MemoryDB': {'AZ', 'INSTANCE_TYPE', 'LINKED_ACCOUNT', 'CACHE_ENGINE', 'REGION'},
1122+
'Amazon Redshift': {'AZ', 'INSTANCE_TYPE', 'LINKED_ACCOUNT', 'REGION'},
1123+
'Amazon OpenSearch Service': {'AZ', 'INSTANCE_TYPE', 'LINKED_ACCOUNT', 'REGION'},
1124+
'Amazon Elasticsearch Service': {'AZ', 'INSTANCE_TYPE', 'LINKED_ACCOUNT', 'REGION'},
1125+
}
1126+
1127+
1128+
async def _ri_description() -> str:
1129+
tool = await ri_performance_server.get_tool('ri-performance')
1130+
assert tool is not None
1131+
return tool.description or ''
1132+
1133+
1134+
# The "<scope> only" suffixes the coverage attribute list uses, and the services each names.
1135+
COVERAGE_SCOPES = {
1136+
'EC2': {'Amazon Elastic Compute Cloud - Compute'},
1137+
'RDS': {'Amazon Relational Database Service'},
1138+
'ElastiCache and MemoryDB': {'Amazon ElastiCache', 'Amazon MemoryDB'},
1139+
}
1140+
1141+
1142+
def _documented_coverage_group_by(description: str) -> Dict[str, set]:
1143+
"""Per service, the coverage attributes documented for it ("<scope> only" or every service)."""
1144+
block = description.split('GROUPING get_reservation_coverage (group_by):', 1)[1]
1145+
documented: Dict[str, set] = {service: set() for service in COVERAGE_GROUP_BY_BY_SERVICE}
1146+
for line in block.split('\n\n', 1)[0].splitlines():
1147+
match = re.match(r'- ([A-Z_]+): .*?(?:\. ([A-Za-z0-9 ]+) only)?$', line)
1148+
if match:
1149+
key, scope = match.groups()
1150+
for service in COVERAGE_SCOPES[scope] if scope else documented:
1151+
documented[service].add(key)
1152+
return documented
1153+
1154+
1155+
@pytest.mark.asyncio
1156+
async def test_description_lists_coverage_group_by_keys_per_service():
1157+
"""Each service's documented coverage group_by keys equal the keys the service accepts."""
1158+
assert _documented_coverage_group_by(await _ri_description()) == COVERAGE_GROUP_BY_BY_SERVICE
1159+
1160+
1161+
@pytest.mark.asyncio
1162+
async def test_description_names_keys_no_service_accepts_for_coverage():
1163+
"""SERVICE, INVOICING_ENTITY and OPERATING_SYSTEM are called out as invalid for every service."""
1164+
description = ' '.join((await _ri_description()).split())
1165+
assert (
1166+
'SERVICE, INVOICING_ENTITY and OPERATING_SYSTEM are NOT valid group_by attributes here'
1167+
in description
1168+
)
1169+
assert 'call once per service to compare across services' in description
1170+
1171+
1172+
@pytest.mark.asyncio
1173+
async def test_description_states_subscription_id_format_and_error():
1174+
"""SubscriptionId is digits only; an ARN or UUID is rejected, not silently unmatched."""
1175+
description = ' '.join((await _ri_description()).split())
1176+
assert 'GetReservationUtilization groups ONLY by SUBSCRIPTION_ID' in description
1177+
assert 'is a NUMERIC id' in description
1178+
assert 'must contain only digits' in description
1179+
assert 'returns no match' not in description
1180+
# Utilization accepts several SERVICE values; only coverage caps them at 2.
1181+
assert 'SERVICE filter takes a single value' not in description
1182+
1183+
10921184
@pytest.mark.asyncio
10931185
async def test_ungrouped_utilization_ignores_max_items_and_next_token(mock_context):
10941186
"""GetReservationUtilization rejects any token without GroupBy, so none is sent."""

0 commit comments

Comments
 (0)