Skip span creation when parent span is not recording

[Udhayarajan]

Problem

Right now, even though span is not recording, redisotel creates new span and still tracing out the information. Can we have someway of skipping?

We do have filter I'm aware of it. But irrespective of any command, if we want to have ability to skip spanning based on the recording state of the span in context at the root/call-site level, there is not option for it.

Proposed Solutions

1. adding a flag on options, that says skip trace if span not recording. (SkipSpanIfNotRecording)
2. adding a context parm to the filter functions (filterProcess , filterProcessPipeline), so that let users create a span from context and check if it is recording and should they skip it.
3. hard respect the span recording (yeah it is quite odd, but otelsql already doing it)

[ndyakov]

Thank you @Udhayarajan, redisotel is supported by the community. If you think this should be done - feel free to open a PR, ping people that contributed previously to the packaged and dicsuss with them your approaches. We will help with final review and merging this.

[thani-ath-nain]

@ndyakov I have raised a PR for this issue and tagged a few contributors. Would be very grateful if you could review it or tag other contributors better suited to review. Thanks!

[ndyakov]

@Udhayarajan please check if #3765 addresses your issue.

[Udhayarajan]

I've some ideas around propagating ctx through the filter hooks. pls check my PR #3792 as well.

To summarize the two approaches addressing this issue:

#3765 - Adds a new WithSkipSpanIfNotRecording boolean option. It solves the problem with a targeted, single-purpose config flag, but introduces a new addition to the options API.

#3792 - Updates the existing filterDial, filterProcess, and filterProcessPipeline signatures to accept context.Context, and introduces context-aware filter options (WithDialFilterCtx, WithCommandFilterCtx, WithCommandsFilterCtx) while deprecating the old ones via backward-compatible wrappers. This makes the filter fully flexible, users can inspect the span recording state (or anything else in ctx) directly in their filter logic.

Both are backward-compatible. The difference is philosophy: #3765 is a narrow opt-in flag, while #3792 gives users full control through context. Happy to go with whichever the maintainers prefer. No strong opinion from my side.

[ndyakov]

@Udhayarajan , @thani-ath-nain feel free to decided between yourselves. I also do not have a strong opinion. @ofekshenawa is activly supporting redisotel-native https://github.com/redis/go-redis/tree/master/extra/redisotel-native and may have opinion on this as well. We are planning to update the documentation and suggest redisotel-native as the otel package for go-redis in the near future (as we can't just break redisotel), so the maintenance of redisotel as a package is left to the community. I am here to help you with merging PRs, addressing CVEs if any and releasing.

Side note as a developer - I do believe a thin otel package is easier to work with. I would prefer my code to have as little logic around tracing and metrics as possible, so the main business logic can be easier to follow. In theory and in practice are two different things here. In practice a simple opt-in flag looks and feels slimmer. In theory the context approach is the go way. Honestly, I would probably go the go way and give the power to the end user to decide how to implement this and they can implement a predefined small filter function that can still make this approach feel slim in the application.

[thani-ath-nain]

@Udhayarajan I’m not against the context-based approach in #3792, but for this specific issue I think the smaller change is the better tradeoff; #3765 does exactly that with one opt-in option and no change to the default behavior. We don't need to think of a deprecation timeline and can just give people the option they need. The PR itself is fully backward-compatible and already includes coverage for dial, process, and pipeline behavior, including no parent span, recording parent span, sampled-out parent span, and preserving the current behavior when the option is disabled. If there’s still interest in broader context-aware filtering after this, that can absolutely be discussed as a separate feature.