stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2070218457


##########
solr/core/src/java/org/apache/solr/core/TracerConfigurator.java:
##########
@@ -38,33 +41,53 @@
 public abstract class TracerConfigurator implements NamedListInitializedPlugin 
{
 
   public static final boolean TRACE_ID_GEN_ENABLED =
-      Boolean.parseBoolean(EnvUtils.getProperty("solr.alwaysOnTraceId", 
"true"));
+      EnvUtils.getPropertyAsBool("solr.alwaysOnTraceId", true);
 
   private static final String DEFAULT_CLASS_NAME =
       EnvUtils.getProperty(
           "solr.otelDefaultConfigurator", 
"org.apache.solr.opentelemetry.OtelTracerConfigurator");
 
   private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  public static Tracer loadTracer(SolrResourceLoader loader, PluginInfo info) {
-    if (info != null && info.isEnabled()) {
-      TracerConfigurator configurator =
-          loader.newInstance(info.className, TracerConfigurator.class);
-      configurator.init(info.initArgs);
-      ExecutorUtil.addThreadLocalProvider(new ContextThreadLocalProvider());
-      return configurator.getTracer();
-    }
-    if (shouldAutoConfigOTEL()) {
-      return autoConfigOTEL(loader);
+  /** Initializes {@link io.opentelemetry.api.GlobalOpenTelemetry} and returns 
a Tracer. */
+  public static synchronized Tracer loadTracer(SolrResourceLoader loader, 
PluginInfo info) {
+    // synchronized to avoid races in tests starting Solr concurrently setting 
GlobalOpenTelemetry
+    if (TraceUtils.OTEL_AGENT_PRESENT) {
+      log.info("OpenTelemetry Java agent was already installed; skipping OTEL 
initialization");
+    } else {
+      OpenTelemetry otel = null;
+      if (info != null && info.isEnabled()) {
+        var configurator = loader.newInstance(info.className, 
TracerConfigurator.class);
+        configurator.init(info.initArgs);
+        otel = configurator.createOtel();
+      } else if (shouldAutoConfigOTEL()) {
+        otel = autoConfigOTEL(loader); // could return null if failed to load
+        if (otel != null) {
+          log.info("OpenTelemetry loaded via auto configuration.");
+        }
+      }
+      if (otel == null && TRACE_ID_GEN_ENABLED) {
+        otel = 
OpenTelemetry.propagating(ContextPropagators.create(SimplePropagator.getInstance()));
+        log.info("OpenTelemetry loaded with simple propagation only.");
+      }
+      if (otel == null) {
+        otel = OpenTelemetry.noop();
+      }
+
+      try {
+        GlobalOpenTelemetry.set(otel); // throws IllegalStateException if 
already set
+        // skips needless ExecutorUtil.addThreadLocalProvider below
+        if (otel == OpenTelemetry.noop()) return otel.getTracer("solr");
+      } catch (IllegalStateException e) {
+        log.info("GlobalOpenTelemetry was already initialized (a Java agent?); 
using that");

Review Comment:
   coming back to this. not sure what your plans are, but I will reshare this 
approach I had in the `SimplePropagator` 
   
https://github.com/apache/solr/pull/2687/files#diff-e61b6fa689b156777ba2ecb37be366c4c312199302a2c90425acb460dbdb3f0eL57
 - I think using a flag to keep track of status works to get around the issue 
of multiple init calls.



-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to