ctubbsii commented on a change in pull request #2363:
URL: https://github.com/apache/accumulo/pull/2363#discussion_r754541157
##########
File path: core/src/main/java/org/apache/accumulo/core/trace/TraceUtil.java
##########
@@ -56,97 +52,32 @@
private static final String SPAN_FORMAT = "%s::%s";
- private static Tracer instance = null;
- private static boolean tracing = false;
+ private static volatile boolean enabled = true;
- private static void initializeInternals(OpenTelemetry ot) {
- instance = ot.getTracer(APPNAME, VERSION);
- tracing = !ot.equals(OpenTelemetry.noop());
- LOG.info("Trace enabled: {}, OpenTelemetry instance: {}, Tracer instance:
{}", tracing,
- ot.getClass(), instance.getClass());
+ public static void initializeTracer(AccumuloConfiguration conf) {
+ enabled = conf.getBoolean(Property.GENERAL_OPENTELEMETRY_ENABLED);
+ LOG.info("Trace enabled: {}, OpenTelemetry instance: {}, Tracer instance:
{}", enabled,
Review comment:
I can try to consolidate the log messages. I didn't before because one
was at INFO and the others at DEBUG, and it didn't seem to save much.
I don't think it makes sense to do the additional check in
`getOpenTelemetry()`, since that won't have any effect on its behavior. I
created that separate method instead of inlining it into `getTracer()`
specifically for the purposes of being able to log it. However, I think I do
need to improve that slightly, so we don't log one OpenTelemetry, and then it
changes, and we log a different OpenTelemetry's tracer. I can make sure
`getTracer` always uses the OpenTelemetry object retrieved from
`getOpenTelemetry()`.
As for it still logging that trace is enabled, I think it's important that
we log the setting that Accumulo has been configured with. If it's still using
the NOOP implementation, that will be revealed in the remainder of the log
message when we print the specific name of the class. I can update the format
string to be more clear about what "true" means, but I'd rather not modify it
to mean that we've set it to enabled *and* that a non-NOOP implementation is
configured. I'd rather it mean just that we've set it to enabled, and leave the
rest of the log message to report on the implementation being used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
