Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- `opentelemetry-sdk`: Fix mutable attributes reference in metrics: attributes passed to instrument `add`/`record` are now copied so that subsequent mutations to the caller's dict do not affect recorded data points
([#4610](https://github.com/open-telemetry/opentelemetry-python/issues/4610))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changelog entries should link to the PR, not issue.

Suggested change
- `opentelemetry-sdk`: Fix mutable attributes reference in metrics: attributes passed to instrument `add`/`record` are now copied so that subsequent mutations to the caller's dict do not affect recorded data points
([#4610](https://github.com/open-telemetry/opentelemetry-python/issues/4610))
- `opentelemetry-sdk`: Fix mutable attributes reference in metrics: attributes passed to instrument `add`/`record` are now copied so that subsequent mutations to the caller's dict do not affect recorded data points
([#5106](https://github.com/open-telemetry/opentelemetry-python/issues/5106))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — updated the changelog link to point to the PR and also updated the PR description to follow the template. Thanks for the review!

- `opentelemetry-sdk`: Add `create_logger_provider`/`configure_logger_provider` to declarative file configuration, enabling LoggerProvider instantiation from config files without reading env vars
([#4990](https://github.com/open-telemetry/opentelemetry-python/pull/4990))
- `opentelemetry-sdk`: Add `service` resource detector support to declarative file configuration via `detection_development.detectors[].service`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def consume_measurement(
if key in self._view._attribute_keys:
attributes[key] = value
elif measurement.attributes is not None:
attributes = measurement.attributes
attributes = dict(measurement.attributes)
else:
attributes = {}

Expand Down
45 changes: 45 additions & 0 deletions opentelemetry-sdk/tests/metrics/test_view_instrument_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,51 @@ def test_collect(self):
self.assertEqual(number_data_point.attributes, {"c": "d"})
self.assertEqual(number_data_point.value, 0)

def test_consume_measurement_attributes_are_copied(self):
"""Mutating the attributes dict after recording must not affect stored data points."""
instrument1 = _Counter(
"instrument1",
Mock(),
Mock(),
description="description",
unit="unit",
)
instrument1.instrumentation_scope = self.mock_instrumentation_scope
view_instrument_match = _ViewInstrumentMatch(
view=View(
instrument_name="instrument1",
name="name",
aggregation=DefaultAggregation(),
),
instrument=instrument1,
instrument_class_aggregation=MagicMock(
**{"__getitem__.return_value": DefaultAggregation()}
),
)

attributes = {"key": "original"}
view_instrument_match.consume_measurement(
Measurement(
value=1,
time_unix_nano=time_ns(),
instrument=instrument1,
context=Context(),
attributes=attributes,
)
)

# Mutate the original dict after recording
attributes["key"] = "mutated"

number_data_points = view_instrument_match.collect(
AggregationTemporality.CUMULATIVE, 0
)
number_data_points = list(number_data_points)
self.assertEqual(len(number_data_points), 1)
self.assertEqual(
number_data_points[0].attributes, {"key": "original"}
)

@patch(
"opentelemetry.sdk.metrics._internal._view_instrument_match.time_ns",
side_effect=[0, 1, 2],
Expand Down