improve performance for attributes http/dup

[boekkooi-impossiblecloud]

Good day,

While turning using OTEL_SEMCONV_STABILITY_OPT_IN=http/dup we noticed an increase of object allocations.

This PR proposes a change where instead of changing the (for example) OldHTTPServer.RequestTraceAttrs and CurrentHTTPServer.RequestTraceAttrs allocate we pass the slice created by OldHTTPServer.RequestTraceAttrs as an argument to CurrentHTTPServer.RequestTraceAttrs. This allows us to reduce the amount of allocation a little bit (see below).

Let me know what you think and thanks for reviewing this PR 😄

benchstat from instrumentation/net/http/otelhttp/internal/semconv with env var OTEL_SEMCONV_STABILITY_OPT_IN=http/dup.

goarch: amd64
pkg: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv
cpu: AMD Ryzen 7 Pro 7735U with Radeon Graphics     
                            │ a50074d5f20689a804d628d482d0b0cdfe73c623-dup.txt │             new-dup.txt             │
                            │                      sec/op                      │   sec/op     vs base                │
HTTPServerRequest-16                                              1046.5n ± 1%   932.8n ± 2%  -10.86% (p=0.001 n=10)
RecordMetrics/empty-16                                             7.202n ± 0%   7.262n ± 0%   +0.84% (p=0.000 n=10)
RecordMetrics/nil_meter-16                                         843.5n ± 1%   858.1n ± 1%   +1.72% (p=0.003 n=10)
RecordMetrics/with_Meter-16                                        842.7n ± 2%   867.6n ± 2%   +2.96% (p=0.000 n=10)
geomean                                                            270.5n        266.5n        -1.50%

                            │ a50074d5f20689a804d628d482d0b0cdfe73c623-dup.txt │              new-dup.txt               │
                            │                       B/op                       │     B/op      vs base                  │
HTTPServerRequest-16                                            2.938Ki ± 0%     2.062Ki ± 0%  -29.79% (p=0.000 n=10)
RecordMetrics/empty-16                                            0.000 ± 0%       0.000 ± 0%        ~ (p=1.000 n=10) ¹
RecordMetrics/nil_meter-16                                      1.313Ki ± 0%     1.313Ki ± 0%        ~ (p=1.000 n=10) ¹
RecordMetrics/with_Meter-16                                     1.313Ki ± 0%     1.313Ki ± 0%        ~ (p=1.000 n=10) ¹
geomean                                                                      ²                  -8.46%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                            │ a50074d5f20689a804d628d482d0b0cdfe73c623-dup.txt │             new-dup.txt              │
                            │                    allocs/op                     │ allocs/op   vs base                  │
HTTPServerRequest-16                                              3.000 ± 0%     2.000 ± 0%  -33.33% (p=0.000 n=10)
RecordMetrics/empty-16                                            0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
RecordMetrics/nil_meter-16                                        8.000 ± 0%     8.000 ± 0%        ~ (p=1.000 n=10) ¹
RecordMetrics/with_Meter-16                                       8.000 ± 0%     8.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                                                                      ²                -9.64%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

Benchstats with OTEL_SEMCONV_STABILITY_OPT_IN unset:

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv
cpu: AMD Ryzen 7 Pro 7735U with Radeon Graphics     
                            │ a50074d5f20689a804d628d482d0b0cdfe73c623.txt │              new.txt               │
                            │                    sec/op                    │   sec/op     vs base               │
HTTPServerRequest-16                                           442.6n ± 2%   444.0n ± 2%       ~ (p=0.669 n=10)
RecordMetrics/empty-16                                         7.229n ± 0%   7.261n ± 0%  +0.44% (p=0.000 n=10)
RecordMetrics/nil_meter-16                                     452.1n ± 1%   457.8n ± 1%  +1.27% (p=0.034 n=10)
RecordMetrics/with_Meter-16                                    453.2n ± 2%   450.7n ± 1%       ~ (p=0.615 n=10)
geomean                                                        160.0n        160.6n       +0.38%

                            │ a50074d5f20689a804d628d482d0b0cdfe73c623.txt │               new.txt               │
                            │                     B/op                     │    B/op     vs base                 │
