Skip to content

fix cloudwatch get_metric_data for multiple dimensions #11270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,11 @@ def get_metric_data_stat(
metric.get("MetricName"),
)

dimension_filter = ""
for dimension in dimensions:
dimension_filter += "AND dimensions LIKE ? "
data = data + (f"%{dimension.get('Name')}={dimension.get('Value','')}%",)

if not dimensions:
dimension_filter = "AND dimensions is null "
dimension_filter = "AND dimensions is null " if not dimensions else "AND dimensions LIKE ? "
if dimensions:
data = data + (
self._get_ordered_dimensions_with_separator(dimensions, for_search=True),
)

unit_filter = ""
if unit:
Expand All @@ -275,8 +273,8 @@ def get_metric_data_stat(
) AS combined
WHERE account_id = ? AND region = ?
AND namespace = ? AND metric_name = ?
{unit_filter}
{dimension_filter}
{unit_filter}
AND timestamp >= ? AND timestamp < ?
ORDER BY timestamp ASC
"""
Expand Down Expand Up @@ -326,18 +324,19 @@ def list_metrics(

namespace_filter = ""
if namespace:
namespace_filter = "AND namespace = ?"
namespace_filter = " AND namespace = ?"
data = data + (namespace,)

metric_name_filter = ""
if metric_name:
metric_name_filter = "AND metric_name = ?"
metric_name_filter = " AND metric_name = ?"
data = data + (metric_name,)

dimension_filter = ""
for dimension in dimensions:
dimension_filter += "AND dimensions LIKE ? "
data = data + (f"%{dimension.get('Name')}={dimension.get('Value','')}%",)
dimension_filter = "" if not dimensions else " AND dimensions LIKE ? "
if dimensions:
data = data + (
self._get_ordered_dimensions_with_separator(dimensions, for_search=True),
)

query = f"""
SELECT DISTINCT metric_name, namespace, dimensions
Expand Down Expand Up @@ -383,13 +382,25 @@ def clear_tables(self):
cur.execute("VACUUM")
conn.commit()

def _get_ordered_dimensions_with_separator(self, dims: Optional[List[Dict]]):
def _get_ordered_dimensions_with_separator(self, dims: Optional[List[Dict]], for_search=False):
"""
Returns a string with the dimensions in the format "Name=Value\tName=Value\tName=Value" in order to store the metric
with the dimensions in a single column in the database

:param dims: List of dimensions in the format [{"Name": "name", "Value": "value"}, ...]
:param for_search: If True, the dimensions will be formatted in a way that can be used in a LIKE query to search. Default is False. Example: " %{Name}={Value}% "
:return: String with the dimensions in the format "Name=Value\tName=Value\tName=Value"
"""
if not dims:
return None
dims.sort(key=lambda d: d["Name"])
dimensions = ""
for d in dims:
dimensions += f"{d['Name']}={d['Value']}\t" # aws does not allow ascii control characters, we can use it a sa separator
if not for_search:
for d in dims:
dimensions += f"{d['Name']}={d['Value']}\t" # aws does not allow ascii control characters, we can use it a sa separator
else:
for d in dims:
dimensions += f"%{d.get('Name')}={d.get('Value','')}%"

return dimensions

Expand Down
95 changes: 95 additions & 0 deletions tests/aws/services/cloudwatch/test_cloudwatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -2507,6 +2507,101 @@ def test_delete_alarm(self, aws_client, snapshot):
describe_alarm = aws_client.cloudwatch.describe_alarms(AlarmNames=[alarm_name])
snapshot.match("describe-after-delete", describe_alarm)

@markers.aws.validated
@markers.snapshot.skip_snapshot_verify(
condition=is_old_provider,
paths=[
"$..list-metrics..Metrics",
],
)
def test_multiple_dimensions_statistics(self, aws_client, snapshot):
snapshot.add_transformer(snapshot.transform.cloudwatch_api())

