internal/shared/semconv: Reduce allocations

[gaiaz-iusipov]

$ go run golang.org/x/perf/cmd/benchstat@latest old.txt new.txt
goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp
cpu: Apple M3 Max
              │   old.txt   │              new.txt               │
              │   sec/op    │   sec/op     vs base               │
_RoundTrip-14   2.099µ ± 1%   2.026µ ± 4%  -3.48% (p=0.018 n=10)

              │   old.txt    │               new.txt               │
              │     B/op     │     B/op      vs base               │
_RoundTrip-14   8.442Ki ± 0%   7.963Ki ± 0%  -5.68% (p=0.000 n=10)

              │  old.txt   │              new.txt               │
              │ allocs/op  │ allocs/op   vs base                │
_RoundTrip-14   39.00 ± 0%   35.00 ± 0%  -10.26% (p=0.000 n=10)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.2%. Comparing base (e81a2b4) to head (f71ecbc).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #8511   +/-   ##
=====================================
  Coverage   82.2%   82.2%           
=====================================
  Files        179     179           
  Lines      13700   13724   +24     
=====================================
+ Hits       11265   11289   +24     
  Misses      2033    2033           
  Partials     402     402           

Files with missing lines	Coverage Δ
.../go-restful/otelrestful/internal/semconv/client.go	86.8% <100.0%> (+0.3%)	⬆️
.../go-restful/otelrestful/internal/semconv/server.go	81.7% <100.0%> (ø)
...m/gin-gonic/gin/otelgin/internal/semconv/client.go	86.8% <100.0%> (+0.3%)	⬆️
...m/gin-gonic/gin/otelgin/internal/semconv/server.go	81.7% <100.0%> (ø)
...com/gorilla/mux/otelmux/internal/semconv/client.go	86.8% <100.0%> (+0.3%)	⬆️
...com/gorilla/mux/otelmux/internal/semconv/server.go	81.7% <100.0%> (ø)
.../labstack/echo/otelecho/internal/semconv/client.go	86.8% <100.0%> (+0.3%)	⬆️
.../labstack/echo/otelecho/internal/semconv/server.go	81.7% <100.0%> (ø)
...httptrace/otelhttptrace/internal/semconv/client.go	86.8% <100.0%> (+0.3%)	⬆️
...httptrace/otelhttptrace/internal/semconv/server.go	81.7% <100.0%> (ø)
... and 2 more

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dmathieu]

We have removed the duplicate metrics. So I think we could remove even more allocations by removing the map[string]MetricOpts and replacing it with a MetricOpts.

[gaiaz-iusipov]

We have removed the duplicate metrics. So I think we could remove even more allocations by removing the map[string]MetricOpts and replacing it with a MetricOpts.

Great. I didn’t initially make that change because it broke the function signature, but this option is, of course, much more preferable.

[dmathieu]

We can change internal module's method signature, as long as we don't change the public ones.

[flc1125]

Could you add some before-and-after performance comparison data? We can use benchstat to provide it.

[gaiaz-iusipov]

If we compare the variants where the slice is allocated only once, then each call results in exactly one fewer allocation. I decided not to account for removing the map wrapper in the benchmark, I think that part is fairly obvious:

func (n HTTPClient) RecordMetricsOld(ctx context.Context, md MetricData, opts MetricOpts) {
	n.requestBodySize.Inst().Record(ctx, md.RequestSize, opts.MeasurementOption())
	n.requestDuration.Inst().Record(ctx, md.ElapsedTime/1000, opts.MeasurementOption())
}

func (n HTTPClient) RecordMetricsNew(ctx context.Context, md MetricData, opts MetricOpts) {
	o := []metric.RecordOption{opts.MeasurementOption()}
	n.requestBodySize.Inst().Record(ctx, md.RequestSize, o...)
	n.requestDuration.Inst().Record(ctx, md.ElapsedTime, o...)
}

Here are the results:

go test . -bench=^Benchmark_RecordMetrics$ -run=^$ -benchmem -benchtime=10s
Benchmark_RecordMetrics/old-14         	444416276	        26.78 ns/op	      32 B/op	       2 allocs/op
Benchmark_RecordMetrics/new-14         	771929202	        15.49 ns/op	      16 B/op	       1 allocs/op

[flc1125]

In addition, please fix the CI. We can check it locally by running make ci.

[pellared]

Can you please add results from benchstat to the PR description?