Skip to content

Commit 4937c97

Browse files
Dinkiesomsubhro
andauthored
fix(billing-cost-management-mcp-server): scope rec-details Cost Explorer calls to the recommendation's account (#4931)
* fix(billing-cost-management-mcp-server): scope rec-details Cost Explorer calls to the recommendation's account rec-details looked up the Cost Explorer purchase recommendation behind a Cost Optimization Hub RI or Savings Plans recommendation without the recommendation's scope. The RI call sent no AccountScope, so Cost Explorer answered at its default (payer) scope with rows for other accounts, instance types and Regions. The Savings Plans call sent the scope but no account, so a Linked recommendation came back with every member account's rows and a summary that totals all of them. - RI: send AccountScope from the recommendation, and for EC2 the offering class (ServiceSpecification.EC2Specification.OfferingClass, 'Standard' -> 'STANDARD', 'Convertible' -> 'CONVERTIBLE'). - RI and SP, Linked: narrow the call with a LINKED_ACCOUNT filter. AccountId is not used: Cost Explorer rejects any AccountId other than the caller's own account ("Request account should not be different from user account"), so a management account cannot name a member account there. - Keep only the rows for the recommendation's account and, for EC2 RIs, its instance type and Region (Cost Explorer returns Region names such as 'US East (Ohio)' and 'EU (Paris)'; codes are mapped through the botocore endpoint descriptions). A "note" counts dropped rows, a "warning" says when no row matched. - The tool description states that the summary is Cost Explorer's own total for every row it returned and is not recomputed. * fix(billing-cost-management-mcp-server): note unmatched non-EC2 RI rows and keep the SP page token - Savings Plans: GetSavingsPlansPurchaseRecommendation returns NextPageToken beside SavingsPlansPurchaseRecommendation, so the no-match warning's "Only the first page was read" hint could never fire. The token is now copied into additional_details. - Non-EC2 RIs (RDS, ElastiCache, Redshift, OpenSearch, MemoryDB, DynamoDB): rows are matched on account only and can span other instance types and regions. The note and the tool description now say so instead of implying the kept rows are the recommendation's own. - constants: note that Cost Explorer accepts only upper-case OfferingClass in requests while its responses echo 'Standard'. * fix(billing-cost-management-mcp-server): keep rec-details RI rows when only the Region name misses - An EC2 RI whose Region name matches no row no longer loses its account and instance-type matches: the rows are kept unfiltered by Region and the note says so, instead of a no-row warning. - The non-EC2 RI note and the tool description say rows are not matched on instance type or region (accurate for Payer scope too), and point DynamoDB at ReservedCapacityDetails. - Page hints say this tool cannot fetch later pages, and the dropped-rows note now mentions unread pages when NextPageToken is set. - Type hints for commitment_config and keep_matching_rows. * fix(billing-cost-management-mcp-server): match rec-details RI Regions per row, not by response-wide fallback The previous fallback kept every account and instance-type match whenever no row's Region name matched, so a row for a genuinely different Region (a regenerated Cost Explorer recommendation) was kept and the no-row warning was lost; it was also decided once for the whole response. Now each row is matched on its own: a row is kept when its Region name is one of the recommendation's Region names, or when it is not the name of any Region botocore knows (a new spelling), and the note names those. A row for another known Region is dropped. The page hint is added whenever rows were kept and NextPageToken is set, even when none were dropped. * test(billing-cost-management-mcp-server): align rec-details tests that #4929, #4956 and this PR each changed the premise of - test_reserved_instances_coh_terms_reach_cost_explorer (from #4929) expected PARTIAL_UPFRONT for a three-year DynamoDB reserved capacity recommendation; #4956 sends HEAVY_UTILIZATION for DynamoDB, so the test failed on main. It now expects HEAVY_UTILIZATION. - test_dynamodb_reserved_capacity_requests_heavy_utilization (from #4956) asserts the exact Cost Explorer request; this PR also sends the recommendation's AccountScope, so the expected request gains AccountScope=PAYER. * fix(billing-cost-management-mcp-server): say which rec-details warning sits where after the ThreeYears merge - The description used "top-level" both for the response and for additional_details. It now says "a note/warning key in additional_details" and "no response-level warning". - The empty-lookup sentence no longer promises an inner warning for every empty commitment answer: the inner warning is added when Cost Explorer returns rows but none for this recommendation. - New test: a Savings Plans lookup whose rows are all for another account leaves the response without a warning and puts one in additional_details. --------- Co-authored-by: Som Chatterjee <5313130+somsubhro@users.noreply.github.com>
1 parent 24fa5e5 commit 4937c97

