Skip to content

[API] Avoid SpanAttributes array enumeration - #7698

Merged
martincostello merged 4 commits into
open-telemetry:mainfrom
thomhurst:perf/span-attributes-collection-fast-paths
Sep 8, 2026
Merged

martincostello merged 4 commits into
open-telemetry:mainfrom
thomhurst:perf/span-attributes-collection-fast-paths

Conversation

@thomhurst

@thomhurst thomhurst commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Changes

Use ReadOnlySpan<T> enumeration when the existing SpanAttributes
IEnumerable<T> constructor receives an array. Arbitrary enumerable inputs
retain the existing path, and public API remains unchanged.

An IReadOnlyList<T> indexed path was also tested. It had no allocation benefit
for List<T> on .NET 10 and mixed timing results, so it is not included.

Benchmark

A task-scoped BenchmarkDotNet benchmark constructed SpanAttributes from a
pre-created array containing 1, 5, or 20 attributes. It was run before and after
this change using BenchmarkDotNet 0.15.8 on .NET 10.0.11, Windows 11, Intel Core
i7-12700K.

Items Before After Change Allocated before Allocated after
1 25.12 ns 22.84 ns -9.1% 200 B 168 B
5 89.52 ns 65.42 ns -26.9% 352 B 320 B
20 413.17 ns 407.74 ns -1.3% 1,168 B 1,136 B

The array path removes 32 B per construction at every tested size.

Benchmark method:

[Benchmark]
public SpanAttributes FromArray()
    => new(this.array);

array was initialized in GlobalSetup; [MemoryDiagnoser] was enabled and
ItemCount used [Params(1, 5, 20)].

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • All 6 SpanAttributesTests pass
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • No public API changes

@thomhurst
thomhurst requested a review from a team as a code owner August 25, 2026 19:42
@github-actions

Copy link
Copy Markdown
Contributor

Welcome, contributor! Thank you for your contribution to opentelemetry-dotnet.

Important reminders:

  • Read our Contributing Guidelines.
  • Sign the CLA if you haven't already.
  • Follow the OpenTelemetry Generative AI policy: disclose any AI use in your contribution, and communicate (PR descriptions, review replies) in your own words rather than AI-generated text.
  • Give reviewers at least a few days before pinging them for feedback.
  • If you need help with general setup, development process, or contributor etiquette, ask in #opentelemetry-new-contributors.

@github-actions github-actions Bot added the pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package label Aug 25, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-09-08 10:14 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@martincostello martincostello changed the title [OpenTelemetry.Api] Avoid SpanAttributes array enumeration [API] Avoid SpanAttributes array enumeration Aug 26, 2026
@codecov

codecov Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.68%. Comparing base (a2b9672) to head (4fd439b).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7698      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         333      333              
  Lines       18287    18290       +3     
==========================================
+ Hits        16769    16770       +1     
- Misses       1518     1520       +2     
Flag Coverage Δ
unittests-Project-Experimental 91.70% <100.00%> (-0.10%) ⬇️
unittests-Project-Stable 91.79% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/OpenTelemetry.Api/Trace/SpanAttributes.cs 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@martincostello

Copy link
Copy Markdown
Member

Could you add/update some tests for the missing patch coverage please?

@opentelemetry-pr-dashboard

This comment has been minimized.

Add coverage for `KeyValuePair<string, object?>[]` being passed to the constructor.
@thomhurst

Copy link
Copy Markdown
Contributor Author

Woops sorry @martincostello I missed these notifications. Looks like you're on it?

@martincostello
martincostello added this pull request to the merge queue Sep 8, 2026
Merged via the queue into open-telemetry:main with commit 4bb4720 Sep 8, 2026
150 of 152 checks passed
@otelbot

otelbot Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution @thomhurst! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

This was referenced Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants