Skip to content

Commit 76a1b67

Browse files
fix(billing-cost-management-mcp-server): one timestamp formatter for datetime, epoch seconds and epoch ms (#4757)
recommendation_details_tools.format_timestamp did datetime.fromtimestamp(timestamp / 1000) with no type check. boto3 returns datetime objects, so every rec-details last_refresh_timestamp became "Error: unsupported operand type(s) for /: 'datetime.datetime' and 'int'". cost_optimization_hub_helpers.format_timestamp assumed every number was epoch milliseconds (so epoch seconds landed in January 1970) and rendered it in local time. Both wrote exception text into the output field on failure. Add time_utils.format_timestamp_to_utc_iso: it accepts datetime (naive is taken as UTC, aware is converted), epoch seconds, and epoch milliseconds (numbers above 1e11), plus numeric or ISO 8601 strings, and returns a UTC ISO string. Missing or unconvertible values return None and log a warning instead of an error string. Both format_timestamp functions now delegate to it.
1 parent 6c7a4ae commit 76a1b67

7 files changed

Lines changed: 213 additions & 44 deletions

File tree

‎src/billing-cost-management-mcp-server/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4747
### Fixed
4848
- Fixed the `rec-details` tool returning an error instead of AWS Compute Optimizer details for RDS recommendations. It called `get_rds_instance_recommendations`, which does not exist in boto3; it now calls `get_rds_database_recommendations` with `resourceArns`.
4949
- Fixed the `compute-optimizer-automation` tool rejecting `start_time`/`end_time` values that end in the ISO 8601 UTC designator `Z` (e.g. `2026-07-10T00:00:00Z`). A single trailing `Z` is now accepted; the values were already interpreted as UTC.
50+
- Fixed timestamp formatting in the `rec-details` tool and Cost Optimization Hub responses. `rec-details` divided boto3 `datetime` values by 1000, so every `last_refresh_timestamp` became an error string; Cost Optimization Hub treated every number as epoch milliseconds (placing epoch seconds in January 1970) and rendered it in local time. Both now use a shared formatter that accepts `datetime`, epoch seconds, and epoch milliseconds and returns a UTC ISO 8601 string, or `null` (with a logged warning) instead of an error string when a value cannot be converted.
5051
- Fixed pagination metadata reporting more pages when an AWS API returns an empty-string pagination token on its last page. Some APIs (e.g. Savings Plans `DescribeSavingsPlansOfferingRates`) return `nextToken: ""` rather than omitting it, which made `paginate_aws_response` report `has_more: true`, `complete_dataset: false`, and `next_token: ""` even though all pages had been fetched. The metadata now treats any falsy token as no more pages (`has_more: false`, `complete_dataset: true`, `next_token: null`). A non-empty token returned when the `max_pages` limit is reached still reports `has_more: true` with that token. The `get_credit_allocation_history` and `get_savings_plans_purchase_recommendation` tools, which build their own pagination metadata instead of using `paginate_aws_response`, carried the same bug and are fixed the same way.
5152
- Fixed `get_savings_plans_utilization` returning "No Savings Plans utilization data found" on every call (#3351). It read `SavingsPlansUtilizations` where the API returns `SavingsPlansUtilizationsByTime`, and behind that early return the figures were read from each row's top level as `{Amount, Unit}` mappings, but the API nests them under `Utilization`, `Savings` and `AmortizedCommitment` as decimal strings. Rows and `Total` now pass through as the API sends them, preserving decimal precision and the previously discarded blocks
5253
- Fixed `get_savings_plans_utilization_details` reporting every plan's utilization and savings as zero, from the same two causes. Rows now pass through as the API sends them, including the previously discarded `AmortizedCommitment` block and the whole of `Attributes` -- superseding the `summary` block, which read lowercase keys the API does not send, and exposing the owning `AccountId`

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

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -37,40 +37,23 @@
3737
)
3838
from ..utilities.logging_utils import get_context_logger
3939
from ..utilities.sql_utils import convert_response_if_needed
40+
from ..utilities.time_utils import format_timestamp_to_utc_iso
4041
from botocore.exceptions import ClientError
41-
from datetime import datetime
4242
from fastmcp import Context
4343
from typing import Any, Dict, Optional
4444

4545

4646
def format_timestamp(timestamp: Any) -> Optional[str]:
47-
"""Format a timestamp to ISO format string.
47+
"""Format a timestamp to a UTC ISO 8601 string.
4848
4949
Args:
50-
timestamp: Timestamp from Cost Optimization Hub API
50+
timestamp: Timestamp from Cost Optimization Hub API: a datetime (boto3),
51+
epoch seconds, or epoch milliseconds.
5152
5253
Returns:
53-
Formatted timestamp string
54+
Formatted timestamp string, or None if missing or unconvertible.
5455
"""
55-
if not timestamp:
56-
return None
57-
58-
try:
59-
# Check if it's already a datetime object
60-
if isinstance(timestamp, datetime):
61-
return timestamp.isoformat()
62-
else:
63-
# Assume it's a Unix timestamp in milliseconds
64-
from datetime import timezone
65-
66-
return (
67-
datetime.fromtimestamp(timestamp / 1000, tz=timezone.utc)
68-
.astimezone()
69-
.replace(tzinfo=None)
70-
.isoformat()
71-
)
72-
except Exception as e:
73-
return str(f'Error: {e}, Timestamp: {timestamp}')
56+
return format_timestamp_to_utc_iso(timestamp)
7457

7558

7659
async def list_recommendations(

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
SERVICE_MAP,
3838
TERM_MAP,
3939
)
40-
from datetime import datetime
40+
from ..utilities.time_utils import format_timestamp_to_utc_iso
4141
from fastmcp import Context, FastMCP
4242
from typing import Any, Dict, Optional
4343

@@ -382,15 +382,9 @@ async def get_compute_optimizer_data(
382382
return {'error': f'Error getting Compute Optimizer data: {str(e)}'}
383383

384384

385-
def format_timestamp(timestamp: Optional[int]) -> Optional[str]:
386-
"""Format Unix timestamp to ISO format string."""
387-
if timestamp is None:
388-
return None
389-
390-
try:
391-
return datetime.fromtimestamp(timestamp / 1000).isoformat()
392-
except Exception as e:
393-
return str(f'Error: {e}, Timestamp: {timestamp}') # Return as string if conversion fails
385+
def format_timestamp(timestamp: Any) -> Optional[str]:
386+
"""Format a COH timestamp (datetime, epoch seconds or epoch ms) as a UTC ISO string."""
387+
return format_timestamp_to_utc_iso(timestamp)
394388

395389

396390
def get_template_for_recommendation(

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@
1414

1515
"""Time utility functions for the AWS Billing and Cost Management MCP server."""
1616

17+
from .logging_utils import get_logger
1718
from datetime import datetime, timezone
18-
from typing import Any, Union
19+
from typing import Any, Optional, Union
20+
21+
22+
logger = get_logger(__name__)
1923

2024

2125
# Supported UTC datetime formats, ordered from most specific to least specific.
@@ -70,6 +74,66 @@ def timestamp_to_utc_iso_string(timestamp: Union[int, float, datetime]) -> str:
7074
return datetime.fromtimestamp(timestamp, tz=timezone.utc).replace(tzinfo=None).isoformat()
7175

7276

77+
# An epoch value above this is in milliseconds: read as seconds it would fall after
78+
# the year 5000, while read as milliseconds it falls after March 1973.
79+
EPOCH_MILLISECONDS_THRESHOLD = 1e11
80+
81+
82+
def format_timestamp_to_utc_iso(timestamp: Any) -> Optional[str]:
83+
"""Format an AWS timestamp of any wire form as a UTC ISO 8601 string.
84+
85+
The same field arrives in different forms depending on the client: boto3
86+
deserializes timestamps into ``datetime`` objects, while JSON callers see epoch
87+
seconds or, for some services, epoch milliseconds. Accepts:
88+
89+
- ``datetime`` (naive values are taken as UTC; aware values are converted)
90+
- epoch seconds (int/float)
91+
- epoch milliseconds (int/float above ``EPOCH_MILLISECONDS_THRESHOLD``)
92+
- strings holding any of the above numbers, or an ISO 8601 datetime
93+
94+
Args:
95+
timestamp: The timestamp value, or None.
96+
97+
Returns:
98+
ISO 8601 UTC string without offset (e.g. "2023-11-14T22:13:20"), or None when
99+
the value is missing or cannot be converted. Conversion failures are logged,
100+
never written into the returned value.
101+
"""
102+
if timestamp is None:
103+
return None
104+
105+
value = timestamp
106+
if isinstance(value, str):
107+
value = _parse_timestamp_string(value)
108+
109+
try:
110+
if isinstance(value, datetime):
111+
return timestamp_to_utc_iso_string(value)
112+
if isinstance(value, (int, float)) and not isinstance(value, bool):
113+
seconds = value / 1000 if abs(value) > EPOCH_MILLISECONDS_THRESHOLD else value
114+
return timestamp_to_utc_iso_string(seconds)
115+
except (OverflowError, OSError, ValueError):
116+
pass
117+
118+
logger.warning(
119+
f'Could not convert timestamp of type {type(timestamp).__name__}; returning None'
120+
)
121+
return None
122+
123+
124+
def _parse_timestamp_string(text: str) -> Any:
125+
"""Parse a timestamp string into a number or datetime; return None if neither."""
126+
stripped = text.strip()
127+
try:
128+
return float(stripped)
129+
except ValueError:
130+
pass
131+
try:
132+
return datetime.fromisoformat(stripped.replace('Z', '+00:00'))
133+
except ValueError:
134+
return None
135+
136+
73137
def normalize_datetimes_to_iso(obj: Any) -> Any:
74138
"""Recursively convert ``datetime`` values in a structure to ISO 8601 UTC strings.
75139

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,25 @@ def test_format_timestamp_with_none(self):
158158
result = format_timestamp(None)
159159
assert result is None
160160

161+
def test_format_timestamp_with_epoch_seconds(self):
162+
"""Epoch seconds are read as seconds, not milliseconds (no 1970 dates)."""
163+
assert format_timestamp(1756728000) == '2025-09-01T12:00:00'
164+
165+
def test_format_timestamp_with_epoch_milliseconds(self):
166+
"""Epoch milliseconds are still supported."""
167+
assert format_timestamp(1756728000000) == '2025-09-01T12:00:00'
168+
169+
def test_format_timestamp_with_aware_datetime(self):
170+
"""An aware datetime is normalized to UTC."""
171+
from datetime import timezone
172+
173+
value = datetime(2025, 9, 1, 12, 0, tzinfo=timezone.utc)
174+
assert format_timestamp(value) == '2025-09-01T12:00:00'
175+
176+
def test_format_timestamp_unconvertible_returns_none(self):
177+
"""Unconvertible input yields None, never an error string."""
178+
assert format_timestamp('garbage') is None
179+
161180

162181
@pytest.mark.asyncio
163182
class TestListRecommendations:
@@ -1930,6 +1949,26 @@ async def test_no_params_calls_api_without_arguments(self, mock_context):
19301949
}
19311950
]
19321951

