Skip to content

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Aug 3, 2025

I am looking into I am looking into https://promlabs.com/blog/2025/07/17/why-i-recommend-native-prometheus-instrumentation-over-opentelemetry/#comparing-counter-increment-performance, and was trying to figure out why incrementing a counter with 10 attributes was so much slower than incrementing a counter with no attributes, or 1 attribute:

$ go test -run=xxxxxMatchNothingxxxxx -cpu=1 -test.benchtime=1s -bench=BenchmarkSyncMeasure/NoView/Int64Counter/Attributes
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/metric
cpu: Intel(R) Xeon(R) CPU @ 2.20GHz
BenchmarkSyncMeasure/NoView/Int64Counter/Attributes/0         	 9905773	       121.3 ns/op
BenchmarkSyncMeasure/NoView/Int64Counter/Attributes/1         	 4079145	       296.5 ns/op
BenchmarkSyncMeasure/NoView/Int64Counter/Attributes/10        	  781627	      1531 ns/op

Looking at the profile, most of the time is spent in "runtime.mapKeyError2" within "runtime.mapaccess2". My best guess is that whatever we are using for Equivalent() is not very performant when used as a map key. This seems like a good opportunity to greatly improve the performance of our metrics (and probably other signals) API + SDK.

To start, i'm adding a simple benchmark within the attribute package to isolate the issue. Results:

$ go test     -run '^$' -bench '^BenchmarkEquivalentMapAccess'     -benchtime .1s -cpu 1 -benchmem 
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/attribute
cpu: Intel(R) Xeon(R) CPU @ 2.20GHz
BenchmarkEquivalentMapAccess/Empty         	 2220508	        53.58 ns/op	       0 B/op	       0 allocs/op
BenchmarkEquivalentMapAccess/1_string_attribute         	  622770	       196.7 ns/op	       0 B/op	       0 allocs/op
BenchmarkEquivalentMapAccess/10_string_attributes       	   77462	      1558 ns/op	       0 B/op	       0 allocs/op
BenchmarkEquivalentMapAccess/1_int_attribute            	  602163	       197.7 ns/op	       0 B/op	       0 allocs/op
BenchmarkEquivalentMapAccess/10_int_attributes          	   76603	      1569 ns/op	       0 B/op	       0 allocs/op

This shows that it is the map lookup and storage itself that is making the metrics API+SDK perform much worse with more attributes.

Some optimization ideas include:

@dashpole dashpole added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Aug 3, 2025
@MrAlias
Copy link
Contributor

MrAlias commented Aug 4, 2025

@dashpole see #5028

dashpole and others added 3 commits August 6, 2025 08:50
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Flc゛ <four_leaf_clover@foxmail.com>
Copy link

codecov bot commented Aug 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.9%. Comparing base (49033aa) to head (3ebd0f3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #7123   +/-   ##
=====================================
  Coverage   82.9%   82.9%           
=====================================
  Files        262     262           
  Lines      24461   24461           
=====================================
+ Hits       20291   20294    +3     
+ Misses      3794    3791    -3     
  Partials     376     376           

see 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dashpole dashpole merged commit eb4f1dc into open-telemetry:main Aug 6, 2025
31 checks passed
@dashpole dashpole deleted the attribute_equivalent_bench branch August 6, 2025 18:45
@pellared pellared removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label Aug 14, 2025
@MrAlias MrAlias added this to the v1.38.0 milestone Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants