Skip to content

Commit 8e8a56a

Browse files
committed
fix(metrics): warnings when there are default_dimensions
This fixes and reorganizes the code in a few ways: * Make it clearer that _metrics, _dimensions, _metadata and _default_dimensions are class attributes. They don't need to be also set as instance attributes when constructing Metrics, as they're only meant to be used to construct the provider with shared data. * Expose metric_set, dimension_set, metadata_set and default_dimensions as attributes just to keep backwards compatibility. Don't expose setters for these as previously setting them would have no side effect. Now users will get an error, which is a small breaking change but arguably for something they should never be doing anyway. * Fix setting the default_dimensions in AmazonCloudWatchEMFProvider, as `default_dimensions or {}` was setting a new dict instance when the given default_dimensions was empty and we want to share the given dict.
1 parent 8db13c7 commit 8e8a56a

3 files changed

Lines changed: 46 additions & 19 deletions

File tree

‎aws_lambda_powertools/metrics/metrics.py‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -91,21 +91,14 @@ def __init__(
9191
provider: AmazonCloudWatchEMFProvider | None = None,
9292
function_name: str | None = None,
9393
):
94-
self.metric_set = self._metrics
95-
self.metadata_set = self._metadata
96-
self.default_dimensions = self._default_dimensions
97-
self.dimension_set = self._dimensions
98-
99-
self.dimension_set.update(**self._default_dimensions)
100-
10194
if provider is None:
10295
self.provider = AmazonCloudWatchEMFProvider(
10396
namespace=namespace,
10497
service=service,
105-
metric_set=self.metric_set,
106-
dimension_set=self.dimension_set,
107-
metadata_set=self.metadata_set,
108-
default_dimensions=self._default_dimensions,
98+
metric_set=Metrics._metrics,
99+
dimension_set=Metrics._dimensions,
100+
metadata_set=Metrics._metadata,
101+
default_dimensions=Metrics._default_dimensions,
109102
function_name=function_name,
110103
)
111104
else:
@@ -174,7 +167,6 @@ def log_metrics(
174167
)
175168

176169
def set_default_dimensions(self, **dimensions) -> None:
177-
self.provider.set_default_dimensions(**dimensions)
178170
"""Persist dimensions across Lambda invocations
179171
180172
Parameters
@@ -195,14 +187,10 @@ def set_default_dimensions(self, **dimensions) -> None:
195187
def lambda_handler():
196188
return True
197189
"""
198-
for name, value in dimensions.items():
199-
self.add_dimension(name, value)
200-
201-
self.default_dimensions.update(**dimensions)
190+
self.provider.set_default_dimensions(**dimensions)
202191

203192
def clear_default_dimensions(self) -> None:
204193
self.provider.default_dimensions.clear()
205-
self.default_dimensions.clear()
206194

207195
def clear_metrics(self) -> None:
208196
self.provider.clear_metrics()
@@ -227,6 +215,22 @@ def service(self):
227215
def service(self, service):
228216
self.provider.service = service
229217

218+
@property
219+
def metric_set(self):
220+
return self.provider.metric_set
221+
222+
@property
223+
def dimension_set(self):
224+
return self.provider.dimension_set
225+
226+
@property
227+
def metadata_set(self):
228+
return self.provider.metadata_set
229+
230+
@property
231+
def default_dimensions(self):
232+
return self.provider.default_dimensions
233+
230234

231235
# Maintenance: until v3, we can't afford to break customers.
232236
# AmazonCloudWatchEMFProvider has the exact same functionality (non-singleton)

‎aws_lambda_powertools/metrics/provider/cloudwatch_emf/cloudwatch.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ def __init__(
8787
):
8888
self.metric_set = metric_set if metric_set is not None else {}
8989
self.dimension_set = dimension_set if dimension_set is not None else {}
90-
self.default_dimensions = default_dimensions or {}
90+
self.default_dimensions = default_dimensions if default_dimensions is not None else {}
9191
self.namespace = resolve_env_var_choice(choice=namespace, env=os.getenv(constants.METRICS_NAMESPACE_ENV))
9292
self.service = resolve_env_var_choice(choice=service, env=os.getenv(constants.SERVICE_NAME_ENV))
9393
self.function_name = function_name
@@ -453,7 +453,8 @@ def clear_metrics(self) -> None:
453453
self.dimension_set.clear()
454454
self.dimension_sets.clear()
455455
self.metadata_set.clear()
456-
self.set_default_dimensions(**self.default_dimensions)
456+
# Initialize dimension_set as in __init__
457+
self.dimension_set.update(**self.default_dimensions)
457458

458459
def flush_metrics(self, raise_on_empty_metrics: bool = False) -> None:
459460
"""Manually flushes the metrics. This is normally not necessary,

‎tests/functional/metrics/required_dependencies/test_metrics_cloudwatch_emf.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,6 +1221,28 @@ def lambda_handler(evt, ctx):
12211221
assert "environment" in second_invocation
12221222

12231223

1224+
def test_flush_metrics_with_default_dimensions(capsys, metrics, dimensions, namespace):
1225+
# GIVEN a Metrics is initialized
1226+
my_metrics = Metrics(namespace=namespace)
1227+
my_metrics.set_default_dimensions(environment="test", log_group="/lambda/test")
1228+
1229+
# WHEN we add_metric and flush_metrics
1230+
# THEN we should have no warnings
1231+
with warnings.catch_warnings(record=True) as w:
1232+
for metric in metrics:
1233+
my_metrics.add_metric(**metric)
1234+
assert not w
1235+
1236+
with warnings.catch_warnings(record=True) as w:
1237+
my_metrics.flush_metrics()
1238+
assert not w
1239+
1240+
# THEN we should have default dimensions in output
1241+
output = capture_metrics_output(capsys)
1242+
assert "environment" in output
1243+
assert "log_group" in output
1244+
1245+
12241246
def test_metrics_reuse_dimension_set(metric, dimension, namespace):
12251247
# GIVEN Metrics is initialized with a metric and dimension
12261248
my_metrics = Metrics(namespace=namespace)

0 commit comments

Comments
 (0)