1952+
@pytest.mark.asyncio
1953+
async def test_epoch_second_timestamps_are_not_1970(self, mock_context):
1954+
"""JSON callers send epoch seconds; enrollment dates must not land in 1970."""
1955+
client = MagicMock()
1956+
client.list_enrollment_statuses.return_value = {
1957+
'items': [
1958+
{
1959+
**self._ITEM,
1960+
'lastUpdatedTimestamp': 1788264000,
1961+
'createdTimestamp': 1735689600.0,
1962+
}
1963+
]
1964+
}
1965+
1966+
result = await list_enrollment_statuses(mock_context, client)
1967+
1968+
(entry,) = result['data']['enrollment_statuses']
1969+
assert entry['last_updated_timestamp'] == '2026-09-01T12:00:00'
1970+
assert entry['created_timestamp'] == '2025-01-01T00:00:00'
1971+
19331972
@pytest.mark.asyncio
19341973
async def test_account_id_forwarded(self, mock_context):
19351974
"""account_id maps to the API's accountId."""

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

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
process_recommendation,
3636
recommendation_details_server,
3737
)
38+
from datetime import datetime, timezone
3839
from fastmcp import Context
3940
from unittest.mock import AsyncMock, MagicMock, mock_open, patch
4041

@@ -334,18 +335,37 @@ def mock_compute_optimizer_client():
334335

