[OTLP] Cache resource bytes for protobuf serializer by unsafePtr · Pull Request #7303 · open-telemetry/opentelemetry-dotnet

unsafePtr · 2026-05-17T19:05:49Z

Changes

Caches bytes for the protobuf serializer for the Resource, since Resource is immutable

Merge requirement checklist

CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
Unit tests added/updated
Appropriate CHANGELOG.md files updated for non-trivial changes
Changes in public API reviewed (if applicable)

unsafePtr · 2026-05-17T19:09:56Z

Benchmarks

Before


BenchmarkDotNet v0.15.8, Windows 11 (10.0.26200.8457/25H2/2025Update/HudsonValley2)
13th Gen Intel Core i7-13700KF 3.40GHz, 1 CPU, 24 logical and 16 physical cores
.NET SDK 10.0.300
  [Host]     : .NET 10.0.8 (10.0.8, 10.0.826.23019), X64 RyuJIT x86-64-v3
  DefaultJob : .NET 10.0.8 (10.0.8, 10.0.826.23019), X64 RyuJIT x86-64-v3

Method	Shape	Mean	Error	StdDev	Median	Allocated
WriteResource	Empty	1.114 ns	0.0032 ns	0.0026 ns	1.114 ns	-
WriteResource	Default	74.061 ns	1.4604 ns	3.7958 ns	72.160 ns	-
WriteResource	Service	174.957 ns	0.5002 ns	0.4679 ns	174.861 ns	-
WriteResource	Production	547.260 ns	2.5829 ns	2.4161 ns	547.481 ns	-

After


BenchmarkDotNet v0.15.8, Windows 11 (10.0.26200.8457/25H2/2025Update/HudsonValley2)
13th Gen Intel Core i7-13700KF 3.40GHz, 1 CPU, 24 logical and 16 physical cores
.NET SDK 10.0.300
  [Host]     : .NET 10.0.8 (10.0.8, 10.0.826.23019), X64 RyuJIT x86-64-v3
  DefaultJob : .NET 10.0.8 (10.0.8, 10.0.826.23019), X64 RyuJIT x86-64-v3

Method	Shape	Mean	Error	StdDev	Allocated
WriteResource	Empty	4.191 ns	0.0034 ns	0.0032 ns	-
WriteResource	Default	5.013 ns	0.0171 ns	0.0160 ns	-
WriteResource	Service	6.254 ns	0.0399 ns	0.0373 ns	-
WriteResource	Production	12.228 ns	0.0825 ns	0.0772 ns	-

codecov · 2026-05-17T19:17:30Z

Codecov Report

❌ Patch coverage is 75.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.85%. Comparing base (2435127) to head (7f4816d).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ation/Serializer/ProtobufOtlpResourceSerializer.cs	75.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7303      +/-   ##
==========================================
+ Coverage   89.82%   89.85%   +0.02%     
==========================================
  Files         275      275              
  Lines       13834    13852      +18     
==========================================
+ Hits        12427    12447      +20     
+ Misses       1407     1405       -2

Flag	Coverage Δ
unittests-Project-Experimental	`89.59% <75.00%> (-0.11%)`	⬇️
unittests-Project-Stable	`89.84% <75.00%> (+0.06%)`	⬆️

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

Files with missing lines	Coverage Δ
...ation/Serializer/ProtobufOtlpResourceSerializer.cs	`85.71% <75.00%> (-5.96%)`	⬇️

... and 4 files with indirect coverage changes

unsafePtr · 2026-05-17T22:07:26Z

@martincostello , theoretically same optimization can be added within Metric, as it contains some metadata which is static anyways. Win there will be way smaller (only Name/Description/Unit are static). Would you mind attaching copilot to work on this, after this PR is closed?

https://github.com/open-telemetry/opentelemetry-dotnet/blob/b0d5e7989dd0016bb993178ed775ea3ae155aa0d/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/Serializer/ProtobufOtlpMetricSerializer.cs

martincostello · 2026-05-18T13:11:06Z

I'm sure I left a comment on my last review but I can't see it now.

Could you re-run the new benchmarks with the latest changes and share the results for main and this PR?

…lp-resource # Conflicts: # src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md

unsafePtr · 2026-05-18T13:54:52Z

