Conversation
nkaradzhov
force-pushed
the
extend-proxy
branch
from
a79a62d to
92ec6fa
Compare
![jit-ci[bot]](https://avatars.githubusercontent.com/in/142441?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ The following Jit checks failed to run:
- secret-detection-trufflehog
#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.
More info in the Jit platform.
|
#jit_bypass_commit |
bobymicroby
left a comment
bobymicroby
left a comment
There was a problem hiding this comment.
LGTM. I've left some minor improvement suggestions.
| this.emit('data', connectionId, 'client->server', data); | ||
| serverSocket.write(data); | ||
|
|
||
| if(!this.interceptorInitializer) { |
There was a problem hiding this comment.
I'm wondering if we can eliminate the branch prediction (if(!this.interceptorInitializer)) entirely by implementing a default "socket write/noop" interceptor that simply performs the socket write when no initializer is provided.
There was a problem hiding this comment.
yes, we can totally do this, in fact it was like that before, but i thought this is better. i will move it back
| interface ActiveConnection extends ConnectionInfo { | ||
| readonly clientSocket: net.Socket; | ||
| readonly serverSocket: net.Socket; | ||
| inflightRequestsCount: number |
There was a problem hiding this comment.
What do you think about borrowing from node-redis's cache stats approach ? We could start by tracking inflightRequestsCount initially and add more metrics later. This design integrates well if you want to hook in traces, metrics, or logs.
There was a problem hiding this comment.
this is only to distinguish between request/response and push for now. it will go away in nest iteration
2 participants
The purpose for this PR is to extend the proxy to provide couple of new functionalities: