Skip to content

opentelemetry-sdk: remove copy on span instantiation#5272

Merged
lzchen merged 2 commits into
open-telemetry:mainfrom
codeboten:codeboten/remove-copy
Jun 4, 2026
Merged

opentelemetry-sdk: remove copy on span instantiation#5272
lzchen merged 2 commits into
open-telemetry:mainfrom
codeboten:codeboten/remove-copy

Conversation

@codeboten

@codeboten codeboten commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Description

The copy isn't necessary as the init method of the BoundedAttributes (inside the span's init method) already iterates through and copies all the attributes in a new Dict.

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 some benchmarks before and after the change with:

pytest opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py::test_start_span_with_attributes \
      --memray --trace-python-allocators --benchmark-disable -v

The following table shows the before and after change savings:

attrs Before (KiB) After (KiB) Saved (KiB)
0 7.5 7.3 0.2
1 8.3 8.1 0.2
10 11.3 11.3 0.0
50 15.8 14.3 1.5
128 28.0 24.8 3.2

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

This copy isn't necessary as the init method of the BoundedAttributes (inside the span's init method) already iterates through and copies all the attributes in a new Dict. Ran some benchmarks before and after the change with:

```
pytest opentelemetry-sdk/benchmarks/trace/test_benchmark_trace.py::test_start_span_with_attributes \
  --memray --trace-python-allocators --benchmark-disable -v
```

The following table shows the before and after change savings:

| attrs | Before (KiB) | After (KiB) | Saved (KiB) |
|------:|-------------:|------------:|------------:|
| 0     | 7.5          | 7.3         | 0.2         |
| 1     | 8.3          | 8.1         | 0.2         |
| 10    | 11.3         | 11.3        | 0.0         |
| 50    | 15.8         | 14.3        | 1.5         |
| 128   | 28.0         | 24.8        | 3.2         |

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten codeboten requested a review from a team as a code owner June 3, 2026 23:09
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>

@tammy-baylis-swi tammy-baylis-swi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Thank you

@github-project-automation github-project-automation Bot moved this to Approved PRs in Python PR digest Jun 4, 2026
@MikeGoldsmith MikeGoldsmith moved this from Approved PRs to Ready for merge in Python PR digest Jun 4, 2026
@lzchen lzchen added this pull request to the merge queue Jun 4, 2026
@github-project-automation github-project-automation Bot moved this from Ready for merge to Approved PRs in Python PR digest Jun 4, 2026
Merged via the queue into open-telemetry:main with commit effa3dd Jun 4, 2026
479 checks passed
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Python PR digest Jun 4, 2026
@codeboten codeboten deleted the codeboten/remove-copy branch June 4, 2026 17:08
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.

5 participants