Home Original page

Extend proxy by nkaradzhov · Pull Request #3105 · redis/node-redis

@nkaradzhov

The purpose for this PR is to extend the proxy to provide couple of new functionalities:

  • request/response
  • response transformations
  • resp support

@nkaradzhov

jit-ci[bot]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ 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.

@nkaradzhov

bobymicroby

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I've left some minor improvement suggestions.

this.emit('data', connectionId, 'client->server', data);
serverSocket.write(data);

if(!this.interceptorInitializer) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only to distinguish between request/response and push for now. it will go away in nest iteration

@nkaradzhov