Skip to content

opentelemetry-sdk: inline the method _clean_attribute_value#5275

Open
codeboten wants to merge 7 commits into
open-telemetry:mainfrom
codeboten:codeboten/inline-_clean_attribute_value
Open

opentelemetry-sdk: inline the method _clean_attribute_value#5275
codeboten wants to merge 7 commits into
open-telemetry:mainfrom
codeboten:codeboten/inline-_clean_attribute_value

Conversation

@codeboten

@codeboten codeboten commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Description

This removes an if statement to check if the value is None and embeds the method inline to remove the need for that check. A second check for None is removed in the loop through the Sequence type.

Note to reviewers: I bumped the number of spans in the benchmark test to 5000 to get a more consistent result, it doesn't have to stay here, let me know if you'd like it removed.

Follow up to #5274, can rebase after it's merged (if it merges) or without it if reviewers would prefer.

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 the following:

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

And got these results for the different types:

type Before (ms) After (ms) Δ
bool 204.1 197.3 -3%
str 211.9 203.9 -4%
int 221.3 213.5 -4%
bytes 226.5 216.8 -4%
float 227.3 222.1 -2%
seq_bool 488.3 453.7 -7%
seq_bytes 513.7 471.7 -8%
seq_float 539.6 508.4 -6%
seq_str 594.3 537.0 -10%
seq_int 636.0 576.3 -9%

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

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

@github-project-automation github-project-automation Bot moved this to Approved PRs in Python PR digest Jun 5, 2026
codeboten added 2 commits June 8, 2026 10:06
This removes an if statement to check if the value is None and embeds the method inline to remove the need for that check. This removes a second check for None in the loop through the Sequence type.

Ran the following:

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

And got these results for the different types:

| type      | Before (ms) | After (ms) | Δ     |
|-----------|------------|-----------|------|
| bool      |       204.1 |      197.3 |  -3%  |
| str       |       211.9 |      203.9 |  -4%  |
| int       |       221.3 |      213.5 |  -4%  |
| bytes     |       226.5 |      216.8 |  -4%  |
| float     |       227.3 |      222.1 |  -2%  |
| seq_bool  |       488.3 |      453.7 |  -7%  |
| seq_bytes |       513.7 |      471.7 |  -8%  |
| seq_float |       539.6 |      508.4 |  -6%  |
| seq_str   |       594.3 |      537.0 | -10%  |
| seq_int   |       636.0 |      576.3 |  -9%  |

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten codeboten force-pushed the codeboten/inline-_clean_attribute_value branch from d9d6219 to 829474b Compare June 8, 2026 17:06
@codeboten codeboten marked this pull request as ready for review June 8, 2026 17:06
@codeboten codeboten requested a review from a team as a code owner June 8, 2026 17:06
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@herin049

herin049 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

If we end up using inlining more often, it might be worth writing a custom Hatchling build hook to allow us to just write

@inline
def _clean_attribute_value(
    value: types.AttributeValue, limit: int | None
) -> types.AttributeValue | None:
    if value is None:
        return None

    if isinstance(value, bytes):
        try:
            value = value.decode()
        except UnicodeDecodeError:
            _logger.warning("Byte attribute could not be decoded.")
            return None

    if limit is not None and isinstance(value, str):
        value = value[:limit]
    return value

codeboten added 4 commits June 8, 2026 12:29
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved PRs

Development

Successfully merging this pull request may close these issues.

3 participants