Skip to content

Conversation

@zadil
Copy link

@zadil zadil commented Oct 25, 2025

Add comprehensive filtering capabilities with performance-optimized command exclusion and unified filtering API.

  • WithExcludedCommands(): O(1) command exclusion using map lookup
  • WithProcessFilter(): Custom filtering for individual commands
  • WithProcessPipelineFilter(): Custom filtering for pipeline operations
  • WithDialFilterFunc(): Custom filtering for dial operations
  • Backward compatible with existing filtering APIs
  • Comprehensive test coverage and updated examples

Fixes #3479

Add comprehensive filtering capabilities with performance-optimized command
exclusion and unified filtering API.

- WithExcludedCommands(): O(1) command exclusion using map lookup
- WithProcessFilter(): Custom filtering for individual commands
- WithProcessPipelineFilter(): Custom filtering for pipeline operations
- WithDialFilterFunc(): Custom filtering for dial operations
- Backward compatible with existing filtering APIs
- Comprehensive test coverage and updated examples

Fixes redis#3479
@jit-ci
Copy link

jit-ci bot commented Oct 25, 2025

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

@ndyakov
Copy link
Member

ndyakov commented Oct 25, 2025

@zadil thank you for your contribution, will review it next week.

@ndyakov
Copy link
Member

ndyakov commented Oct 28, 2025

pinging @Udhayarajan and @lucawol for comments here as they were active in the discussion.

@Udhayarajan
Copy link
Contributor

I've a few things to mark on.

0. excludedCommands

The goal is to filter in O(1). Anyone who want to achieve O(1) is still possible via existing methods. They can create a custom stuct and still can achieve the same (as I mentioned in discussion). Afaik go inline methods, when I tested with same I proposed in discussion I can see the go compiler says both FilterPipeline and FilterCommand can inline. The O(1) map lookup logic is now as efficient as possible since the filtering methods are inlined.

This excludeCommands just offers a way so that people no need to create their own struct to achieve O(1).

Trade off:

IMO, We are trading off library code quality with pre-cooked fields in config struct. I love the way you reduced double maintenance with simple helper functions, everything looks like over-kill, at lease to me ,as func(cmds []redis.Cmder) bool and func(cmd redis.Cmder) bool can do what excludedCommands does.

1. unified* vs filter* (excluding unifiedDialFilter)

everything is exactly the same do we really need to duplicate this fields? Though the version is released these are non-exported filed and shouldn't affect any user, may be we just have one field and use them in 2 different option functions, what's your opinion?

2. unifiedDialFilter

I designed dial filter assuming, user won't filter the trace based on network and address thus kept is simple. Current proposed approach gives better flexibility to filter the dial with help of address and port.

Trade off:

I don't think there is any critical trade-off, but in rare case, say user configured to filter from their current address and port but later if redis server is changed then they suddenly starting seeing trace. Again, I'm not sure is this trade-off or can this be even labeled as careless update by user. If I'm that user for sure I won't be remembering this part where I configured to ignore dial from certain address and port. But it offers great flexibility to users for filtering out dial for specific server in cluster mode.

I'm in favor of this unifiedDialFilter

@ndyakov pls add your inputs too.

cc: @htemelski-redis

@Udhayarajan
Copy link
Contributor

Udhayarajan commented Oct 28, 2025

I would like to pull in one more person, since we're just adding toppings to the base baked by @Sovietaced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Filter out specific commands for OTEL Tracing

3 participants