After merging main and addressing the latest review feedback, here are updated benchmark numbers. The benchmark was also reparameterized by AttributeCount (per @martincostello's suggestion).


BenchmarkDotNet v0.15.8, Windows 11 (10.0.26200.8457/25H2/2025Update/HudsonValley2)
13th Gen Intel Core i7-13700KF 3.40GHz, 1 CPU, 24 logical and 16 physical cores
.NET SDK 10.0.300
  [Host]     : .NET 10.0.8 (10.0.8, 10.0.826.23019), X64 RyuJIT x86-64-v3
  DefaultJob : .NET 10.0.8 (10.0.8, 10.0.826.23019), X64 RyuJIT x86-64-v3

Method	AttributeCount	Mean	Error	StdDev	Median	Allocated
WriteResource	0	0.7169 ns	0.0492 ns	0.0622 ns	0.7010 ns	-
WriteResource	4	11.1713 ns	0.0897 ns	0.0700 ns	11.1659 ns	-
WriteResource	8	14.0683 ns	0.3087 ns	0.3431 ns	14.3080 ns	-
WriteResource	16	17.5844 ns	0.1248 ns	0.1106 ns	17.5339 ns	-
WriteResource	32	28.0397 ns	0.0774 ns	0.0686 ns	28.0134 ns	-

Notes:

Empty case (AttributeCount = 0) now goes through the ReadOnlySpan<byte> inline-bypass path and is actually slightly faster than the pre-cache baseline (~0.7 ns vs ~1.1 ns), since Span.CopyTo of 5 bytes is cheaper than the original WriteTag + WriteReservedLength calls.
Non-empty cases scale roughly linearly with N (cache hit + single Buffer.BlockCopy).
Zero allocations across the board — the original serializer was already alloc-free; this PR cuts CPU work, not memory.

martincostello · 2026-05-18T14:03:20Z

Would you mind attaching copilot to work on this, after this PR is closed?

It sounds reasonable to make the same change for metrics. It's probably overall easiest if you do the PR (or ask Copilot to do it locally) as then I can review and merge the PR for you.

unsafePtr · 2026-05-18T14:33:24Z

Would you mind attaching copilot to work on this, after this PR is closed?

It sounds reasonable to make the same change for metrics. It's probably overall easiest if you do the PR (or ask Copilot to do it locally) as then I can review and merge the PR for you.

I thought Microsoft had an infinite amount of tokens. That's why I asked whether a dedicated Copilot could be assigned to work on it, since it would already have an example of exactly what to change 🙂

I am also working on some improvements in the dotnet/runtime telemetry area. It really puzzles me that IEnumerable<Measurement<T>> allocates in *Obseravble types. Maybe I am perfectionist, but I would expect such an API to be zero-alloc.

dotnet/runtime#128039 (comment)

martincostello · 2026-05-18T14:42:22Z

I thought Microsoft had an infinite amount of tokens

That's not really relevant here - this isn't a Microsoft project, and I'm not a Microsoft employee.

I could assign it to use my personal Copilot access, but then it counts as written by me so I can't review/merge it and we're tight on review bandwidth here. If you submitted a PR I can take a look at then it would potentially move faster.

If you're too busy with other things that's fine, I can just pick it up in the next day or so.

github-actions · 2026-05-18T14:56:44Z

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

cijothomas · 2026-05-18T16:45:08Z

    private const int ReserveSizeForLength = 4;
+    private const int InitialBufferSize = 2048;
+
+    private static readonly ConcurrentDictionary<Resource, byte[]> CachedResourceBytes = new();


unsure if static is the right choice here. if we store it in MeterProvider or exporter itself, then we can avoid the Dictionary completely as there is single Resource associated with an exporter always.

So you mean move the cache up one level to the caller?

That sounds similar to something I've done in #7279 to cache Prometheus metrics.

@cijothomas , Resource is immutable, isn't it? static dictionary is the right choice here.

Immutability of Resource isn't really the concern — the issue is lifetime. A static ConcurrentDictionary<Resource, byte[]> strongly references every Resource instance the process ever sees, including ones from disposed MeterProviders. For Resource it's usually bounded in practice so the leak is small, but caching on the exporter/provider (where there's a single Resource per exporter) would avoid the dictionary entirely and bound the lifetime correctly.

I thought about it, but the changes surface was quite big, plus same bytes would be effectively duplicated across 3 exporters. While static dictionary is a simple change and I thought that Resource reference is typically living for a lifetime of the application. Several dangling references for a long-running processes, is usually acceptable. Still it's quite easy to swap ConcurrentDicitonary with ConditionalWeakTable here with loosing a bit on perf if dangling references are problem.

Fair. This one has low risk, but it'd be still better to switch to ConditionalWeakTable here in a follow up PR. I am more worried about the Metric/scope change PR (#7307), as that has more real leak risks.

cijothomas · 2026-05-18T16:45:37Z

https://github.com/open-telemetry/opentelemetry-dotnet/pull/7303/changes#r3260565691

Already merged, but I'd suggest to reconsider.

martincostello · 2026-05-20T11:40:04Z

Have opened #7326 as a follow-up for discussion.

feat: pre-cache resource bytes for protobuf oltp serializer

fc4c2e7

unsafePtr requested a review from a team as a code owner May 17, 2026 19:05

github-actions Bot added pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package perf Performance related labels May 17, 2026

docs: add changelog entry for resource bytes cache

7329d50

martincostello reviewed May 17, 2026

View reviewed changes

unsafePtr added 6 commits May 17, 2026 22:44

refactor: cache ArrayPool<byte>.Shared into a local

20db64f

perf: write empty-resource bytes inline via ReadOnlySpan literal

b34f252

perf: lower initial scratch size to 2 KB

48b546c

test: parameterize resource benchmark by attribute count

e80ce52

docs: move changelog entry to end of unreleased section

dd5e2d2

fix: avoid range syntax in test for net462 compatibility

f6e12d6

martincostello reviewed May 18, 2026

View reviewed changes

martincostello changed the title ~~Cache resource bytes for protobuf oltp serializer~~ [OTLP] Cache resource bytes for protobuf serializer May 18, 2026

refactor: address review nits — rename locals and tighten test buffer

4f1d8a5

Merge remote-tracking branch 'origin/main' into perf/pre-serialize-ot…

7f4816d

…lp-resource # Conflicts: # src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md

martincostello approved these changes May 18, 2026

View reviewed changes

martincostello added this pull request to the merge queue May 18, 2026

Merged via the queue into open-telemetry:main with commit ed44995 May 18, 2026
60 checks passed

unsafePtr mentioned this pull request May 18, 2026

[OTLP] Cache metric metadata bytes for protobuf serializer #7307

Open

4 tasks

cijothomas reviewed May 18, 2026

View reviewed changes

martincostello mentioned this pull request May 20, 2026

[OTLP] Use ConditionalWeakTable for cache #7326

Merged

4 tasks

rajkumar-rangaraj added a commit to martincostello/opentelemetry-dotnet that referenced this pull request May 20, 2026

Merge branch 'main' into open-telemetrygh-7303-follow-up

c3e06d3

martincostello added a commit to martincostello/opentelemetry-dotnet that referenced this pull request May 21, 2026

Merge branch 'main' into open-telemetrygh-7303-follow-up

5c20c87

Conversation

unsafePtr commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Merge requirement checklist

Uh oh!

unsafePtr commented May 17, 2026

Uh oh!

codecov Bot commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

unsafePtr commented May 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

martincostello commented May 18, 2026

Uh oh!

unsafePtr commented May 18, 2026

Uh oh!

martincostello commented May 18, 2026

Uh oh!

unsafePtr commented May 18, 2026

Uh oh!

martincostello commented May 18, 2026

Uh oh!

Uh oh!

github-actions Bot commented May 18, 2026

Uh oh!

cijothomas May 18, 2026

Choose a reason for hiding this comment

Uh oh!

martincostello May 18, 2026

Choose a reason for hiding this comment

Uh oh!

unsafePtr May 18, 2026

Choose a reason for hiding this comment

Uh oh!

cijothomas May 18, 2026

Choose a reason for hiding this comment

Uh oh!

unsafePtr May 18, 2026

Choose a reason for hiding this comment

Uh oh!

cijothomas May 18, 2026

Choose a reason for hiding this comment

Uh oh!

cijothomas commented May 18, 2026

Uh oh!

martincostello commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

unsafePtr commented May 17, 2026 •

edited

Loading

codecov Bot commented May 17, 2026 •

edited

Loading