Skip to content

Added ability to log redis commands and their replies#701

Closed
alexcarruthers wants to merge 1 commit into
redis:masterfrom
alexcarruthers:master
Closed

Added ability to log redis commands and their replies#701
alexcarruthers wants to merge 1 commit into
redis:masterfrom
alexcarruthers:master

Conversation

@alexcarruthers

Copy link
Copy Markdown

This allows for insight into what's going into and out of redis without the performance hit created by monitor.

@brycebaril

Copy link
Copy Markdown
Contributor

There is already a debug mode you can enable in the client -- does that not do what you need?

@alexcarruthers

Copy link
Copy Markdown
Author

I'm looking for something that is not used purely for testing purposes. This is more for debugging a production environment after something has gone wrong. This produces one statement that is easy to track in logging tools whereas debug produces a bunch of unconnected statements that don't mean anything on their own and is almost impossible to group them together.

@BridgeAR

BridgeAR commented Sep 3, 2015

Copy link
Copy Markdown
Contributor

Ping @brycebaril I think it does not really hurt to add this.

@brycebaril

Copy link
Copy Markdown
Contributor

I'll leave it up to you @BridgeAR, my biggest issues are:

  1. interface bloat (adding options to the constructor when there are already alternatives)
  2. this still could be a significant burden to run in production if your values are large
  3. it misses the error case

I agree though that this could be more easily used than the existing debug messages if you simply want to see the commands and values being returned.

@BridgeAR

BridgeAR commented Sep 8, 2015

Copy link
Copy Markdown
Contributor

@alexcarruthers if you fix the mentioned issues by @brycebaril I'd be fine to merge this.

Please do so by:

  1. Use a env variable instead of adding another option. That way it is also possible to start and end this behavior at any point of the user program.
  2. Only print replies that are shorter than an arbitrary small value (e.g. 100 chars) and add dots to the end of the reply if the reply exceeds that value.
  3. Add the error case

@BridgeAR

BridgeAR commented Sep 8, 2015

Copy link
Copy Markdown
Contributor

And please add a test too

@BridgeAR BridgeAR closed this Oct 4, 2015
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