335336
def test_format_timestamp():
336337
"""Test format_timestamp function."""
337-
# Test with a valid timestamp
338-
timestamp = 1632825600000
339-
result = format_timestamp(timestamp)
340-
assert result and '2021-09-28' in result
338+
# Epoch milliseconds
339+
assert format_timestamp(1632825600000) == '2021-09-28T10:40:00'
341340

342341
# Test with None
343-
result = format_timestamp(None)
344-
assert result is None
342+
assert format_timestamp(None) is None
345343

346-
# Test with invalid timestamp
347-
result = format_timestamp(None)
348-
assert result is None
344+
345+
def test_format_timestamp_accepts_boto3_datetime():
346+
"""boto3 returns datetime; it must format, not fail on datetime / int."""
347+
assert format_timestamp(datetime(2021, 9, 28, 10, 40)) == '2021-09-28T10:40:00'
348+
349+
350+
def test_format_timestamp_accepts_epoch_seconds():
351+
"""Epoch seconds (AWS JSON wire form) are not divided by 1000 into 1970."""
352+
assert format_timestamp(1632825600) == '2021-09-28T10:40:00'
353+
354+
355+
def test_format_timestamp_never_returns_error_text():
356+
"""An unconvertible value yields None rather than an 'Error: ...' string."""
357+
assert format_timestamp(object()) is None
358+
359+
360+
def test_format_base_recommendation_with_boto3_datetime():
361+
"""The rec-details last_refresh_timestamp is a real timestamp for boto3 input."""
362+
result = format_base_recommendation(
363+
{
364+
'recommendationId': 'rec-1',
365+
'lastRefreshTimestamp': datetime(2026, 9, 1, 12, 0, tzinfo=timezone.utc),
366+
}
367+
)
368+
assert result['last_refresh_timestamp'] == '2026-09-01T12:00:00'
349369

