mlbiscoc commented on code in PR #3379:
URL: https://github.com/apache/solr/pull/3379#discussion_r2114832472


##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -423,6 +433,11 @@ public CoreContainer(NodeConfig config, CoresLocator 
locator, boolean asyncSolrC
     this.solrHome = config.getSolrHome();
     this.solrCores = SolrCores.newSolrCores(this);
     this.nodeKeyPair = new SolrNodeKeyPair(cfg.getCloudConfig());
+    this.prometheusMetricReader = new PrometheusMetricReader(true, null);
+    initializeOpenTelemetrySdk();
+    this.tracer = TraceUtils.getGlobalTracer();
+    this.meterProvider = MetricUtils.getMeterProvider();

Review Comment:
   This was initially done with `TracerConfigurator.loadTracer` inside of 
`loadInternal` Moved it up to be called earlier.



##########
solr/core/src/java/org/apache/solr/core/OpenTelemetryConfigurator.java:
##########
@@ -46,26 +50,53 @@ public abstract class TracerConfigurator implements 
NamedListInitializedPlugin {
 
   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);
-    }
+  private static volatile boolean loaded = false;
+
+  public static synchronized void configureOpenTelemetrySdk(
+      SdkMeterProvider sdkMeterProvider, SdkTracerProvider sdkTracerProvider) {
+    if (loaded) return;

Review Comment:
   Its a bit annoying Open Telemetry doesn't have a method to call if the 
`GlobalOpenTelemetry` was already set. So I understand why this boolean loaded 
existed on `SimplePropagator` now.



##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -54,19 +50,6 @@ public class SimplePropagator implements TextMapPropagator {
 
   private static final AtomicLong traceCounter = new AtomicLong(0);
 
-  private static volatile boolean loaded = false;
-
-  public static synchronized Tracer load() {
-    if (!loaded) {
-      log.info("OpenTelemetry tracer enabled with simple propagation only.");
-      OpenTelemetry otel =
-          
OpenTelemetry.propagating(ContextPropagators.create(SimplePropagator.getInstance()));
-      GlobalOpenTelemetry.set(otel);
-      loaded = true;
-    }
-    return TraceUtils.getGlobalTracer();
-  }

Review Comment:
   This basically moved into the new `OpenTelemetryConfigurator`



-- 
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