Add ProxyConnectionSettings to all connection types #221
Conversation
michel-laterman
force-pushed
the
feat/proxy-connection-settings
branch
from
f76d025 to
203dded
Compare
| string url = 1; | ||
|
|
||
| // Optional headers to send to proxies during CONNECT requests. | ||
| // These headers can be ignored for non-HTTP based proxies. |
There was a problem hiding this comment.
What's a non-HTTP based proxy?
| @@ -432,6 +444,18 @@ message TLSConnectionSettings { | |||
| repeated string cipher_suites = 6; | |||
| } | |||
|
|
|||
| // Status: [Development] | |||
| message ProxyConnectionSettings { | |||
There was a problem hiding this comment.
Does this only apply to plain http transport or also to WebSocket? I do not know how proxies work for WebSocket connections.
There was a problem hiding this comment.
gorilla/websocket is lacking support for connection headers, but the URL will apply.
There was a problem hiding this comment.
I've made a PR to add this, hopefully it can be review soon: gorilla/websocket#988
| ##### ProxyConnectionSettings.connect_headers | ||
|
|
||
| Option headers the Client should set for HTTP based proxies initial CONNECT | ||
| request. Other proxy types, such as SOCKS5 may ignore these headers. |
There was a problem hiding this comment.
Where is the type of the proxy specified? Does the Client need to know the type of the proxy?
There was a problem hiding this comment.
I think due to how websockets behaves our proxy settings will differ for websocket/http servers.
There's currently an open issue: http://github.com/gorilla/websocket/issues/479 to allow connection headers with websockets.
EDIT: To answer your question; the schema in the URL should indicate the type i.e., http, sock5.
HTTP proxies should use the headers, but as stated above there is an issue that effects opamp-go implementation when using an http proxy with a websocket connection
There was a problem hiding this comment.
My demo now sets proxy connect headers: open-telemetry/opamp-go#389
|
Sorry for the delay, I had the comments in Pending state, forgot to publish them 🤦 |
Add a new
ProxyConnectionSettingsmessage to all connection types.WIP Demo: open-telemetry/opamp-go#389