improve netProtocol performance for well-known protocols

[boekkooi-impossiblecloud]

Good day,

While I was review some profiles I noticed that the open-telemetry code was calling strings.ToLower a bit more then I expected.
After some digging I notice that netProtocol was called multiple times causing strings.ToLower to be called multiple times.

Now for our application 99.9% of all requests are HTTP/1.1 (and I expect this is the same for many others) so instead of calling strings.ToLower every-time netProtocol now checks some well known protocols first.

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

P.S. sorry for not updating the changelog but I honestly have no idea how to document this change as it's internal.

[dmathieu]

Could you share benchmarks showing the actual impact of this change?

[boekkooi-impossiblecloud]

@dmathieu please see below.

Benchstat from instrumentation/net/http/otelhttp/internal/semconv

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv
cpu: AMD Ryzen 7 Pro 7735U with Radeon Graphics     
                            │    old.txt    │               pr.txt                │
                            │    sec/op     │   sec/op     vs base                │
HTTPServerRequest-16          2581.5n ± 80%   419.6n ± 4%  -83.75% (p=0.000 n=10)
RecordMetrics/empty-16         10.44n ±  5%   10.12n ± 5%        ~ (p=0.085 n=10)
RecordMetrics/nil_meter-16     501.5n ±  4%   459.2n ± 3%   -8.42% (p=0.000 n=10)
RecordMetrics/with_Meter-16    507.2n ±  3%   447.9n ± 4%  -11.67% (p=0.000 n=10)
geomean                        287.7n         171.9n       -40.25%

                            │   old.txt    │               pr.txt                │
                            │     B/op     │    B/op     vs base                 │
HTTPServerRequest-16          712.0 ± 0%     704.0 ± 0%  -1.12% (p=0.000 n=10)
RecordMetrics/empty-16        0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
RecordMetrics/nil_meter-16    680.0 ± 0%     672.0 ± 0%  -1.18% (p=0.000 n=10)
RecordMetrics/with_Meter-16   680.0 ± 0%     672.0 ± 0%  -1.18% (p=0.000 n=10)
geomean                                  ²               -0.87%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                            │   old.txt    │                pr.txt                │
                            │  allocs/op   │ allocs/op   vs base                  │
HTTPServerRequest-16          2.000 ± 0%     1.000 ± 0%  -50.00% (p=0.000 n=10)
RecordMetrics/empty-16        0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
RecordMetrics/nil_meter-16    5.000 ± 0%     4.000 ± 0%  -20.00% (p=0.000 n=10)
RecordMetrics/with_Meter-16   5.000 ± 0%     4.000 ± 0%  -20.00% (p=0.000 n=10)
geomean                                  ²               -24.79%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

Main branch (89cbae1)

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv
cpu: AMD Ryzen 7 Pro 7735U with Radeon Graphics     
BenchmarkHTTPServerRequest-16    	 2369716	       524.4 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	 2454291	       470.1 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	 2357473	       538.1 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	 2265892	      1723 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	  451032	      2472 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	  505610	      2691 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	  426222	      3316 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	  368545	      3395 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	  334755	      3689 ns/op	     712 B/op	       2 allocs/op
BenchmarkHTTPServerRequest-16    	  346980	      3088 ns/op	     712 B/op	       2 allocs/op
BenchmarkRecordMetrics/empty-16  	50991614	        32.87 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	119909341	        10.20 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	124932790	        10.38 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.39 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	120080523	         9.715 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.48 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	121441090	         9.947 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.48 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.90 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.61 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2472054	       491.6 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2196092	       493.1 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2197831	       490.7 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2500448	       498.9 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2237450	       523.9 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2403915	       494.0 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2360803	       511.4 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2323566	       520.9 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2368915	       504.1 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2481480	       530.8 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2329453	       507.1 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2380334	       498.3 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2136846	       507.2 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2452105	       505.0 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2395195	       510.3 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2239412	       510.1 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2409913	       497.0 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2231613	       557.3 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2236351	       498.2 ns/op	     680 B/op	       5 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2431416	       524.7 ns/op	     680 B/op	       5 allocs/op
PASS
ok  	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv	70.880s

PR (2074e6f)

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv
cpu: AMD Ryzen 7 Pro 7735U with Radeon Graphics     
BenchmarkHTTPServerRequest-16    	 3104797	       425.1 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 2791826	       401.9 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 2901752	       407.1 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 2658422	       452.8 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 2667374	       428.2 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 3016105	       407.2 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 2882822	       432.3 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 2949312	       399.5 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 2901847	       417.1 ns/op	     704 B/op	       1 allocs/op
BenchmarkHTTPServerRequest-16    	 3006976	       422.0 ns/op	     704 B/op	       1 allocs/op
BenchmarkRecordMetrics/empty-16  	127014920	         9.864 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.40 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.39 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	123866020	         9.788 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	121044003	        10.23 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	123466990	         9.619 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.01 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.57 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	126233925	         9.660 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/empty-16  	100000000	        10.38 ns/op	       0 B/op	       0 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2468714	       451.0 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2743141	       460.6 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2706985	       471.7 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2653555	       446.4 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2748116	       485.3 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2649477	       457.9 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2649898	       447.7 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2516574	       463.2 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2585972	       447.9 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/nil_meter-16         	 2767576	       464.7 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2587676	       459.6 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2627959	       441.8 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2724260	       462.9 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2557381	       448.6 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2645035	       439.7 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2560101	       485.4 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2605543	       447.3 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2758518	       466.1 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2516584	       440.9 ns/op	     672 B/op	       4 allocs/op
BenchmarkRecordMetrics/with_Meter-16        	 2707358	       445.7 ns/op	     672 B/op	       4 allocs/op
PASS
ok  	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp/internal/semconv	65.950s

[pellared]

sorry for not updating the changelog but I honestly have no idea how to document this change as it's internal.

We have some "Improve performance" changelog entries.

[codecov]

Codecov Report

Attention: Patch coverage is 69.69697% with 30 lines in your changes missing coverage. Please review.

Project coverage is 76.0%. Comparing base (ed30671) to head (7a4878d).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ei/go-restful/otelrestful/internal/semconv/util.go	33.3%	6 Missing ⚠️
...com/gin-gonic/gin/otelgin/internal/semconv/util.go	33.3%	6 Missing ⚠️
...b.com/gorilla/mux/otelmux/internal/semconv/util.go	33.3%	6 Missing ⚠️
...p/httptrace/otelhttptrace/internal/semconv/util.go	33.3%	6 Missing ⚠️
...ntation/net/http/otelhttp/internal/semconv/util.go	33.3%	6 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #6845     +/-   ##
=======================================
- Coverage   76.1%   76.0%   -0.1%     
=======================================
  Files        219     219             
  Lines      21299   21387     +88     
=======================================
+ Hits       16210   16265     +55     
- Misses      4529    4561     +32     
- Partials     560     561      +1     

Files with missing lines	Coverage Δ
...estful/otelrestful/internal/semconvutil/netconv.go	100.0% <100.0%> (ø)
...-gonic/gin/otelgin/internal/semconvutil/netconv.go	100.0% <100.0%> (ø)
...orilla/mux/otelmux/internal/semconvutil/netconv.go	100.0% <100.0%> (ø)
...tack/echo/otelecho/internal/semconvutil/netconv.go	100.0% <100.0%> (ø)
...race/otelhttptrace/internal/semconvutil/netconv.go	100.0% <100.0%> (ø)
.../net/http/otelhttp/internal/semconvutil/netconv.go	100.0% <100.0%> (ø)
...ei/go-restful/otelrestful/internal/semconv/util.go	81.5% <33.3%> (-8.0%)	⬇️
...com/gin-gonic/gin/otelgin/internal/semconv/util.go	81.5% <33.3%> (-8.0%)	⬇️
...b.com/gorilla/mux/otelmux/internal/semconv/util.go	81.5% <33.3%> (-8.0%)	⬇️
...p/httptrace/otelhttptrace/internal/semconv/util.go	81.5% <33.3%> (-8.0%)	⬇️
... and 1 more

... and 1 file with indirect coverage changes

[boekkooi-impossiblecloud]

sorry for not updating the changelog but I honestly have no idea how to document this change as it's internal.

We have some "Improve performance" changelog entries.

Yes but these always mention public packages not internals and a previous PR for opentelemetry-go it was mentioned not to mention the private packages,

[dmathieu]

Changelog entries focus on user-facing changes. Hence that comment in otel-go, where the entry showed changed in private methods and had to be changed to show the impact for end-users.

Mentioning that performance (and allocations) have been improved would be fine. However, the changelog entry shouldn't mention the private package, but every public package it's used in.

[boekkooi-impossiblecloud]

Hey @dmathieu

I added 7a4878d with the changelog entry and resolved the make precommit error in 21776ed (sorry about that I was sure I ran that before pushing last time)