janhoy commented on code in PR #1922:
URL: https://github.com/apache/solr/pull/1922#discussion_r1325056090
##########
solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java:
##########
@@ -49,17 +49,17 @@ public OtelTracerConfigurator() {
@Override
public Tracer getTracer() {
- // TODO remove reliance on global
- return GlobalOpenTelemetry.getTracer("solr");
+ return TraceUtils.getGlobalTracer();
Review Comment:
Minor: While this is correct, it feels backward for a plugin method to ask a
core class to return its instance.
##########
solr/core/src/java/org/apache/solr/core/TracerConfigurator.java:
##########
@@ -18,29 +18,78 @@
package org.apache.solr.core;
import io.opentelemetry.api.trace.Tracer;
+import java.lang.invoke.MethodHandles;
+import java.util.Locale;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.NamedList;
import org.apache.solr.util.plugin.NamedListInitializedPlugin;
import org.apache.solr.util.tracing.SimplePropagator;
import org.apache.solr.util.tracing.TraceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/** Produces a {@link Tracer} from configuration. */
public abstract class TracerConfigurator implements NamedListInitializedPlugin
{
public static final boolean TRACE_ID_GEN_ENABLED =
Boolean.parseBoolean(System.getProperty("solr.alwaysOnTraceId", "true"));
+ private static final String DEFAULT_CLASS_NAME =
+ System.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);
return configurator.getTracer();
-
- } else if (TRACE_ID_GEN_ENABLED) {
+ }
+ if (shouldAutoConfigOTEL()) {
+ return autoConfigOTEL(loader);
+ }
+ if (TRACE_ID_GEN_ENABLED) {
return SimplePropagator.load();
- } else {
- return TraceUtils.noop();
}
+ return TraceUtils.getGlobalTracer();
}
protected abstract Tracer getTracer();
+
+ private static Tracer autoConfigOTEL(SolrResourceLoader loader) {
+ try {
+ TracerConfigurator configurator =
+ loader.newInstance(DEFAULT_CLASS_NAME, TracerConfigurator.class);
+ configurator.init(new NamedList<>());
+ return configurator.getTracer();
+ } catch (SolrException e) {
+ log.error(
+ "Unable to auto-config OpenTelemetry with class {}. Make sure you
have enabled the 'opentelemetry' module",
+ DEFAULT_CLASS_NAME,
+ e);
+ }
+ return TraceUtils.getGlobalTracer();
+ }
+
+ /**
+ * best effort way to determine if we should attempt to init OTEL from
system properties.
Review Comment:
```suggestion
* Best effort way to determine if we should attempt to init OTEL from
system properties.
```
##########
solr/core/src/java/org/apache/solr/core/TracerConfigurator.java:
##########
@@ -18,29 +18,78 @@
package org.apache.solr.core;
import io.opentelemetry.api.trace.Tracer;
+import java.lang.invoke.MethodHandles;
+import java.util.Locale;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.NamedList;
import org.apache.solr.util.plugin.NamedListInitializedPlugin;
import org.apache.solr.util.tracing.SimplePropagator;
import org.apache.solr.util.tracing.TraceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/** Produces a {@link Tracer} from configuration. */
public abstract class TracerConfigurator implements NamedListInitializedPlugin
{
public static final boolean TRACE_ID_GEN_ENABLED =
Boolean.parseBoolean(System.getProperty("solr.alwaysOnTraceId", "true"));
+ private static final String DEFAULT_CLASS_NAME =
+ System.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);
return configurator.getTracer();
-
- } else if (TRACE_ID_GEN_ENABLED) {
+ }
+ if (shouldAutoConfigOTEL()) {
+ return autoConfigOTEL(loader);
+ }
+ if (TRACE_ID_GEN_ENABLED) {
return SimplePropagator.load();
- } else {
- return TraceUtils.noop();
}
+ return TraceUtils.getGlobalTracer();
}
protected abstract Tracer getTracer();
+
+ private static Tracer autoConfigOTEL(SolrResourceLoader loader) {
+ try {
+ TracerConfigurator configurator =
+ loader.newInstance(DEFAULT_CLASS_NAME, TracerConfigurator.class);
+ configurator.init(new NamedList<>());
+ return configurator.getTracer();
+ } catch (SolrException e) {
+ log.error(
+ "Unable to auto-config OpenTelemetry with class {}. Make sure you
have enabled the 'opentelemetry' module",
+ DEFAULT_CLASS_NAME,
+ e);
+ }
+ return TraceUtils.getGlobalTracer();
+ }
+
+ /**
+ * best effort way to determine if we should attempt to init OTEL from
system properties.
+ *
+ * @return true if OTEL should be init
+ */
+ static boolean shouldAutoConfigOTEL() {
+ boolean isSdkDisabled =
Boolean.parseBoolean(getConfig("OTEL_SDK_DISABLED"));
+ if (isSdkDisabled) {
+ return false;
+ }
+ return getConfig("OTEL_SERVICE_NAME") != null;
+ }
+
+ private static String getConfig(String envName) {
+ String envValue = System.getenv().get(envName);
+ String sysName = envName.toLowerCase(Locale.ROOT).replace("_", ".");
+ String sysValue = System.getProperty(sysName);
+ return sysValue != null ? sysValue : envValue;
Review Comment:
This is duplicating `OtelTracerConfigurator#getEnvOrSysprop`. Perhaps you
can extract some functionality into a common static utility?
--
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]
