Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to disable tracing for individual commands #2373

Closed
huangmiao opened this issue Mar 30, 2023 · 22 comments
Closed

Add option to disable tracing for individual commands #2373

huangmiao opened this issue Mar 30, 2023 · 22 comments
Labels
type: feature A new feature
Milestone

Comments

@huangmiao
Copy link

l dont show ping hello command . set observationPredicate
image
l use zipkin
image
only change or add tag don`t remove ping and hello .. command
Could you tell me where I can filter

@mp911de
Copy link
Collaborator

mp911de commented Mar 30, 2023

Right now, you can achieve that by wrapping the Tracing object and return a no-op span from Tracer.Span.start(RedisCommand<?, ?, ?> command) and do not finish the span on Tracer.Span.finish().

@huangmiao
Copy link
Author

image
image

I probably looked at the logic of this piece, and I will come to this method

@huangmiao
Copy link
Author

image
If I want to ignore the corresponding command, it can only be achieved by rewriting the brave package. Is there any corresponding support for this piece of follow-up, for example, I can specify certain commands to enter the trace

@huangmiao
Copy link
Author

In fact, I thought of another solution, that is, do not use the tracing of lettuce, and bury it in the redisTemplate, but I found that lettuce has the extension of tracing, and the provided one is not so perfect. I would like to ask if you will consider adding this piece in the future can be expanded

@huangmiao
Copy link
Author

Can I provide the corresponding code to expand here, add ignore request

@huangmiao
Copy link
Author

image
hi I found that the custom span can be loaded again after getting the result, which is convenient for us to customize some attributes

@huangmiao
Copy link
Author

Now I have encountered a difficult problem. I need to record the results. I can’t get its output in the buried point. If the customized span can be called again in the output, it will be convenient for this block to perform some logic processing and other functions.

@huangmiao
Copy link
Author

In fact, I hope that the customized span can be executed after redis completes the result, otherwise the corresponding response result will not be obtained

@mp911de
Copy link
Collaborator

mp911de commented Apr 18, 2023

We could consider an extension in the form of a predicate whether to trace individual commands. A per-command Tracing.isEnabled(RedisCommand) could work, the predicate would be configured via BraveTracing.Builder.tracePredicate(Predicate<RedisCommand>).

Feel free to submit a pull request.

@mp911de mp911de changed the title lettuce trace how to ignore ping hello command Add option to disable tracing for individual commands Apr 18, 2023
@mp911de mp911de added type: feature A new feature status: help-wanted An issue that a contributor can help us with labels Apr 18, 2023
@huangmiao
Copy link
Author

ok.
I submit code

@mp911de
Copy link
Collaborator

mp911de commented Apr 19, 2023

Would it be an option for you to switch to Micrometer Tracing (from Brave Tracing)? If so, then you could leverage the existing ObservationPredicate API from Micrometer. I'm currently in the process of adding Micrometer Tracing support to Lettuce (#2391)

@huangmiao
Copy link
Author

image
I see that the code you submitted has been switched to the micrometer, and the observation method is adopted. At present, I only need to wait for the new version to be released.

@huangmiao
Copy link
Author

image
Is it possible to add a listening event here? My requirement is to process the corresponding response results once.

@mp911de
Copy link
Collaborator

mp911de commented Apr 20, 2023

You can attach a callback to the future if you want to listen for responses.

@huangmiao
Copy link
Author

That's what I thought too. At present, I can't receive the response data as before.

@andrewkruse
Copy link

Was there any traction on this? I am attempting to use micrometer with spring data redis but unfortunately... the tracing spits out the password if the HELLO command ever fails which is a no-go (we wont go into why its failing, its embarrassing...).

I attempted to wire myself in via spring with an ObservationPredicate, however the context has no data for me to creep on and exclude the observation that appears. 😢 I suspect I'm possibly in the wrong spot seeing as how the code I'm digging through says spring all over it, but would love to have some guidance if there is any.

All I really need is the ability to trash the one tag on the one span for the hello operation.

@mp911de
Copy link
Collaborator

mp911de commented Oct 26, 2023

ObservationPredicate is applied pretty early in the process when we create an Observation object. All the interesting data is being set after we created the Observation (command name, endpoint, command details).

Paging @marcingrzejszczak on how to suppress sending/handling an Observation on Observation.stop() by providing bits of configuration.

@mp911de mp911de added this to the 6.2.7.RELEASE milestone Oct 26, 2023
@mp911de mp911de removed the status: help-wanted An issue that a contributor can help us with label Oct 26, 2023
@mp911de mp911de modified the milestones: 6.2.7.RELEASE, 6.3 Oct 26, 2023
@mp911de
Copy link
Collaborator

mp911de commented Oct 26, 2023

Deferring Observation creation works well. We collect all details and populate LettuceObservationContext before we create the Observation. That allows a registered ObservationPredicate to react to RedisCommand.

mp911de added a commit that referenced this issue Oct 26, 2023
ObservationPredicate can now be used to filter unwanted commands.
@mp911de
Copy link
Collaborator

mp911de commented Oct 26, 2023

That change is in place now.

@mp911de mp911de closed this as completed Oct 26, 2023
@andrewkruse
Copy link

andrewkruse commented Oct 26, 2023

Thank you for the guidance/hint on the predicates being the wrong spot.

Here's the snippet to blast it away without needing these tweaks.

    @Bean
    fun noRedisHelloCommandError(): ObservationRegistryCustomizer<ObservationRegistry> =
        ObservationRegistryCustomizer<ObservationRegistry> { registry ->
            registry.observationConfig()
                .observationFilter { context ->
                    if (context is LettuceObservationContext) {
                        if (context.requiredCommand.type == CommandType.HELLO) {
                            context.removeHighCardinalityKeyValue("spring.data.redis.command.error")
                        }
                    }
                    context
                }
        }

@just-ysc
Copy link

@mp911de Hi, I'm using lettuce via spring data redis.
I want to use ObservationPredicate in order to exclude some commands(HELLO, CLUSTER, INFO) from tracing, but when using spring data redis, the LettuceObservationContext is not from lettuce-core. It's from spring-data-redis.
When do you think this change will be applied to spring-data-redis, too? And under these circumstances, is there any workaround to use ObservationPredicate?
The code snippet is as following:

@Bean
fun observationRegistryCustomizer(): ObservationRegistryCustomizer<ObservationRegistry> =
    ObservationRegistryCustomizer<ObservationRegistry> { registry ->
        registry.observationConfig()
            .observationPredicate { observationName, context ->
                if (context is LettuceObservationContext) {
                    if (context.requiredCommand.type == CommandType.HELLO || context.requiredCommand.type == CommandType.INFO) {
                        return@observationPredicate false
                    }
                }
                true
            }
    }

@mp911de
Copy link
Collaborator

mp911de commented Jan 16, 2024

As of Lettuce 6.3, there's a Lettuce-specific LettuceObservationContext.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A new feature
Projects
None yet
Development

No branches or pull requests

4 participants