Skip to content

Fix/copy measurement attributes #4627

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#4637](https://github.com/open-telemetry/opentelemetry-python/pull/4637))
- Logging API accepts optional `context`; deprecates `trace_id`, `span_id`, `trace_flags`.
([#4597](https://github.com/open-telemetry/opentelemetry-python/pull/4597))
- opentelemetry-sdk: `Measurement`s `Attributes` are now copied when instantiating a `Measurement`. This stops the accidental modification of `Attibutes` after the `Measurement` is created.
([#4627](https://github.com/open-telemetry/opentelemetry-python/pull/4627))

## Version 1.34.0/0.55b0 (2025-06-04)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from copy import deepcopy
from dataclasses import dataclass
from typing import Union

Expand All @@ -34,12 +34,19 @@ class Measurement:
"""

# TODO Fix doc - if using valid Google `Attributes:` key, the attributes are duplicated
# one will come from napoleon extension and the other from autodoc extension. This
# will raise an sphinx error of duplicated object description
# See https://github.com/sphinx-doc/sphinx/issues/8664
# one will come from napoleon extension and the other from autodoc extension. This
# will raise an sphinx error of duplicated object description
# See https://github.com/sphinx-doc/sphinx/issues/8664
Comment on lines +37 to +39
Copy link
Member

Choose a reason for hiding this comment

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

was this intentional?


value: Union[int, float]
time_unix_nano: int
instrument: Instrument
context: Context
attributes: Attributes = None

def __post_init__(self) -> None:
if self.attributes is not None:
super().__setattr__(
"attributes",
deepcopy(self.attributes),
)
Comment on lines +49 to +52
Copy link
Member

Choose a reason for hiding this comment

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

Is this to work around frozen=True?

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import pytest

from opentelemetry.sdk.metrics import Meter, MeterProvider
from opentelemetry.sdk.metrics.export import (
InMemoryMetricReader,
)
from opentelemetry.util.types import Attributes


@pytest.fixture
def attributes() -> Attributes:
return {
"key": "value",
}


@pytest.fixture
def meter_name() -> str:
return "test_meter"


@pytest.fixture
def reader() -> InMemoryMetricReader:
return InMemoryMetricReader()


@pytest.fixture
def meter_provider(reader: InMemoryMetricReader) -> MeterProvider:
return MeterProvider(metric_readers=[reader])


@pytest.fixture
def meter(meter_provider: MeterProvider, meter_name: str) -> Meter:
return meter_provider.get_meter("test_meter")


def test_measurement_collection(
reader: InMemoryMetricReader,
meter: Meter,
attributes: Attributes,
) -> None:
"""
Validate that adjusting attributes after a data point is created does not affect
the already collected measurement.
"""
counter = meter.create_counter("test_counter")
counter.add(1, attributes)
attributes["key"] = "new_value"
counter.add(1, attributes)

reader.collect()

metrics_data = reader.get_metrics_data()
resource_metric, *_ = metrics_data.resource_metrics
scope_metric, *_ = resource_metric.scope_metrics
metrics, *_ = scope_metric.metrics
data = metrics.data
data_point_1, data_point_2 = data.data_points

assert data_point_1.attributes == {"key": "value"}
assert data_point_2.attributes == {"key": "new_value"}
67 changes: 67 additions & 0 deletions opentelemetry-sdk/tests/metrics/test_measurement.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
from time import time_ns
from unittest.mock import Mock

import pytest

from opentelemetry.context import Context
from opentelemetry.metrics import Instrument
from opentelemetry.sdk.metrics._internal.measurement import (
Measurement,
)
from opentelemetry.util.types import Attributes


@pytest.fixture
def attributes() -> Attributes:
return {
"key": "value",
}


@pytest.fixture
def unix_time() -> int:
return time_ns()
Copy link
Member

Choose a reason for hiding this comment

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

Can you hardcode a timestamp here just for repeatability?



@pytest.fixture
def context() -> Context:
return Context()


@pytest.fixture
def instrument():
return Mock(spec=Instrument)


@pytest.fixture
def measurement(
unix_time: int,
instrument: Instrument,
context: Context,
attributes: Attributes,
) -> Measurement:
return Measurement(
value=1.0,
time_unix_nano=unix_time,
instrument=instrument,
context=context,
attributes=attributes,
)


def test_measurement_attribute_is_a_different_object(
measurement: Measurement,
attributes: Attributes,
):
assert measurement.attributes is not attributes


def test_measurement_attribute_uneffected_by_change(
measurement: Measurement,
attributes: Attributes,
) -> None:
attributes["new_key"] = "new_value"

assert measurement.attributes == {
"key": "value",
}
Loading