HTTPServerRequest-16                                          704.0 ± 0%     704.0 ± 0%       ~ (p=1.000 n=10) ¹
RecordMetrics/empty-16                                        0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
RecordMetrics/nil_meter-16                                    672.0 ± 0%     672.0 ± 0%       ~ (p=1.000 n=10) ¹
RecordMetrics/with_Meter-16                                   672.0 ± 0%     672.0 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                                                  ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                            │ a50074d5f20689a804d628d482d0b0cdfe73c623.txt │               new.txt               │
                            │                  allocs/op                   │ allocs/op   vs base                 │
HTTPServerRequest-16                                          1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
RecordMetrics/empty-16                                        0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
RecordMetrics/nil_meter-16                                    4.000 ± 0%     4.000 ± 0%       ~ (p=1.000 n=10) ¹
RecordMetrics/with_Meter-16                                   4.000 ± 0%     4.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                                                  ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

[dmathieu]

How about rebasing your PR against #6899 ?

[MrAlias]

How about rebasing your PR against #6899 ?

Merged. Rebasing against main is needed.

[MrAlias]

Seems like a reasonable change overall. Just needs to be updated with main for evaluation.

[codecov]

Codecov Report

Attention: Patch coverage is 87.93103% with 28 lines in your changes missing coverage. Please review.

Project coverage is 80.9%. Comparing base (5997de2) to head (4c66f06).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...stful/otelrestful/internal/semconvutil/httpconv.go	80.0%	2 Missing and 1 partial ⚠️
...gonic/gin/otelgin/internal/semconvutil/httpconv.go	80.0%	2 Missing and 1 partial ⚠️
...rilla/mux/otelmux/internal/semconvutil/httpconv.go	80.0%	2 Missing and 1 partial ⚠️
...ack/echo/otelecho/internal/semconvutil/httpconv.go	80.0%	2 Missing and 1 partial ⚠️
...ace/otelhttptrace/internal/semconvutil/httpconv.go	80.0%	2 Missing and 1 partial ⚠️
...net/http/otelhttp/internal/semconvutil/httpconv.go	80.0%	2 Missing and 1 partial ⚠️
...lei/go-restful/otelrestful/internal/semconv/env.go	94.7%	1 Missing ⚠️
...go-restful/otelrestful/internal/semconv/v1.20.0.go	88.8%	1 Missing ⚠️
....com/gin-gonic/gin/otelgin/internal/semconv/env.go	94.7%	1 Missing ⚠️
.../gin-gonic/gin/otelgin/internal/semconv/v1.20.0.go	88.8%	1 Missing ⚠️
... and 6 more

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #7180     +/-   ##
=======================================
- Coverage   80.9%   80.9%   -0.1%     
=======================================
  Files        204     204             
  Lines      18030   18062     +32     
=======================================
+ Hits       14604   14625     +21     
- Misses      3007    3011      +4     
- Partials     419     426      +7     

Files with missing lines	Coverage Δ
...hub.com/emicklei/go-restful/otelrestful/restful.go	100.0% <100.0%> (ø)
...entation/github.com/labstack/echo/otelecho/echo.go	100.0% <100.0%> (ø)
...lei/go-restful/otelrestful/internal/semconv/env.go	91.4% <94.7%> (+0.4%)	⬆️
...go-restful/otelrestful/internal/semconv/v1.20.0.go	77.7% <88.8%> (+0.6%)	⬆️
....com/gin-gonic/gin/otelgin/internal/semconv/env.go	94.7% <94.7%> (+0.2%)	⬆️
.../gin-gonic/gin/otelgin/internal/semconv/v1.20.0.go	77.7% <88.8%> (+0.6%)	⬆️
...ub.com/gorilla/mux/otelmux/internal/semconv/env.go	92.7% <94.7%> (+0.4%)	⬆️
...om/gorilla/mux/otelmux/internal/semconv/v1.20.0.go	76.5% <88.8%> (+0.5%)	⬆️
...tp/httptrace/otelhttptrace/internal/semconv/env.go	90.1% <94.7%> (+0.5%)	⬆️
...ttptrace/otelhttptrace/internal/semconv/v1.20.0.go	76.5% <88.8%> (+0.5%)	⬆️
... and 8 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

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

[boekkooi-impossiblecloud]

Hey @dmathieu
The PR is rebased please let me know what you think 😄