350370

351371
def test_format_base_recommendation():

‎src/billing-cost-management-mcp-server/tests/utilities/test_time_utils.py‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616

1717
import pytest
1818
from awslabs.billing_cost_management_mcp_server.utilities.time_utils import (
19+
EPOCH_MILLISECONDS_THRESHOLD,
20+
format_timestamp_to_utc_iso,
1921
normalize_datetimes_to_iso,
2022
timestamp_to_utc_iso_string,
2123
)
22-
from datetime import datetime, timezone
24+
from datetime import datetime, timedelta, timezone
25+
from unittest.mock import patch
2326

2427

2528
class TestUtcDatetimeStringToEpochSeconds:
@@ -153,3 +156,68 @@ def test_scalar_passthrough(self):
153156
assert normalize_datetimes_to_iso('hello') == 'hello'
154157
assert normalize_datetimes_to_iso(42) == 42
155158
assert normalize_datetimes_to_iso(None) is None
159+
160+
161+
class TestFormatTimestampToUtcIso:
162+
"""Tests for format_timestamp_to_utc_iso (datetime, epoch seconds, epoch ms)."""
163+
164+
# 2021-09-28T10:40:00Z in each wire form.
165+
EXPECTED = '2021-09-28T10:40:00'
166+
EPOCH_SECONDS = 1632825600
167+
EPOCH_MS = 1632825600000
168+
169+
def test_none_returns_none(self):
170+
"""A missing timestamp is None, not an error string."""
171+
assert format_timestamp_to_utc_iso(None) is None
172+
173+
def test_naive_datetime_is_taken_as_utc(self):
174+
"""boto3-style naive datetime is formatted as-is (UTC)."""
175+
assert format_timestamp_to_utc_iso(datetime(2021, 9, 28, 10, 40)) == self.EXPECTED
176+
177+
def test_aware_datetime_is_converted_to_utc(self):
178+
"""An aware datetime in another zone is converted to UTC."""
179+
eastern = timezone(timedelta(hours=-4))
180+
value = datetime(2021, 9, 28, 6, 40, tzinfo=eastern)
181+
assert format_timestamp_to_utc_iso(value) == self.EXPECTED
182+
183+
def test_epoch_seconds(self):
184+
"""Epoch seconds (the AWS JSON wire form) are not divided by 1000."""
185+
assert format_timestamp_to_utc_iso(self.EPOCH_SECONDS) == self.EXPECTED
186+
187+
def test_epoch_seconds_float(self):
188+
"""Fractional epoch seconds keep their sub-second part."""
189+
assert format_timestamp_to_utc_iso(1632825600.5) == '2021-09-28T10:40:00.500000'
190+
191+
def test_epoch_milliseconds(self):
192+
"""Epoch milliseconds are recognized by magnitude."""
193+
assert format_timestamp_to_utc_iso(self.EPOCH_MS) == self.EXPECTED
194+
195+
def test_threshold_boundary(self):
196+
"""Values at the threshold are seconds; values above are milliseconds."""
197+
at = format_timestamp_to_utc_iso(EPOCH_MILLISECONDS_THRESHOLD)
198+
assert at is not None and at.startswith('5138-')
199+
above = format_timestamp_to_utc_iso(EPOCH_MILLISECONDS_THRESHOLD + 1)
200+
assert above is not None and above.startswith('1973-')
201+
202+
def test_epoch_zero_is_a_value(self):
203+
"""Epoch 0 is a real instant, not a missing value."""
204+
assert format_timestamp_to_utc_iso(0) == '1970-01-01T00:00:00'
205+
206+
@pytest.mark.parametrize(
207+
'value',
208+
['1632825600', '1632825600000', '2021-09-28T10:40:00Z', '2021-09-28T10:40:00+00:00'],
209+
)
210+
def test_strings(self, value):
211+
"""Numeric and ISO 8601 strings are accepted."""
212+
assert format_timestamp_to_utc_iso(value) == self.EXPECTED
213+
214+
@pytest.mark.parametrize(
215+
'value', ['not a timestamp', '', True, float('nan'), float('inf'), 1e20, [1], {}]
216+
)
217+
def test_unconvertible_returns_none_and_logs(self, value):
218+
"""Unconvertible input returns None and logs; no error text in the value."""
219+
with patch(
220+
'awslabs.billing_cost_management_mcp_server.utilities.time_utils.logger'
221+
) as mock_logger:
222+
assert format_timestamp_to_utc_iso(value) is None
223+
mock_logger.warning.assert_called_once()

0 commit comments

Comments
 (0)