Remove gin specific probes

[grcevski]

Since we switched to putting the server HTTP probes on net/http.serverHandler.ServeHTTP the Gin specific probe is not needed. The handler for Gin is called from the default HTTP server handler and effectively we get duplicate probes. The e2e tests are purposefully left in place so that we can still test that Gin instrumentation is working. I modified the gin tests to do the same level of validation as the net/http tests and I removed the double server span.

This is the backtrace from Delve attached to our Gin example:

   569:	// ServeHTTP conforms to the http.Handler interface.
=> 570:	func (engine *Engine) ServeHTTP(w http.ResponseWriter, req *http.Request) {
   571:		c := engine.pool.Get().(*Context)
   572:		c.writermem.reset(w)
   573:		c.Request = req
   574:		c.reset()
   575:	
(dlv) bt
0  0x0000000000770a4e in github.com/gin-gonic/gin.(*Engine).ServeHTTP
   at /home/nino/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/gin.go:570
1  0x000000000065242e in net/http.serverHandler.ServeHTTP <--- /** the default HTTP server handler **/
   at /usr/local/go/src/net/http/server.go:3137
2  0x000000000064ea08 in net/http.(*conn).serve
   at /usr/local/go/src/net/http/server.go:2039
3  0x0000000000652c48 in net/http.(*Server).Serve.gowrap3
   at /usr/local/go/src/net/http/server.go:3285
4  0x000000000046ece1 in runtime.goexit
   at /usr/local/go/src/runtime/asm_amd64.s:1695

Closes #171 as not needed

[grcevski]

Hm, the test failure shouldn't have happened, when I test manually it works as expected with the same main.go test file in the e2e test:

{"level":"info","ts":1712962564.0520866,"logger":"Instrumentation.Controller","caller":"opentelemetry/controller.go:54","msg":"got event","attrs":[{"Key":"http.request.method","Value":{"Type":"STRING","Value":"GET"}},{"Key":"url.path","Value":{"Type":"STRING","Value":"/hello-gin"}},{"Key":"http.response.status_code","Value":{"Type":"INT64","Value":200}},{"Key":"network.peer.address","Value":{"Type":"STRING","Value":"::1"}},{"Key":"network.peer.port","Value":{"Type":"INT64","Value":48980}},{"Key":"server.address","Value":{"Type":"STRING","Value":"localhost"}},{"Key":"server.port","Value":{"Type":"INT64","Value":8080}},{"Key":"network.protocol.version","Value":{"Type":"STRING","Value":"1.1"}}],"status":{"Code":0,"Description":""},"context":{"TraceID":"1a9bb4d3e636d5657bb47b167431aa24","SpanID":"7f0bddaa6495f765","TraceFlags":"01","TraceState":"","Remote":false},"parent":{"TraceID":"1a9bb4d3e636d5657bb47b167431aa24","SpanID":"8853ec505dd0e9f8","TraceFlags":"01","TraceState":"","Remote":true}}
{"level":"info","ts":1712962564.0524924,"logger":"Instrumentation.Controller","caller":"opentelemetry/controller.go:54","msg":"got event","attrs":[{"Key":"http.request.method","Value":{"Type":"STRING","Value":"GET"}},{"Key":"url.path","Value":{"Type":"STRING","Value":"/hello-gin"}},{"Key":"http.response.status_code","Value":{"Type":"INT64","Value":200}},{"Key":"server.address","Value":{"Type":"STRING","Value":"localhost"}},{"Key":"server.port","Value":{"Type":"INT64","Value":8080}},{"Key":"network.protocol.version","Value":{"Type":"STRING","Value":"1.1"}}],"status":{"Code":0,"Description":""},"context":{"TraceID":"1a9bb4d3e636d5657bb47b167431aa24","SpanID":"8853ec505dd0e9f8","TraceFlags":"01","TraceState":"","Remote":false},"parent":null}
2024/04/12 18:56:07 traces export: Post

[grcevski]

I had to update the gin test at the end, we were checking for the scope to match the gin instrumentation and it was looking for a double server span.

[RonFed]

LGTM, not sure we should keep the e2e test since we have both http and http_custom e2e tests, so I don't see the added value of keeping this test.

[MrAlias]

LGTM, not sure we should keep the e2e test since we have both http and http_custom e2e tests, so I don't see the added value of keeping this test.

I think keeping them as a forward compatibility test with Gin is a decent reason to keep them. We can always remove them in the future if they become a burden.

[grcevski]

LGTM, not sure we should keep the e2e test since we have both http and http_custom e2e tests, so I don't see the added value of keeping this test.

I thought it would be useful to keep them around to prove we can still correctly instrument Gin, we are not dropping support for it, we are just handling it correctly with the server http instrumentation.