Skip to content

opentelemetry-sdk: remove unnecessary dict in set_attribute method#5274

Merged
lzchen merged 3 commits into
open-telemetry:mainfrom
codeboten:codeboten/rm-set-attribute-dict-alloc
Jun 8, 2026
Merged

opentelemetry-sdk: remove unnecessary dict in set_attribute method#5274
lzchen merged 3 commits into
open-telemetry:mainfrom
codeboten:codeboten/rm-set-attribute-dict-alloc

Conversation

@codeboten

@codeboten codeboten commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Description

set_attribute was unnecessarily creating a dict to set the attribute via set_attributes. removed that call and instead put the code from set_attributes directly into set_attribute. there is a small amount of time saved from doing this, the memory footprint is essentially the same:

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Ran:

pytest opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py::test_set_attribute \
  --memray --trace-python-allocators \
  --benchmark-min-rounds=10 --benchmark-columns=mean,median,ops

And got the following results:

attrs Before mean (µs) After mean (µs) Time saved Before mem (KiB) After mem (KiB) Mem saved
1 22.7 21.6 5% 176.8 174.6 1.2
10 39.9 34.4 14% 356.0 408.9 -52.9
50 117.6 90.4 23% 190.9 240.1 -49.2
128 266.3 199.3 25% 115.3 139.9 -24.6

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Benchmark tests have been added
  • Documentation has been updated

set_attribute was unnecessarily creating a dict to set the attribute via set_attributes. removed that call and instead put the code from set_attributes directly into set_attribute. there is a small amount of time saved from doing this, the memory footprint is essentially the same:

Ran:

```
pytest opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py::test_set_attribute \
  --memray --trace-python-allocators \
  --benchmark-min-rounds=10 --benchmark-columns=mean,median,ops
```

And got the following results:

| attrs | Before mean (µs) | After mean (µs) | Time saved | Before mem (KiB) | After mem (KiB) | Mem saved |
|------|-----------------|----------------|-----------|-----------------|----------------|----------|
|     1 |            22.7  |            21.6 |        5%  |            176.8 |           174.6 |      1.2  |
|    10 |            39.9  |            34.4 |       14%  |            356.0 |           408.9 |     -52.9 |
|    50 |           117.6  |            90.4 |       23%  |            190.9 |           240.1 |     -49.2 |
|   128 |           266.3  |           199.3 |       25%  |            115.3 |           139.9 |     -24.6 |

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten codeboten requested a review from a team as a code owner June 4, 2026 17:40
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten codeboten changed the title opentelemetry-sdk: remove unnecessary dict in method opentelemetry-sdk: remove unnecessary dict in set_attribute method Jun 4, 2026
@tammy-baylis-swi tammy-baylis-swi moved this to Ready for review in Python PR digest Jun 4, 2026

@MikeGoldsmith MikeGoldsmith left a comment

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.

I like the change. I wasn't expecting to see memory usage to grow inline with more attributes though. Any thoughts @codeboten?

@github-project-automation github-project-automation Bot moved this from Ready for review to Approved PRs in Python PR digest Jun 5, 2026
@codeboten

Copy link
Copy Markdown
Contributor Author

I like the change. I wasn't expecting to see memory usage to grow inline with more attributes though. Any thoughts @codeboten?

The memory usage recorded was the result of the benchmarks running more rounds through the test (because of the time savings 🤦🏻 )

re-ran it with an exact number of rounds and the memory is essentially the same:

attrs Before mem (KiB) After mem (KiB) Before mean (µs) After mean (µs) Time Δ
1 17.1 17.0 24.6 21.9 -11%
10 10.1 10.1 39.5 33.6 -15%
50 16.2 16.2 119.1 95.3 -20%
128 27.5 27.2 263.0 196.0 -25%

@lzchen lzchen added this pull request to the merge queue Jun 8, 2026
Merged via the queue into open-telemetry:main with commit 60b6749 Jun 8, 2026
479 checks passed
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Python PR digest Jun 8, 2026
@codeboten codeboten deleted the codeboten/rm-set-attribute-dict-alloc branch June 8, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants