Counter measurement attributes aren't immutable

[P4rk]

Describe your environment

OS: macOS Sequoia 15.4.1 (24E263) Python version: 3.12.8 SDK version: 1.33.1 API version: 1.33.1

What happened?

Attributes for a datapoint are mutable after the datapoint has been recorded. I discovered this using counters, I assume it affects all instruments, though.

If a reference to the mapping of the attributes passed into counter.add is kept, the datapoints attributes created by calling add can be changed.

Steps to Reproduce

from opentelemetry.sdk.metrics import MeterProvider
from opentelemetry.sdk.metrics.export import (
    ConsoleMetricExporter,
    PeriodicExportingMetricReader,
)

reader = PeriodicExportingMetricReader(ConsoleMetricExporter())
provider = MeterProvider(metric_readers=[reader])
meter = provider.get_meter("my-meter")

counter = meter.create_counter("my_counter")

# create a reference to the attributes
attributes = {"att": "value"}
counter.add(1, attributes)

# adjust the attributes after the first increment
attributes["att"] = "value2"
counter.add(1, attributes)

provider.force_flush()
provider.shutdown()

Expected Result

counter.add creates an immutable copy of the attributes used in recording the metrics. expected datapoints, the attributes are different for each datapoint:

"data_points": [
    {
        "attributes": {
            "att": "value"
        },
        ...
        "value": 1,
        ...
    },
    {
        "attributes": {
            "att": "value2"
        },
        ...
        "value": 1,
        ...
    }
]

Actual Result

counter.add references the attribute object passed in without recording its value at the time of incrementation. resulting in these datapoints, the attribute values are the same for each datapoint:

"data_points": [
    {
        "attributes": {
            "att": "value2"
        },
        ...
        "value": 1,
        ...
    },
    {
        "attributes": {
            "att": "value2"
        },
        ...
        "value": 1,
        ...
    }
]

Additional context

Downstream clients can fix this by passing in copies of their objects as a workaround. However, it was not obvious to me what was happening. My previous experience using Prometheus libraries suggests that holding a reference to the attributes object is ok.

It's also likely that I have misunderstood or misconfigured something. Please, feel free to point me in the correct direction if that is the case.

Please let me know if you agree if this is something that needs fixing and I'll see if I can submit a pull request.

Would you like to implement a fix?

Yes

[aabmass]

Thanks for the clear repro. It is a little surprising to me as well, I would expect this case to work. The reason you see two separate data points is the SDK correctly creates an immutable key, but it just lazily stores a reference to the attributes for encoding later.

https://github.com/open-telemetry/opentelemetry-python/blob/36ac612f263f224665865e2ecc692f3c32a66a23/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py#L105 https://github.com/open-telemetry/opentelemetry-python/blob/36ac612f263f224665865e2ecc692f3c32a66a23/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py#L132-L134

~However this seems buggy since it takes a shallow copy and attributes may contain a list~ nvm frozenset would just fail here which is a separate bug.

IMO we should fix this

[aabmass]

Would you like to implement a fix?

Yes

Would be very appreciated! We don't assign issues generally but please drop a comment if you start working on this.

[P4rk]

@aabmass I have taken a swing at the fix here (draft): https://github.com/open-telemetry/opentelemetry-python/pull/4627. Let me know what you think or if things need changing.

[aabmass]

Thanks! The approach looks good, assuming there are no unintended consequences of deepcopy 👍

[P4rk]

@aabmass The PR is ready for review now. I assume it'll be picked up in some process by the approvers/maintainers?

re:deepcopy - I really don't foresee any issues with it, IMO any speed or memory effects would be negligible as the Attributes type can't be nested.

All and any feedback is more than welcome.