dsmiley commented on code in PR #3379:
URL: https://github.com/apache/solr/pull/3379#discussion_r2132810513
##########
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:
Agreed; I said the same in my OTEL Agent PR.
##########
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:
A growing concern on my mind is that CoreContainer is a kitchen sink and has
gotten out of control. That said, if you need (or makes most logical sense)
for a field on CoreContainer, really, then fine. If you can keep OTEL stuff to
another source file in some way, that would probably be ideal.
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -465,6 +480,39 @@ public CoreContainer(NodeConfig config, CoresLocator
locator, boolean asyncSolrC
new SolrNamedThreadFactory("IndexFingerprintPool"));
}
+ /**
Review Comment:
Docs are always well intentioned but here, I think this is saying way too
much. It's about as much as there is code. This is a private method; why
bother? Docs need to be maintained as well; it's not a free gift (from
whatever person/LLM wrote it).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
