You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "dsmiley (via GitHub)" <gi...@apache.org> on 2024/09/02 05:22:36 UTC
[PR] SOLR-17432: Allow OTel Java Agent [solr]
dsmiley opened a new pull request, #2687:
URL: https://github.com/apache/solr/pull/2687
https://issues.apache.org/jira/browse/SOLR-17432
Admittedly there's a lot going on here; the PR is more of a refactoring / minor improvements that also happens to support the Otel agent too, as it's a small aspect. I can separate this PR. A number of things were gnawing at me, which I started to deal with.
API Change: TracerConfigurator subclasses now create an OpenTelemetry not a Tracer. Since we depend on GlobalOpenTelemetry, if a configurator were to return some custom Tracer thing (independent of an OpenTelemetry), it's not clear how we would install it into a GlobalOpenTelemetry. Maybe possible (?) but doesn't seem a good direction. We could stop our use of GlobalOpenTelemetry easily as it's only used in one spot that could actually get the Tracer from elsewhere. Maybe we should do that anyway, but nonetheless installing it is something we should continue to do.
Refactorings / changes:
* removed all tracing config from MiniSolrCloudCluster
* SolrTestCase resets tracing now
* Only TracerConfigurator should call GlobalOpenTelemetry.set (removed from SimplePropagator & OtelTracerConfigurator) and it does so in exactly one spot.
* Tests examining spans now use OpenTelemetryRule. Removed CustomTestOtelTracerConfigurator.java
todo: test Custom TracerConfigurator ?
Do we even have a test actually using OtelTracerConfigurator.java?
Admittedly there's no test here for the Agent approach. Would need a bats test to do right, and it'd need to go download a 20MB jar. I tested it manually when running a test (to debug a test) and I have used it in Solr 9.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2066398299
##########
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
Review Comment:
not sure `synchronized` is needed. the trouble I had seen were from tests trying to race to init this and failing because otel was already set. I think it was happening when setting up a cluster with a few nodes, but I need to revisit some of this to check.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2066367300
##########
solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java:
##########
@@ -39,24 +39,22 @@
import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.util.tracing.TraceUtils;
-import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
public class TestDistributedTracing extends SolrCloudTestCase {
private static final String COLLECTION = "collection1";
+ @ClassRule public static OpenTelemetryRule otelRule = OpenTelemetryRule.create();
Review Comment:
this is pretty cool, I did not know this existed.
##########
solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/BasicAuthIntegrationTracingTest.java:
##########
@@ -31,23 +33,23 @@
import org.apache.solr.security.BasicAuthPlugin;
import org.apache.solr.util.SecurityJson;
import org.apache.solr.util.tracing.TraceUtils;
-import org.junit.AfterClass;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
public class BasicAuthIntegrationTracingTest extends SolrCloudTestCase {
private static final String COLLECTION = "collection1";
+ @ClassRule public static OpenTelemetryRule otelRule = OpenTelemetryRule.create();
Review Comment:
this is pretty cool, I did not know this existed.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4168702275
##########
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:
> (me) I so wish GlobalOpenTelemetry had a way to know if it's been set already
It's there now, as of v1.57! This PR is now updated to use it. It should reduce possible confusion in logs of tests that are multi-node.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4168720490
##########
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:
good idea; done.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4168850217
##########
solr/solr-ref-guide/modules/deployment-guide/pages/opentelemetry.adoc:
##########
@@ -0,0 +1,188 @@
+= OpenTelemetry
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+https://opentelemetry.io/[OpenTelemetry] ("OTEL") is the vendor-neutral standard that Solr uses.
+Solr uses its Java APIs for Metrics and Tracing (not logging), and to emit observability data via OTLP (if desired).
+This page covers how OpenTelemetry gets into a Solr JVM and how it is configured; what Solr emits through it is covered on the pages for each signal.
+
+== Traces and Metrics
+
+Solr uses OpenTelemetry for two signals, configured independently:
+
+xref:distributed-tracing.adoc[]::
+Spans describing requests as they move between nodes.
+Exported by the OpenTelemetry SDK itself, so it is configured with the standard `OTEL_*` environment variables (or `otel.*` system properties).
+
+xref:metrics-reporting.adoc#otlp[Metrics over OTLP]::
+Solr's own metrics, pushed periodically.
+Solr builds and owns the meter providers these come from -- it has to, so that the `/metrics` endpoints can read them -- so they are configured with `solr.metrics.otlp*` properties, with their own endpoint and protocol settings.
+
+Only tracing goes through `GlobalOpenTelemetry`.
+A Java agent therefore takes over tracing but not Solr's metrics, which keep their own configuration and exporter either way.
+
+NOTE: The `opentelemetry` module turns the autoconfigured SDK's metrics and logs exporters off (`otel.metrics.exporter=none`, `otel.logs.exporter=none`) so they cannot duplicate or conflict with the above.
+Setting `OTEL_METRICS_EXPORTER` or `OTEL_LOGS_EXPORTER` has no effect and logs a warning.
+
+== Integration Options
+
+[cols="2,5",options="header"]
+|===
+| Option | When to use it
+
+| <<opentelemetry-java-agent,OpenTelemetry Java agent>>
+| You want automatic instrumentation of third-party libraries (e.g. for backups), `@WithSpan` support, wide exporter format support, or a vendor's agent distribution. Requires a JVM argument and an external special JAR that you must install.
+
+| <<opentelemetry-module,`opentelemetry` module>>
+| You want OTLP metrics push, or prefer the convenience of using software that ships with Solr.
+
+| <<custom-configurator,Custom configurator>>
+| Neither of the above fits, for example an OTEL SDK setup your organization standardizes on.
+
+| _(none)_
+| Solr falls back to xref:distributed-tracing.adoc#always-on-trace-id-generation[always-on trace ID generation], which propagates a trace ID but exports nothing.
+|===
+
+Solr resolves these in a fixed order, using the first that applies: a Java agent, then a `<tracerConfig>` in `solr.xml`, then auto-activation from `OTEL_SERVICE_NAME`, then always-on trace ID generation.
+A Java agent always wins; when one is present Solr configures nothing and `<tracerConfig>` is ignored.
+
+== OpenTelemetry Java Agent
+
+Run Solr with the https://opentelemetry.io/docs/zero-code/java/agent/[OpenTelemetry Java agent] and it takes over OpenTelemetry entirely:
+
+[source,bash]
+----
+SOLR_OPTS="-javaagent:/path/to/opentelemetry-javaagent.jar"
+----
+
+The agent loads its dependencies in an isolated classloader, so it cannot conflict with Solr's. The `opentelemetry` module should not be enabled.
+All configuration is the agent's; see its documentation.
+
+Solr's own instrumentation continues to work as long as the agent's `opentelemetry-api` instrumentation stays enabled.
+That matters if you disable instrumentation by default in order to opt in selectively:
+
+[source,properties]
+----
+otel.instrumentation.common.default-enabled=false
+otel.instrumentation.opentelemetry-api.enabled=true
+otel.instrumentation.opentelemetry-instrumentation-annotations.enabled=true
+----
+
+Agents from observability vendors generally work the same way, provided they register an OpenTelemetry `GlobalOpenTelemetry` instance.
+
+== OpenTelemetry Module
+
+The `opentelemetry` xref:configuration-guide:solr-modules.adoc[module] bundles the OpenTelemetry SDK and an OTLP exporter.
+Enable it with either the system property `-Dsolr.modules=opentelemetry` or the environment variable `SOLR_MODULES=opentelemetry`.
+
+Then activate it in one of two ways.
+Either declare it in `solr.xml`:
+
+[source,xml]
+----
+<solr>
+ <tracerConfig name="tracerConfig" class="org.apache.solr.opentelemetry.OtelTracerConfigurator"/>
+</solr>
+----
+
+Or, without touching `solr.xml`, set the system property `otel.service.name` or the environment variable `OTEL_SERVICE_NAME`; Solr then loads the module's configurator automatically.
+Setting `OTEL_SDK_DISABLED=true` suppresses that auto-activation.
+
+=== Configuration
+
+The SDK is configured through environment variables or Java system properties -- see https://opentelemetry.io/docs/reference/specification/sdk-environment-variables/[OTEL SDK Environment Variables] and https://github.com/open-telemetry/opentelemetry-java/blob/v{dep-version-opentelemetry}/sdk-extensions/autoconfigure/README.md[Java SDK Autoconfigure].
+The effective defaults, some set by Solr and the rest by the SDK, are:
+
+[source,bash]
+----
+OTEL_SDK_DISABLED=false
+OTEL_SERVICE_NAME=solr
+OTEL_TRACES_EXPORTER=otlp
+OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf
+OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
+OTEL_TRACES_SAMPLER=parentbased_always_on
+OTEL_PROPAGATORS=tracecontext,baggage
+----
+
+So out of the box, traces go to a collector on localhost using https://opentelemetry.io/docs/reference/specification/protocol/[OTLP] over HTTP (Protobuf), and trace IDs propagate using https://www.w3.org/TR/trace-context/[W3C TraceContext].
+
+To send to a remote https://opentelemetry.io/docs/collector/[OTEL Collector]:
+
+[source,bash]
+----
+OTEL_EXPORTER_OTLP_ENDPOINT=http://my-remote-collector:4318
+----
+
+The equivalent with system properties:
+
+[source,bash]
+----
+SOLR_OPTS=-Dotel.exporter.otlp.endpoint=http://my-remote-collector:4318
+----
+
+=== Exporters and Transports
+
+The module ships a deliberately minimal set of dependencies: OTLP over HTTP, and nothing else.
+Anything beyond that means adding JARs yourself (or use the Java agent).
+
+gRPC:: Add the required gRPC and Netty JARs to the module's lib directory, then set `OTEL_EXPORTER_OTLP_PROTOCOL=grpc`.
+
+Other backends:: Exporters such as Jaeger and Zipkin are supported by the SDK but not shipped. Add the exporter JAR(s) to `$SOLR_TIP/lib/` and configure the exporter, for example:
++
+[source,bash]
+----
+OTEL_TRACES_EXPORTER=zipkin
+OTEL_EXPORTER_ZIPKIN_ENDPOINT=http://localhost:9411/api/v2/spans
+----
+
+A Java agent avoids the need to add more JARs and risking version conflicts, since it carries its own exporters in an isolated classloader.
+
+== Custom Configurator
+
+`org.apache.solr.core.OpenTelemetryConfigurator` is the plugin API behind `<tracerConfig>`.
+A subclass implements `createOpenTelemetry()`, returning the `io.opentelemetry.api.OpenTelemetry` that Solr will install as `GlobalOpenTelemetry`; implementations must not install it themselves.
+Configure it exactly as the module's own configurator is configured, with your class name:
+
+[source,xml]
+----
+<solr>
+ <tracerConfig name="tracerConfig" class="com.example.MyOpenTelemetryConfigurator"/>
+</solr>
+----
+
+NOTE: The `<tracerConfig>` element name and the module's `OtelTracerConfigurator` class name predate OpenTelemetry covering more than tracing; both now configure OpenTelemetry as a whole.
+
+== Verifying Which Integration Is Active
+
+Each mode logs a distinct line at startup, which is the quickest way to confirm what Solr actually picked up:
+
+[cols="3,2",options="header"]
+|===
+| Log message | Meaning
+
+| `OpenTelemetry Java agent is installed; using the OpenTelemetry it registered.`
+| A Java agent was detected; Solr configured nothing.
+
+| `OpenTelemetry loaded via auto configuration.`
+| The module's configurator was loaded, via `<tracerConfig>` or `OTEL_SERVICE_NAME`.
Review Comment:
```suggestion
| `OpenTelemetry loaded via <class name>`
| The configurator named in `<tracerConfig>` was loaded.
| `OpenTelemetry loaded via auto configuration.`
| `OTEL_SERVICE_NAME` is set, so the `opentelemetry` module's configurator was loaded automatically.
```
##########
solr/solr-ref-guide/modules/deployment-guide/pages/opentelemetry.adoc:
##########
@@ -0,0 +1,188 @@
+= OpenTelemetry
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+https://opentelemetry.io/[OpenTelemetry] ("OTEL") is the vendor-neutral standard that Solr uses.
+Solr uses its Java APIs for Metrics and Tracing (not logging), and to emit observability data via OTLP (if desired).
+This page covers how OpenTelemetry gets into a Solr JVM and how it is configured; what Solr emits through it is covered on the pages for each signal.
+
+== Traces and Metrics
+
+Solr uses OpenTelemetry for two signals, configured independently:
+
+xref:distributed-tracing.adoc[]::
+Spans describing requests as they move between nodes.
+Exported by the OpenTelemetry SDK itself, so it is configured with the standard `OTEL_*` environment variables (or `otel.*` system properties).
+
+xref:metrics-reporting.adoc#otlp[Metrics over OTLP]::
+Solr's own metrics, pushed periodically.
+Solr builds and owns the meter providers these come from -- it has to, so that the `/metrics` endpoints can read them -- so they are configured with `solr.metrics.otlp*` properties, with their own endpoint and protocol settings.
+
+Only tracing goes through `GlobalOpenTelemetry`.
+A Java agent therefore takes over tracing but not Solr's metrics, which keep their own configuration and exporter either way.
+
+NOTE: The `opentelemetry` module turns the autoconfigured SDK's metrics and logs exporters off (`otel.metrics.exporter=none`, `otel.logs.exporter=none`) so they cannot duplicate or conflict with the above.
+Setting `OTEL_METRICS_EXPORTER` or `OTEL_LOGS_EXPORTER` has no effect and logs a warning.
+
+== Integration Options
+
+[cols="2,5",options="header"]
+|===
+| Option | When to use it
+
+| <<opentelemetry-java-agent,OpenTelemetry Java agent>>
+| You want automatic instrumentation of third-party libraries (e.g. for backups), `@WithSpan` support, wide exporter format support, or a vendor's agent distribution. Requires a JVM argument and an external special JAR that you must install.
+
+| <<opentelemetry-module,`opentelemetry` module>>
+| You want OTLP metrics push, or prefer the convenience of using software that ships with Solr.
+
+| <<custom-configurator,Custom configurator>>
+| Neither of the above fits, for example an OTEL SDK setup your organization standardizes on.
+
+| _(none)_
+| Solr falls back to xref:distributed-tracing.adoc#always-on-trace-id-generation[always-on trace ID generation], which propagates a trace ID but exports nothing.
+|===
+
+Solr resolves these in a fixed order, using the first that applies: a Java agent, then a `<tracerConfig>` in `solr.xml`, then auto-activation from `OTEL_SERVICE_NAME`, then always-on trace ID generation.
+A Java agent always wins; when one is present Solr configures nothing and `<tracerConfig>` is ignored.
+
+== OpenTelemetry Java Agent
+
+Run Solr with the https://opentelemetry.io/docs/zero-code/java/agent/[OpenTelemetry Java agent] and it takes over OpenTelemetry entirely:
+
+[source,bash]
+----
+SOLR_OPTS="-javaagent:/path/to/opentelemetry-javaagent.jar"
+----
+
+The agent loads its dependencies in an isolated classloader, so it cannot conflict with Solr's. The `opentelemetry` module should not be enabled.
+All configuration is the agent's; see its documentation.
+
+Solr's own instrumentation continues to work as long as the agent's `opentelemetry-api` instrumentation stays enabled.
+That matters if you disable instrumentation by default in order to opt in selectively:
+
+[source,properties]
+----
+otel.instrumentation.common.default-enabled=false
+otel.instrumentation.opentelemetry-api.enabled=true
+otel.instrumentation.opentelemetry-instrumentation-annotations.enabled=true
+----
+
+Agents from observability vendors generally work the same way, provided they register an OpenTelemetry `GlobalOpenTelemetry` instance.
+
+== OpenTelemetry Module
+
+The `opentelemetry` xref:configuration-guide:solr-modules.adoc[module] bundles the OpenTelemetry SDK and an OTLP exporter.
+Enable it with either the system property `-Dsolr.modules=opentelemetry` or the environment variable `SOLR_MODULES=opentelemetry`.
+
+Then activate it in one of two ways.
+Either declare it in `solr.xml`:
+
+[source,xml]
+----
+<solr>
+ <tracerConfig name="tracerConfig" class="org.apache.solr.opentelemetry.OtelTracerConfigurator"/>
+</solr>
+----
+
+Or, without touching `solr.xml`, set the system property `otel.service.name` or the environment variable `OTEL_SERVICE_NAME`; Solr then loads the module's configurator automatically.
+Setting `OTEL_SDK_DISABLED=true` suppresses that auto-activation.
+
+=== Configuration
+
+The SDK is configured through environment variables or Java system properties -- see https://opentelemetry.io/docs/reference/specification/sdk-environment-variables/[OTEL SDK Environment Variables] and https://github.com/open-telemetry/opentelemetry-java/blob/v{dep-version-opentelemetry}/sdk-extensions/autoconfigure/README.md[Java SDK Autoconfigure].
+The effective defaults, some set by Solr and the rest by the SDK, are:
+
+[source,bash]
+----
+OTEL_SDK_DISABLED=false
+OTEL_SERVICE_NAME=solr
+OTEL_TRACES_EXPORTER=otlp
+OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf
+OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
+OTEL_TRACES_SAMPLER=parentbased_always_on
+OTEL_PROPAGATORS=tracecontext,baggage
+----
+
+So out of the box, traces go to a collector on localhost using https://opentelemetry.io/docs/reference/specification/protocol/[OTLP] over HTTP (Protobuf), and trace IDs propagate using https://www.w3.org/TR/trace-context/[W3C TraceContext].
+
+To send to a remote https://opentelemetry.io/docs/collector/[OTEL Collector]:
+
+[source,bash]
+----
+OTEL_EXPORTER_OTLP_ENDPOINT=http://my-remote-collector:4318
+----
+
+The equivalent with system properties:
+
+[source,bash]
+----
+SOLR_OPTS=-Dotel.exporter.otlp.endpoint=http://my-remote-collector:4318
+----
+
+=== Exporters and Transports
+
+The module ships a deliberately minimal set of dependencies: OTLP over HTTP, and nothing else.
+Anything beyond that means adding JARs yourself (or use the Java agent).
+
+gRPC:: Add the required gRPC and Netty JARs to the module's lib directory, then set `OTEL_EXPORTER_OTLP_PROTOCOL=grpc`.
+
+Other backends:: Exporters such as Jaeger and Zipkin are supported by the SDK but not shipped. Add the exporter JAR(s) to `$SOLR_TIP/lib/` and configure the exporter, for example:
++
+[source,bash]
+----
+OTEL_TRACES_EXPORTER=zipkin
+OTEL_EXPORTER_ZIPKIN_ENDPOINT=http://localhost:9411/api/v2/spans
+----
+
+A Java agent avoids the need to add more JARs and risking version conflicts, since it carries its own exporters in an isolated classloader.
+
+== Custom Configurator
+
+`org.apache.solr.core.OpenTelemetryConfigurator` is the plugin API behind `<tracerConfig>`.
+A subclass implements `createOpenTelemetry()`, returning the `io.opentelemetry.api.OpenTelemetry` that Solr will install as `GlobalOpenTelemetry`; implementations must not install it themselves.
+Configure it exactly as the module's own configurator is configured, with your class name:
+
+[source,xml]
+----
+<solr>
+ <tracerConfig name="tracerConfig" class="com.example.MyOpenTelemetryConfigurator"/>
+</solr>
+----
+
+NOTE: The `<tracerConfig>` element name and the module's `OtelTracerConfigurator` class name predate OpenTelemetry covering more than tracing; both now configure OpenTelemetry as a whole.
+
+== Verifying Which Integration Is Active
+
+Each mode logs a distinct line at startup, which is the quickest way to confirm what Solr actually picked up:
+
+[cols="3,2",options="header"]
+|===
+| Log message | Meaning
+
+| `OpenTelemetry Java agent is installed; using the OpenTelemetry it registered.`
+| A Java agent was detected; Solr configured nothing.
+
+| `OpenTelemetry loaded via auto configuration.`
+| The module's configurator was loaded, via `<tracerConfig>` or `OTEL_SERVICE_NAME`.
Review Comment:
Just to add the new log to the doc
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-2351370178
When I run the agent, I provide an Otel properties file with: (an excerpt)
```
otel.metrics.exporter=none
otel.logs.exporter=none
otel.instrumentation.common.default-enabled=false
otel.instrumentation.opentelemetry-api.enabled=true
otel.instrumentation.opentelemetry-instrumentation-annotations.enabled=true
otel.instrumentation.aws-sdk.enabled=true
otel.instrumentation.java-http-client.enabled=true
```
Notice I disable auto-instrumentation by default and selectively enable the specific instrumentations useful to me. This works with Solr's manual instrumentation of itself as well because of enabling "opentelemetry-api". All this is documented quite well in Otel.
I don't see how this PR would have unintentional changes on existing traces/spans since they are tested and the PR doesn't change them. Any way, I'm happy to share a screenshot later.
This PR misses documentation to let users know they don't have to enable the opentelemetry module just to use opentelemetry :-). (an aside -- nor do we tell users they don't have to have an HDFS cluster to make use of the HDFS Directory). Also I'd like to somewhere document a tip on how to use tracing during development / debugging of a unit test.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4149877894
##########
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:
I don't believe there's a race: there is a `loaded` boolean, and`initializeOpenTelemetrySdk` is synchronized.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-5923984514
FWIW this is deployed where I work.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r1740338583
##########
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
Review Comment:
this is the one spot where we call this. Previously it was in multiple places which was confusing and hard to reason about.
##########
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:
Note that its common tests will log "OpenTelemetry loaded with simple propagation only." but then log immediately after "GlobalOpenTelemetry was already initialized (a Java agent?); using that" if the test uses more than one CoreContainer (Solr node). I so wish GlobalOpenTelemetry had a way to know if it's been set already.
##########
solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/BasicAuthIntegrationTracingTest.java:
##########
@@ -31,23 +33,23 @@
import org.apache.solr.security.BasicAuthPlugin;
import org.apache.solr.util.SecurityJson;
import org.apache.solr.util.tracing.TraceUtils;
-import org.junit.AfterClass;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
public class BasicAuthIntegrationTracingTest extends SolrCloudTestCase {
private static final String COLLECTION = "collection1";
+ @ClassRule public static OpenTelemetryRule otelRule = OpenTelemetryRule.create();
Review Comment:
I was about to write an equivalent of OpenTelemetryRule when I found that it already exists :-)
FYI note it has docs that it doesn't work in a ClassRule yet I see it works here.
##########
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");
+ }
}
- if (TRACE_ID_GEN_ENABLED) {
- ExecutorUtil.addThreadLocalProvider(new ContextThreadLocalProvider());
- return SimplePropagator.load();
- }
- return TraceUtils.getGlobalTracer();
+
+ ExecutorUtil.addThreadLocalProvider(new ContextThreadLocalProvider());
+ return TraceUtils.getGlobalTracer(); // GlobalOpenTelemetry.get...
}
- protected abstract Tracer getTracer();
+ protected abstract OpenTelemetry createOtel();
Review Comment:
this is the API change
##########
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");
+ }
}
- if (TRACE_ID_GEN_ENABLED) {
- ExecutorUtil.addThreadLocalProvider(new ContextThreadLocalProvider());
- return SimplePropagator.load();
- }
- return TraceUtils.getGlobalTracer();
+
+ ExecutorUtil.addThreadLocalProvider(new ContextThreadLocalProvider());
Review Comment:
Now called in exactly one spot, which I find easier to read than before
##########
solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java:
##########
@@ -49,14 +48,13 @@ public OtelTracerConfigurator() {
}
@Override
- public Tracer getTracer() {
- return TraceUtils.getGlobalTracer();
Review Comment:
A needless GlobalOpenTelemetry reference
##########
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
Review Comment:
I didn't personally encounter an issue here but saw existing comments (from Alex?) that suggested to me making this synchronized would be helpful. I don't think there's a true issue; ultimately GlobalOpenTelemetry will be set exactly once so it doesn't much matter if others try and race to do so.
##########
solr/modules/opentelemetry/src/test-files/solr/solr.xml:
##########
@@ -34,8 +34,6 @@
<int name="connTimeout">${connTimeout:15000}</int>
</shardHandlerFactory>
- <tracerConfig name="tracerConfig" class="org.apache.solr.opentelemetry.CustomTestOtelTracerConfigurator" />
Review Comment:
Perhaps this class could return but only for a test explicitly testing that tracerConfig works and in a separate solr.xml for exactly that test as well (not for the other tracing tests). Not to be used for other tests or trying to collect spans for examination.
##########
solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java:
##########
@@ -49,14 +48,13 @@ public OtelTracerConfigurator() {
}
@Override
- public Tracer getTracer() {
- return TraceUtils.getGlobalTracer();
+ public void init(NamedList<?> args) {
+ prepareConfiguration(args);
}
@Override
- public void init(NamedList<?> args) {
- prepareConfiguration(args);
- AutoConfiguredOpenTelemetrySdk.initialize();
+ public OpenTelemetry createOtel() {
+ return AutoConfiguredOpenTelemetrySdk.builder().build().getOpenTelemetrySdk();
Review Comment:
notice the change to *not* call initialize. That method arranges for GlobalOpenTelemetry to be set. I only want that set in TracerConfigurator.
##########
solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/CustomTestOtelTracerConfigurator.java:
##########
Review Comment:
this thing was strange; didn't need to exist, at least not as it existed
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-2480856141
This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4149850630
##########
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
Review Comment:
It's already set in CoreContainer constructor `OpenTelemetryConfigurator.initializeOpenTelemetrySdk(cfg, loader);` -- but I will move it a few lines higher and add a comment.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-5771476991
Dusted this off with Claude; things have been renamed/changed since when I last touched this. Also, I updated the ref guide to have a dedicated opentelemetry page to avoid re-discussing aspects in metrics & tracing.
It's a bit of a misnomer to title this as "Allow OTEL Java Agent" because it's already allowed, albeit with an ugly ClassNotFoundException that can be ignored. Overall this makes it more first-class / documented.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-2778987448
I very much want to get back to this for Solr 10... need to maybe reformulate what's happening into multiple things.
CC @mlbiscoc
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley merged PR #2687:
URL: https://github.com/apache/solr/pull/2687
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4168718797
##########
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:
You are right; moved. In addition, I added a check on the 2 methods there that reference `GlobalOpenTelemetry` to ensure that our initialization has happened, thus catching an accidental too-early use of getTracer, perhaps in some hypothetical NodeConfig or ServletFilter.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4168850217
##########
solr/solr-ref-guide/modules/deployment-guide/pages/opentelemetry.adoc:
##########
@@ -0,0 +1,188 @@
+= OpenTelemetry
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+https://opentelemetry.io/[OpenTelemetry] ("OTEL") is the vendor-neutral standard that Solr uses.
+Solr uses its Java APIs for Metrics and Tracing (not logging), and to emit observability data via OTLP (if desired).
+This page covers how OpenTelemetry gets into a Solr JVM and how it is configured; what Solr emits through it is covered on the pages for each signal.
+
+== Traces and Metrics
+
+Solr uses OpenTelemetry for two signals, configured independently:
+
+xref:distributed-tracing.adoc[]::
+Spans describing requests as they move between nodes.
+Exported by the OpenTelemetry SDK itself, so it is configured with the standard `OTEL_*` environment variables (or `otel.*` system properties).
+
+xref:metrics-reporting.adoc#otlp[Metrics over OTLP]::
+Solr's own metrics, pushed periodically.
+Solr builds and owns the meter providers these come from -- it has to, so that the `/metrics` endpoints can read them -- so they are configured with `solr.metrics.otlp*` properties, with their own endpoint and protocol settings.
+
+Only tracing goes through `GlobalOpenTelemetry`.
+A Java agent therefore takes over tracing but not Solr's metrics, which keep their own configuration and exporter either way.
+
+NOTE: The `opentelemetry` module turns the autoconfigured SDK's metrics and logs exporters off (`otel.metrics.exporter=none`, `otel.logs.exporter=none`) so they cannot duplicate or conflict with the above.
+Setting `OTEL_METRICS_EXPORTER` or `OTEL_LOGS_EXPORTER` has no effect and logs a warning.
+
+== Integration Options
+
+[cols="2,5",options="header"]
+|===
+| Option | When to use it
+
+| <<opentelemetry-java-agent,OpenTelemetry Java agent>>
+| You want automatic instrumentation of third-party libraries (e.g. for backups), `@WithSpan` support, wide exporter format support, or a vendor's agent distribution. Requires a JVM argument and an external special JAR that you must install.
+
+| <<opentelemetry-module,`opentelemetry` module>>
+| You want OTLP metrics push, or prefer the convenience of using software that ships with Solr.
+
+| <<custom-configurator,Custom configurator>>
+| Neither of the above fits, for example an OTEL SDK setup your organization standardizes on.
+
+| _(none)_
+| Solr falls back to xref:distributed-tracing.adoc#always-on-trace-id-generation[always-on trace ID generation], which propagates a trace ID but exports nothing.
+|===
+
+Solr resolves these in a fixed order, using the first that applies: a Java agent, then a `<tracerConfig>` in `solr.xml`, then auto-activation from `OTEL_SERVICE_NAME`, then always-on trace ID generation.
+A Java agent always wins; when one is present Solr configures nothing and `<tracerConfig>` is ignored.
+
+== OpenTelemetry Java Agent
+
+Run Solr with the https://opentelemetry.io/docs/zero-code/java/agent/[OpenTelemetry Java agent] and it takes over OpenTelemetry entirely:
+
+[source,bash]
+----
+SOLR_OPTS="-javaagent:/path/to/opentelemetry-javaagent.jar"
+----
+
+The agent loads its dependencies in an isolated classloader, so it cannot conflict with Solr's. The `opentelemetry` module should not be enabled.
+All configuration is the agent's; see its documentation.
+
+Solr's own instrumentation continues to work as long as the agent's `opentelemetry-api` instrumentation stays enabled.
+That matters if you disable instrumentation by default in order to opt in selectively:
+
+[source,properties]
+----
+otel.instrumentation.common.default-enabled=false
+otel.instrumentation.opentelemetry-api.enabled=true
+otel.instrumentation.opentelemetry-instrumentation-annotations.enabled=true
+----
+
+Agents from observability vendors generally work the same way, provided they register an OpenTelemetry `GlobalOpenTelemetry` instance.
+
+== OpenTelemetry Module
+
+The `opentelemetry` xref:configuration-guide:solr-modules.adoc[module] bundles the OpenTelemetry SDK and an OTLP exporter.
+Enable it with either the system property `-Dsolr.modules=opentelemetry` or the environment variable `SOLR_MODULES=opentelemetry`.
+
+Then activate it in one of two ways.
+Either declare it in `solr.xml`:
+
+[source,xml]
+----
+<solr>
+ <tracerConfig name="tracerConfig" class="org.apache.solr.opentelemetry.OtelTracerConfigurator"/>
+</solr>
+----
+
+Or, without touching `solr.xml`, set the system property `otel.service.name` or the environment variable `OTEL_SERVICE_NAME`; Solr then loads the module's configurator automatically.
+Setting `OTEL_SDK_DISABLED=true` suppresses that auto-activation.
+
+=== Configuration
+
+The SDK is configured through environment variables or Java system properties -- see https://opentelemetry.io/docs/reference/specification/sdk-environment-variables/[OTEL SDK Environment Variables] and https://github.com/open-telemetry/opentelemetry-java/blob/v{dep-version-opentelemetry}/sdk-extensions/autoconfigure/README.md[Java SDK Autoconfigure].
+The effective defaults, some set by Solr and the rest by the SDK, are:
+
+[source,bash]
+----
+OTEL_SDK_DISABLED=false
+OTEL_SERVICE_NAME=solr
+OTEL_TRACES_EXPORTER=otlp
+OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf
+OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
+OTEL_TRACES_SAMPLER=parentbased_always_on
+OTEL_PROPAGATORS=tracecontext,baggage
+----
+
+So out of the box, traces go to a collector on localhost using https://opentelemetry.io/docs/reference/specification/protocol/[OTLP] over HTTP (Protobuf), and trace IDs propagate using https://www.w3.org/TR/trace-context/[W3C TraceContext].
+
+To send to a remote https://opentelemetry.io/docs/collector/[OTEL Collector]:
+
+[source,bash]
+----
+OTEL_EXPORTER_OTLP_ENDPOINT=http://my-remote-collector:4318
+----
+
+The equivalent with system properties:
+
+[source,bash]
+----
+SOLR_OPTS=-Dotel.exporter.otlp.endpoint=http://my-remote-collector:4318
+----
+
+=== Exporters and Transports
+
+The module ships a deliberately minimal set of dependencies: OTLP over HTTP, and nothing else.
+Anything beyond that means adding JARs yourself (or use the Java agent).
+
+gRPC:: Add the required gRPC and Netty JARs to the module's lib directory, then set `OTEL_EXPORTER_OTLP_PROTOCOL=grpc`.
+
+Other backends:: Exporters such as Jaeger and Zipkin are supported by the SDK but not shipped. Add the exporter JAR(s) to `$SOLR_TIP/lib/` and configure the exporter, for example:
++
+[source,bash]
+----
+OTEL_TRACES_EXPORTER=zipkin
+OTEL_EXPORTER_ZIPKIN_ENDPOINT=http://localhost:9411/api/v2/spans
+----
+
+A Java agent avoids the need to add more JARs and risking version conflicts, since it carries its own exporters in an isolated classloader.
+
+== Custom Configurator
+
+`org.apache.solr.core.OpenTelemetryConfigurator` is the plugin API behind `<tracerConfig>`.
+A subclass implements `createOpenTelemetry()`, returning the `io.opentelemetry.api.OpenTelemetry` that Solr will install as `GlobalOpenTelemetry`; implementations must not install it themselves.
+Configure it exactly as the module's own configurator is configured, with your class name:
+
+[source,xml]
+----
+<solr>
+ <tracerConfig name="tracerConfig" class="com.example.MyOpenTelemetryConfigurator"/>
+</solr>
+----
+
+NOTE: The `<tracerConfig>` element name and the module's `OtelTracerConfigurator` class name predate OpenTelemetry covering more than tracing; both now configure OpenTelemetry as a whole.
+
+== Verifying Which Integration Is Active
+
+Each mode logs a distinct line at startup, which is the quickest way to confirm what Solr actually picked up:
+
+[cols="3,2",options="header"]
+|===
+| Log message | Meaning
+
+| `OpenTelemetry Java agent is installed; using the OpenTelemetry it registered.`
+| A Java agent was detected; Solr configured nothing.
+
+| `OpenTelemetry loaded via auto configuration.`
+| The module's configurator was loaded, via `<tracerConfig>` or `OTEL_SERVICE_NAME`.
Review Comment:
```suggestion
| `OpenTelemetry loaded via <class name>`
| The configurator named in `<tracerConfig>` was loaded.
| `OpenTelemetry loaded via auto configuration.`
| The module's configurator was loaded, via `<tracerConfig>` or `OTEL_SERVICE_NAME`.
```
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-2838883757
@dsmiley is this something you are still looking at? I will try to find time to review over the next weeks.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-2351373380
I'm a little unhappy with keeping TracerConfigurator named this while having it return an OpenTelemetry. Really; we're embracing OpenTelemetry here. I would love to use OpenTelemetry for metrics eventually, to be honest.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2070218457
##########
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:
coming back to this. not sure what your plans are, but I will reshare this approach I had in the `SimplePropagator`
https://github.com/apache/solr/pull/2687/files#diff-e61b6fa689b156777ba2ecb37be366c4c312199302a2c90425acb460dbdb3f0eL57 - I think using a flag to keep track of status works to get around the issue of multiple init calls.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2029138937
##########
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
Review Comment:
Maybe out of scope but I feel like this whole setting of `GlobalOpenTelemetry.set(otel)` should sit in its own function called `loadGlobalOpenTelemetry` and be called as [early as possible as per its documentation](https://opentelemetry.io/docs/languages/java/api/#:~:text=If%20GlobalOpenTelemetry.get,by%20any%20instrumentation.) maybe inside the constructor of CoreContainer. Then on initialization of Solr core container, to set it's tracer just call `TraceUtils.getGlobalTracer()`. But also this whole file is called `TracerConfigurator` when a lot of work is configuring OTel then just returning a static global tracer. This same logic is still used for metrics as it uses the same Java Agent and OTLP exporter from the module not just Tracer loading.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2066362901
##########
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:
hmm. if I remember correctly this is why I created `CustomTestOtelTracerConfigurator` so tests that race to set this would not fail. it felt awkward to have to catch `IllegalStateException` for problems that are triggered only via testing code.
I also remember there was an issue with checking for noop? there were some docs that were stating you should not check if the impl is noop, but I can't find a reference 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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-2844804550
I'll come back to this today/tomorrow. I really appreciate the close look @stillalex :-)
My biggest question is really scoping. This PR is a bit all over the place; it will take some work to break it into more meaningful chunks. I'll think about this further when I look closer again.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4149900264
##########
solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java:
##########
@@ -49,14 +48,13 @@ public OtelTracerConfigurator() {
}
@Override
- public Tracer getTracer() {
- return TraceUtils.getGlobalTracer();
Review Comment:
I suppose I merely mean the changes resulted in not needing to refer to GlobalOpenTelemetry (albeit indirectly). But it's been a while 🤪 A goal of this PR is to do that initialization in a singular spot.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4149884517
##########
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");
+ }
}
- if (TRACE_ID_GEN_ENABLED) {
- ExecutorUtil.addThreadLocalProvider(new ContextThreadLocalProvider());
- return SimplePropagator.load();
- }
- return TraceUtils.getGlobalTracer();
+
+ ExecutorUtil.addThreadLocalProvider(new ContextThreadLocalProvider());
+ return TraceUtils.getGlobalTracer(); // GlobalOpenTelemetry.get...
}
- protected abstract Tracer getTracer();
+ protected abstract OpenTelemetry createOtel();
Review Comment:
I restored backwards compatibility yesterday, supporting new and existing API.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-2354999765
I just saw SOLR-17455 coming in. While mentioning OpenTracing is correct for 9.x, for 10.0 it is gone, so perhaps when you add refGuide docs for this PR, you can also `s/OpenTracing/OpenTelemetry/` in the guide?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on PR #2687:
URL: https://github.com/apache/solr/pull/2687#issuecomment-2844758194
+1 this looks good to me. when you have some time, could you rebase? curious to see what errors are left.
I think the open question is the issue of multiple init calls causing that OTEL exception during tests.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2066367300
##########
solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/TestDistributedTracing.java:
##########
@@ -39,24 +39,22 @@
import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.util.tracing.TraceUtils;
-import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
public class TestDistributedTracing extends SolrCloudTestCase {
private static final String COLLECTION = "collection1";
+ @ClassRule public static OpenTelemetryRule otelRule = OpenTelemetryRule.create();
Review Comment:
this is pretty cool, I did not know this existed.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2066624897
##########
solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java:
##########
@@ -54,19 +46,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() {
Review Comment:
+1 moving this to TracerConfigurator is a good change.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2066477252
##########
solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java:
##########
@@ -49,14 +48,13 @@ public OtelTracerConfigurator() {
}
@Override
- public Tracer getTracer() {
- return TraceUtils.getGlobalTracer();
+ public void init(NamedList<?> args) {
+ prepareConfiguration(args);
}
@Override
- public void init(NamedList<?> args) {
- prepareConfiguration(args);
- AutoConfiguredOpenTelemetrySdk.initialize();
+ public OpenTelemetry createOtel() {
+ return AutoConfiguredOpenTelemetrySdk.builder().build().getOpenTelemetrySdk();
Review Comment:
this diff would have been easier to read if the 2 methods were not swapped.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "stillalex (via GitHub)" <gi...@apache.org>.
stillalex commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r2066471164
##########
solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java:
##########
@@ -49,14 +48,13 @@ public OtelTracerConfigurator() {
}
@Override
- public Tracer getTracer() {
- return TraceUtils.getGlobalTracer();
Review Comment:
sorry, could you clarify what this means?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17432: Allow OTel Java Agent [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2687:
URL: https://github.com/apache/solr/pull/2687#discussion_r4068605547
##########
solr/solr-ref-guide/modules/deployment-guide/deployment-nav.adoc:
##########
Review Comment:
re-org a bit + add opentelemetry.adoc
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org