mlbiscoc commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4166180079
##########
solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java:
##########
@@ -61,6 +61,21 @@ public class TraceUtils {
public static final String TAG_DB_TYPE_SOLR = "solr";
+ /** Is the OpenTelemetry Java agent present? */
+ public static final boolean OTEL_AGENT_PRESENT;
Review Comment:
Wouldn't this living in OpenTelemetryConfigurator better?
##########
solr/core/src/java/org/apache/solr/core/OpenTelemetryConfigurator.java:
##########
@@ -53,67 +55,93 @@ public abstract class OpenTelemetryConfigurator implements
NamedListInitializedP
private static volatile boolean loaded = false;
/**
- * Initializes the {@link io.opentelemetry.api.GlobalOpenTelemetry} instance
by configuring the
- * {@link io.opentelemetry.sdk.OpenTelemetrySdk} through custom plugin,
auto-configure or default
- * SDK.
+ * Initializes {@link io.opentelemetry.api.GlobalOpenTelemetry} from a
custom plugin,
+ * auto-configuration, or simple trace ID propagation. Does nothing if the
OpenTelemetry Java
+ * agent is present, since it has already done this.
*/
public static synchronized void initializeOpenTelemetrySdk(
NodeConfig cfg, SolrResourceLoader loader) {
- PluginInfo info = (cfg != null) ? cfg.getTracerConfiguratorPluginInfo() :
null;
-
- if (info != null && info.isEnabled()) {
- OpenTelemetryConfigurator.configureCustomOpenTelemetrySdk(
- loader, cfg.getTracerConfiguratorPluginInfo());
- } else if (OpenTelemetryConfigurator.shouldAutoConfigOTEL()) {
- OpenTelemetryConfigurator.autoConfigureOpenTelemetrySdk(loader);
- } else {
- OpenTelemetryConfigurator.configureOpenTelemetrySdk();
- }
- }
-
- private static void configureOpenTelemetrySdk() {
+ // synchronized & "loaded" to avoid races in tests starting Solr nodes
concurrently
if (loaded) return;
+ loaded = true;
- if (TRACE_ID_GEN_ENABLED) {
- log.info("OpenTelemetry tracer enabled with simple propagation only.");
- ExecutorUtil.addThreadLocalProvider(new ContextThreadLocalProvider());
+ if (TraceUtils.OTEL_AGENT_PRESENT) {
+ log.info("OpenTelemetry Java agent is installed; using the OpenTelemetry
it registered.");
+ } else {
+ PluginInfo info = (cfg != null) ? cfg.getTracerConfiguratorPluginInfo()
: null;
+ OpenTelemetry otel = null;
+ if (info != null && info.isEnabled()) {
+ OpenTelemetryConfigurator configurator =
+ loader.newInstance(info.className,
OpenTelemetryConfigurator.class);
+ configurator.init(info.initArgs);
+ otel = configurator.createOpenTelemetry();
Review Comment:
I think we should add a basic info log here if using custom class
configuarator. The documentation below now has a `Verifying Which Integration
Is Active` section which gives more reason to now.
--
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]