3 files changed

Lines changed: 743 additions & 8 deletions

File tree

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

Lines changed: 223 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
Updated to use shared utility functions.
1818
"""
1919

20+
import botocore.session
21+
import functools
2022
import os
2123
from ..utilities.aws_service_base import create_aws_client, format_response, handle_aws_error
2224
from ..utilities.constants import (
@@ -27,6 +29,7 @@
2729
ACTION_TYPE_STOP,
2830
DYNAMODB_PAYMENT_OPTION_MAP,
2931
LOOKBACK_PERIOD_MAP,
32+
OFFERING_CLASS_MAP,
3033
PAYMENT_OPTION_MAP,
3134
RESOURCE_TYPE_EBS_VOLUME,
3235
RESOURCE_TYPE_EC2_ASG,
@@ -40,7 +43,7 @@
4043
)
4144
from ..utilities.time_utils import format_timestamp_to_utc_iso
4245
from fastmcp import Context, FastMCP
43-
from typing import Any, Dict, Optional
46+
from typing import Any, Callable, Dict, FrozenSet, List, Optional, Tuple
4447

4548

4649
recommendation_details_server = FastMCP(
@@ -62,12 +65,23 @@
6265
6366
COMMITMENT PURCHASE DETAILS:
6467
- For PurchaseReservedInstances, additional_details is the Cost Explorer reservation purchase
65-
recommendation for the recommendation's service, term and payment option. It may list several
66-
instance types and regions; EstimatedBreakEvenInMonths is per RecommendationDetails row.
68+
recommendation for the recommendation's service, term, payment option and, for EC2, offering
69+
class; EstimatedBreakEvenInMonths is per RecommendationDetails row.
70+
- Cost Explorer is asked for the recommendation's account scope (Payer or Linked), filtered to
71+
the recommendation's account when Linked. RecommendationDetails and
72+
SavingsPlansPurchaseRecommendationDetails then keep only the rows for the recommendation's
73+
own account (and, for EC2 RIs, its instance type and region). The summary is not recomputed
74+
from the kept rows: it totals every row Cost Explorer returned, which for RIs spans other
75+
instance types and regions, so use the kept row(s) for this recommendation. Non-EC2 RI rows are
76+
not matched on instance type or region; an EC2 row whose Region name is not a known Region is
77+
kept and the note names it. A "note" key in additional_details counts dropped rows; a
78+
"warning" key there means Cost Explorer returned rows but none for this recommendation.
6779
- When the Cost Explorer or Compute Optimizer lookup fails or is skipped,
6880
the response leads with a warning and additional_details holds only an error;
6981
base_recommendation is still valid. A lookup that runs but finds nothing
70-
returns empty results and no warning.
82+
returns empty results and no response-level warning (a commitment lookup whose
83+
rows are all for other recommendations still adds its own "warning" inside
84+
additional_details).
7185
- Savings Plans recommendations carry no break-even. They return EstimatedROI (estimated savings /
7286
Savings Plan cost x 100 over the lookback period), which is not a payback period.
7387
- DynamoDB reserved capacity: Cost Optimization Hub shows the payment option as PartialUpfront,
@@ -204,17 +218,195 @@ async def get_cost_explorer_data(ctx: Context, recommendation: Dict[str, Any]) -
204218
try:
205219
if action_type == ACTION_TYPE_PURCHASE_SAVINGS_PLAN:
206220
# Handle Savings Plans recommendations
207-
return await get_savings_plans_recommendation(ctx, recommendation, ce_client)
221+
details = await get_savings_plans_recommendation(ctx, recommendation, ce_client)
222+
return scope_savings_plans_details(details, recommendation)
208223
elif action_type == ACTION_TYPE_PURCHASE_RESERVED_INSTANCE:
209224
# Handle Reserved Instance recommendations
210-
return await get_reserved_instances_recommendation(ctx, recommendation, ce_client)
225+
details = await get_reserved_instances_recommendation(ctx, recommendation, ce_client)
226+
return scope_reserved_instances_details(details, recommendation)
211227
else:
212228
return {'error': 'Unsupported action type for Cost Explorer data'}
213229
except Exception as e:
214230
await ctx.error(f'Error getting Cost Explorer data: {str(e)}')
215231
return {'error': f'Error getting Cost Explorer data: {str(e)}'}
216232

217233

234+
def commitment_config(
235+
recommendation: Dict[str, Any], key_map: Dict[str, str]
236+
) -> Tuple[Optional[str], Dict[str, Any]]:
237+
"""Return the (key, configuration) of the recommendation's commitment details."""
238+
recommended_details = recommendation.get('recommendedResourceDetails', {})
239+
key = next((k for k in recommended_details if k in key_map), None)
240+
if not key:
241+
return None, {}
242+
return key, recommended_details[key].get('configuration', {})
243+
244+
245+
def linked_account(config: Dict[str, Any], recommendation: Dict[str, Any]) -> Optional[str]:
246+
"""Return the account ID of a Linked recommendation, else None."""
247+
if config.get('accountScope') != 'Linked':
248+
return None
249+
return recommendation.get('accountId') or None
250+
251+
252+
def linked_account_filter(
253+
config: Dict[str, Any], recommendation: Dict[str, Any]
254+
) -> Optional[Dict[str, Any]]:
255+
"""Cost Explorer Filter that narrows a Linked recommendation to its own account."""
256+
# Cost Explorer rejects an AccountId other than the caller's own account, so a
257+
# management account cannot name a member account there; LINKED_ACCOUNT can.
258+
account_id = linked_account(config, recommendation)
259+
if not account_id:
260+
return None
261+
return {'Dimensions': {'Key': 'LINKED_ACCOUNT', 'Values': [account_id]}}
262+
263+
264+
@functools.lru_cache(maxsize=None)
265+
def region_names(region_code: str) -> Optional[FrozenSet[str]]:
266+
"""Names Cost Explorer may give a Region code in RI rows, or None if it is unknown."""
267+
for partition in botocore.session.get_session().get_data('endpoints').get('partitions', []):
268+
description = partition.get('regions', {}).get(region_code, {}).get('description')
269+
if description:
270+
# Cost Explorer names older European Regions 'EU (...)', e.g. 'EU (Paris)'.
271+
return frozenset(
272+
{region_code, description, description.replace('Europe (', 'EU (', 1)}
273+
)
274+
return None
275+
276+
277+
@functools.lru_cache(maxsize=None)
278+
def known_region_names() -> FrozenSet[str]:
279+
"""Every name region_names() gives any Region botocore knows."""
280+
codes = {
281+
code
282+
for partition in botocore.session.get_session().get_data('endpoints').get('partitions', [])
283+
for code in partition.get('regions', {})
284+
}
285+
return frozenset(name for code in codes for name in region_names(code) or ())
286+
287+
288+
def keep_matching_rows(
289+
rows: List[Dict[str, Any]], matches: Callable[[Dict[str, Any]], bool]
290+
) -> Tuple[List[Dict[str, Any]], int]:
291+
"""Return (kept rows, number of rows dropped)."""
292+
kept = [row for row in rows if matches(row)]
293+
return kept, len(rows) - len(kept)
294+
295+
296+
def scope_reserved_instances_details(
297+
response: Dict[str, Any], recommendation: Dict[str, Any]
298+
) -> Dict[str, Any]:
299+
"""Keep only the RI rows for the recommendation's account, instance type and Region."""
300+
if 'Recommendations' not in response:
301+
return response
302+
ri_key, config = commitment_config(recommendation, SERVICE_MAP)
303+
account_id = linked_account(config, recommendation)
304+
criteria: Dict[str, str] = {'account': account_id} if account_id else {}
305+
instance_type = config.get('instanceType')
306+
region = config.get('reservedInstancesRegion')
307+
names = region_names(region) if region else None
308+
# Only EC2InstanceDetails is checked against the COH instance type and Region.
309+
if ri_key == 'ec2ReservedInstances':
310+
if instance_type:
311+
criteria['instance type'] = instance_type
312+
if names and region:
313+
criteria['region'] = region
314+
315+
unknown_regions = set()
316+
317+
def region_matches(row_region: Any) -> bool:
318+
if row_region in (names or ()):
319+
return True
320+
# A name that is no known Region's (a new spelling) is kept rather than discarded;
321+
# a row for another known Region is dropped.
322+
if isinstance(row_region, str) and row_region and row_region not in known_region_names():
323+
unknown_regions.add(row_region)
324+
return True
325+
return False
326+
327+
def matches(row: Dict[str, Any]) -> bool:
328+
ec2 = row.get('InstanceDetails', {}).get('EC2InstanceDetails', {})
329+
return (
330+
(not account_id or row.get('AccountId') == account_id)
331+
and ('instance type' not in criteria or ec2.get('InstanceType') == instance_type)
332+
and ('region' not in criteria or region_matches(ec2.get('Region')))
333+
)
334+
335+
scoped = [
336+
keep_matching_rows(ri.get('RecommendationDetails', []), matches)
337+
for ri in response['Recommendations']
338+
]
339+
extra = []
340+
if unknown_regions:
341+
extra.append(
342+
f'Rows whose Region name is not a known Region ({", ".join(sorted(unknown_regions))}) '
343+
f'were kept, not matched to {region}: check their Region.'
344+
)
345+
if ri_key != 'ec2ReservedInstances':
346+
extra.append(
347+
'Rows for this service are not matched on instance type or region, so they can '
348+
'include other instance types and regions: use the row whose InstanceDetails '
349+
"(DynamoDB: ReservedCapacityDetails) match the recommendation's configuration."
350+
)
351+
352+
dropped = kept = 0
353+
for ri, (rows, removed) in zip(response['Recommendations'], scoped):
354+
ri['RecommendationDetails'] = rows
355+
dropped += removed
356+
kept += len(rows)
357+
result = add_scope_notes(response, criteria, kept, dropped, 'RecommendationDetails')
358+
if kept and extra:
359+
result['note'] = ' '.join(([result['note']] if 'note' in result else []) + extra)
360+
return result
361+
362+
363+
def scope_savings_plans_details(
364+
details: Dict[str, Any], recommendation: Dict[str, Any]
365+
) -> Dict[str, Any]:
366+
"""Keep only the Savings Plans rows for a Linked recommendation's account."""
367+
if 'SavingsPlansPurchaseRecommendationDetails' not in details:
368+
return details
369+
_, config = commitment_config(recommendation, SAVINGS_PLANS_TYPE_MAP)
370+
account_id = linked_account(config, recommendation)
371+
criteria: Dict[str, str] = {'account': account_id} if account_id else {}
372+
rows, dropped = keep_matching_rows(
373+
details['SavingsPlansPurchaseRecommendationDetails'],
374+
lambda row: not account_id or row.get('AccountId') == account_id,
375+
)
376+
details['SavingsPlansPurchaseRecommendationDetails'] = rows
377+
return add_scope_notes(
378+
details, criteria, len(rows), dropped, 'SavingsPlansPurchaseRecommendationDetails'
379+
)
380+
381+
382+
def add_scope_notes(
383+
result: Dict[str, Any], criteria: Dict[str, str], kept: int, dropped: int, rows_key: str
384+
) -> Dict[str, Any]:
385+
"""Say when rows were dropped (the summary still covers them) or none matched."""
386+
wanted = ', '.join(f'{name} {value}' for name, value in criteria.items()) or 'its scope'
387+
if kept == 0:
388+
result['warning'] = (
389+
f'Cost Explorer returned no {rows_key} row for this recommendation ({wanted}). '
390+
'Cost Explorer may have regenerated its recommendations since Cost Optimization '
391+
'Hub produced this one.'
392+
)
393+
if result.get('NextPageToken'):
394+
result['warning'] += (
395+
' Only the first page was read (this tool cannot fetch more); the row may be on '
396+
'a later page.'
397+
)
398+
elif dropped:
399+
result['note'] = (
400+
f'{rows_key} keeps only the {kept} row(s) for {wanted}; {dropped} other row(s) '
401+
"were dropped. Cost Explorer's summary still totals all of them, so use the kept "
402+
'row(s) for this recommendation.'
403+
)
404+
if kept and result.get('NextPageToken'):
405+
page_hint = 'Later pages, which this tool cannot fetch, were not read.'
406+
result['note'] = f'{result["note"]} {page_hint}' if 'note' in result else page_hint
407+
return result
408+
409+
218410
def missing_details_error(recommended_details: Dict[str, Any], kind: str) -> Dict[str, Any]:
219411
"""Explain why no supported commitment key was found in recommendedResourceDetails."""
220412
# boto3 parses a union member its service model does not know as
@@ -289,10 +481,18 @@ async def get_savings_plans_recommendation(
289481
if lookback_value is not None:
290482
params['LookbackPeriodInDays'] = lookback_value
291483

484+
account_filter = linked_account_filter(config, recommendation)
485+
if account_filter:
486+
params['Filter'] = account_filter
487+
292488
try:
293489
# Call Cost Explorer API
294490
response = ce_client.get_savings_plans_purchase_recommendation(**params)
295-
return response.get('SavingsPlansPurchaseRecommendation', {})
491+
details = response.get('SavingsPlansPurchaseRecommendation', {})
492+
# NextPageToken sits beside the recommendation, not inside it.
493+
if response.get('NextPageToken'):
494+
details['NextPageToken'] = response['NextPageToken']
495+
return details
296496
except Exception as e:
297497
await ctx.error(f'Error getting Savings Plans recommendation: {str(e)}')
298498
return {'error': f'Error getting Savings Plans recommendation: {str(e)}'}
@@ -347,6 +547,22 @@ async def get_reserved_instances_recommendation(
347547
if lookback_value is not None:
348548
params['LookbackPeriodInDays'] = lookback_value
349549

550+
# Map account scope (when omitted, Cost Explorer uses the caller's own: PAYER for a payer)
551+
account_scope = config.get('accountScope')
552+
if account_scope in ACCOUNT_SCOPE_MAP:
553+
params['AccountScope'] = ACCOUNT_SCOPE_MAP[account_scope]
554+
555+
account_filter = linked_account_filter(config, recommendation)
556+
if account_filter:
557+
params['Filter'] = account_filter
558+
559+
# Map EC2 offering class; ServiceSpecification has no member for other services
560+
offering_class = config.get('offeringClass')
561+
if ri_key == 'ec2ReservedInstances' and offering_class in OFFERING_CLASS_MAP:
562+
params['ServiceSpecification'] = {
563+
'EC2Specification': {'OfferingClass': OFFERING_CLASS_MAP[offering_class]}
564+
}
565+
350566
try:
351567
# Call Cost Explorer API
352568
response = ce_client.get_reservation_purchase_recommendation(**params)

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/utilities/constants.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,10 @@ def _positive_int_env(name: str, default: int) -> int:
231231
# Account scope mapping
232232
ACCOUNT_SCOPE_MAP = {'Linked': 'LINKED', 'Payer': 'PAYER'}
233233

234+
# EC2 RI offering class mapping (Cost Optimization Hub -> Cost Explorer)
235+
# Cost Explorer accepts only the upper-case request value; its responses echo 'Standard'.
236+
OFFERING_CLASS_MAP = {'Standard': 'STANDARD', 'Convertible': 'CONVERTIBLE'}
237+
234238
# Lookback period mapping
235239
LOOKBACK_PERIOD_MAP = {
236240
7: 'SEVEN_DAYS',

0 commit comments

Comments
 (0)