dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r1741041201
##########
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:
A possible hacky work-around would be to `get()` initially and check for
noop. If not noop, do nothing (keep current impl). If noop, create one
according to our logic and then call `resetForTest()` then `set()`. Hacky
because we'd be calling resetForTest in a production scenario.
--
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]