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

Add backend URL to HTTP Request object context #31

Closed
wants to merge 2 commits into from

Conversation

KevinRandolph
Copy link

I've added a new member to the structs for roundrobin/rr.go and roundrobin/rebalancer.go. These are flags that will allow conditional setting of the backend URL into the HTTP Request object via the gorilla/context package.

These changes should be transparent to existing users since the default behavior is not to set the context.

The intent of the changes is to allow logging of the backend URL in the traefik reverse proxy.

@KevinRandolph
Copy link
Author

Please disregard. I think the changes should have been made in the mailgun repo. I'm closing this PR.

@KevinRandolph
Copy link
Author

Actually, this is the right repo. For some reason I thought oxy was in mailgun.

@klizhentas
Copy link
Contributor

hey, thanks for the PR! Can you implement it using more generic interface and functional parameter instead of pulling dependency on gorilla for this?

Something like this will work:

type ContextSetter interface {
   Set(key string, val interface{})
}


func SetContextSetter(...) ForwardParam {
...
}

This will allow a more generic solution to plug in other context types. Let me know what do you think

@KevinRandolph
Copy link
Author

Alexander,

Thank you for taking the time to acknowledge my PR.

Yes, I agree it is better to have a generic interface and not be tied to a particular context package. I'll work on implementing that.

@klizhentas
Copy link
Contributor

😄 thanks!

@mikaelnousiainen
Copy link

I was looking for similar functionality in "roundrobin" and also in "forward" package, as I need to log the backend URL.

Additionally, I'd like to perform accurate timing of RoundTripper requests, so I'd basically need a pre- and post- callbacks around the RoundTripper call with access to both client request and response.

This kind of solution (optional pre- and post-callback functions with request and response as function parameters) could be used to extract any information from the request and response, including the URL.

@mikaelnousiainen
Copy link

The proposed ContextSetter interface would not work, because it assumes that the context is global, while in many cases the context contains request-specific data and needs to be set per request. Additionally, using request pointers as keys to map (like in gorilla/context package) does not work properly, since some of the oxy middleware handlers clone the request before modifying it.

Thus, the only clean way to resolve this (in my opinion) is to introduce an alternative HTTP handler interface and pass a context (from x/net/context, for example) as a parameter to the handler. Each middleware handler should then implement another handler method and check if the "next" handler accepts a context as well.

This blog post explain the possible alternatives very clearly:
https://joeshaw.org/net-context-and-http-handler/

@mikaelnousiainen
Copy link

I think I'll implement this in roundrobin and forward packages for my own purposes and create another pull request so that you can see the proposed solution.

@KevinRandolph
Copy link
Author

We have taken a different approach to solving the URL issue. I am closing this PR.

@klizhentas, thank you for your consideration and comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants