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