ISPN-14426 Allow to disable tracing propagation on HotRod client #10552
Conversation
...rod-client/src/main/java/org/infinispan/client/hotrod/impl/operations/OperationsFactory.java
Outdated
Show resolved
Hide resolved
Good idea. I added it. Thanks for the review @karesti |
|
if CI is ok |
| @@ -91,6 +91,7 @@ public class ConfigurationBuilder implements ConfigurationChildBuilder, Builder< | |||
| private final List<SerializationContextInitializer> contextInitializers = new ArrayList<>(); | |||
| private final Map<String, RemoteCacheConfigurationBuilder> remoteCacheBuilders; | |||
| private TransportFactory transportFactory = TransportFactory.DEFAULT; | |||
| private boolean tracingEnabled = true; | |||
There was a problem hiding this comment.
Should we disable it by default (I know: API change, but still)
There was a problem hiding this comment.
I don't know. Since we are not disabling the tracing here, only the client project can do it. We are disabling the tracing context propagation from the client (valid only if the client already uses tracing) to the server.
Maybe we should change the property name to tracingPropagationEnabled. @karesti what do you think of this?
There was a problem hiding this comment.
I think we should enable it by default since depends also on the presence of a jar
|
thanks @tristantarrant for the review. I made some changes |
|
In order for this to be complete, it's missing the following:
|
I tried to do it. @tristantarrant is it OK? Thanks |
| @@ -391,4 +391,12 @@ public interface Log extends BasicLogger { | |||
| @Message(value = "OpenTelemetry API is not present in the classpath. Client context tracing will not be propagated.", id = 4109) | |||
| void noOpenTelemetryAPI(@Cause Throwable throwable); | |||
|
|
|||
| @LogMessage(level = DEBUG) | |||
| @Message(value = "OpenTelemetry API is present in the classpath and the tracing propagation is enabled. Client context tracing be propagated.", id = 4110) | |||
There was a problem hiding this comment.
s/tracing be propagated/tracing will be propagated/
There was a problem hiding this comment.
Thanks @tristantarrant just fixed!
|
Merged, thanks |
|
Thank you! |
https://issues.redhat.com/browse/ISPN-14426