utc_now = datetime.now(tz=timezone.utc)
namespace = f"test/{short_uid()}"
metric_name = "http.server.requests.count"
dimensions = [
{"Name": "error", "Value": "none"},
{"Name": "exception", "Value": "none"},
{"Name": "method", "Value": "GET"},
{"Name": "outcome", "Value": "SUCCESS"},
{"Name": "uri", "Value": "/greetings"},
{"Name": "status", "Value": "200"},
]
aws_client.cloudwatch.put_metric_data(
Namespace=namespace,
MetricData=[
{
"MetricName": metric_name,
"Value": 0.0,
"Unit": "Count",
"StorageResolution": 1,
"Dimensions": dimensions,
"Timestamp": datetime.now(tz=timezone.utc),
}
],
)
aws_client.cloudwatch.put_metric_data(
Namespace=namespace,
MetricData=[
{
"MetricName": metric_name,
"Value": 5.0,
"Unit": "Count",
"StorageResolution": 1,
"Dimensions": dimensions,
"Timestamp": datetime.now(tz=timezone.utc),
}
],
)

def assert_results():
response = aws_client.cloudwatch.get_metric_data(
MetricDataQueries=[
{
"Id": "result1",
"MetricStat": {
"Metric": {
"Namespace": namespace,
"MetricName": metric_name,
"Dimensions": dimensions,
},
"Period": 10,
"Stat": "Maximum",
"Unit": "Count",
},
}
],
StartTime=utc_now - timedelta(seconds=60),
EndTime=utc_now + timedelta(seconds=60),
)

assert len(response["MetricDataResults"][0]["Values"]) > 0
snapshot.match("get-metric-stats-max", response)

retries = 10 if is_aws_cloud() else 1
sleep_before = 2 if is_aws_cloud() else 0
retry(assert_results, retries=retries, sleep_before=sleep_before)

def list_metrics():
res = aws_client.cloudwatch.list_metrics(
Namespace=namespace, MetricName=metric_name, Dimensions=dimensions
)
assert len(res["Metrics"]) > 0
return res

retries = 10 if is_aws_cloud() else 1
sleep_before = 2 if is_aws_cloud() else 0
list_metrics_res = retry(list_metrics, retries=retries, sleep_before=sleep_before)

# Function to sort the dimensions by "Name"
def sort_dimensions(data: dict):
for metric in data["Metrics"]:
metric["Dimensions"] = sorted(metric["Dimensions"], key=lambda x: x["Name"])

sort_dimensions(list_metrics_res)
snapshot.match("list-metrics", list_metrics_res)


def _get_lambda_logs(logs_client: "CloudWatchLogsClient", fn_name: str):
log_events = logs_client.filter_log_events(logGroupName=f"/aws/lambda/{fn_name}")["events"]
Expand Down
61 changes: 61 additions & 0 deletions tests/aws/services/cloudwatch/test_cloudwatch.snapshot.json
Original file line number Diff line number Diff line change
Expand Up @@ -1984,5 +1984,66 @@
}
}
}
},
"tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_multiple_dimensions_statistics": {
"recorded-date": "26-07-2024, 15:38:56",
"recorded-content": {
"get-metric-stats-max": {
"Messages": [],
"MetricDataResults": [
{
"Id": "result1",
"Label": "http.server.requests.count",
"StatusCode": "Complete",
"Timestamps": "timestamp",
"Values": [
5.0
]
}
],
"ResponseMetadata": {
"HTTPHeaders": {},
"HTTPStatusCode": 200
}
},
"list-metrics": {
"Metrics": [
{
"Dimensions": [
{
"Name": "error",
"Value": "none"
},
{
"Name": "exception",
"Value": "none"
},
{
"Name": "method",
"Value": "GET"
},
{
"Name": "outcome",
"Value": "SUCCESS"
},
{
"Name": "status",
"Value": "200"
},
{
"Name": "uri",
"Value": "/greetings"
}
],
"MetricName": "http.server.requests.count",
"Namespace": "<namespace:1>"
}
],
"ResponseMetadata": {
"HTTPHeaders": {},
"HTTPStatusCode": 200
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
"tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_insight_rule": {
"last_validated_date": "2023-10-26T08:07:59+00:00"
},
"tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_multiple_dimensions_statistics": {
"last_validated_date": "2024-07-29T07:56:05+00:00"
},
"tests/aws/services/cloudwatch/test_cloudwatch.py::TestCloudwatch::test_put_metric_alarm": {
"last_validated_date": "2024-01-19T14:26:26+00:00"
},
Expand Down
Loading