Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] cannot use __orchestrion_instrument.WrapHandlerFunc #400

Closed
tonyredondo opened this issue Nov 18, 2024 · 2 comments · Fixed by #403
Closed

[BUG] cannot use __orchestrion_instrument.WrapHandlerFunc #400

tonyredondo opened this issue Nov 18, 2024 · 2 comments · Fixed by #403
Labels
bug Something isn't working

Comments

@tonyredondo
Copy link
Member

Version of orchestrion

0.9.4

Describe what happened:

Received this build error:

config/service/http_health_forwarder.go:157: cannot use __orchestrion_instrument.WrapHandlerFunc(func(w http.ResponseWriter, r *http.Request) {…}) (value of type "net/http".HandlerFunc) as handlerFunc value in return statement

function:

func healthCheckBuilder(serviceName string, healthClient *httpHealthClient) handlerFunc {
	return func(w http.ResponseWriter, r *http.Request) {
		// ...
	}
}

Describe what you expected:

A normal build execution

Steps to reproduce the issue:

Additional environment details (Version of Go, Operating System, etc.):

@tonyredondo tonyredondo added the bug Something isn't working label Nov 18, 2024
@rarguelloF
Copy link
Contributor

@tonyredondo Thanks for reporting the issue!

Could you please share the handlerFunc type? To make sure I accurately reproduce the issue.

@tonyredondo
Copy link
Member Author

@tonyredondo Thanks for reporting the issue!

Could you please share the handlerFunc type? To make sure I accurately reproduce the issue.

shared offline.

github-merge-queue bot pushed a commit that referenced this issue Nov 22, 2024
Resolves #173, resolves
#400

This PR changes `net/http` server-side instrumentation aspects to trace
the library itself (similar approach as we are doing today with the
client-side instrumentation). The hook where we inject is the
`http.Server.Serve` method, which should be the common method that is
always called when the http server is started (and at this point, the
Handler should already be set).

This fixes problems in the existing instrumentation where we:
- Don't trace custom `http.Handler` implementations
(#173).
- We double-trace `http.HandlerFunc(func(w http.ResponseWriter, r
*http.Request))` declarations.
- We might even break the build (as described in
#400).
- Patterns using assignment like `srv.Handler = myHandler` are not
traced.

As a side-effect, this approach removes the ability to use
`orchestrion:ignore` comments to skip tracing on http servers (which
happens for other aspects as well where we instrument the library code).

---------

Co-authored-by: Romain Marcadier <[email protected]>
Co-authored-by: Romain Marcadier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants