janhoy commented on code in PR #1841:
URL: https://github.com/apache/solr/pull/1841#discussion_r1294975785
##########
solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java:
##########
@@ -16,38 +16,56 @@
*/
package org.apache.solr.opentelemetry;
-import io.opentelemetry.opentracingshim.OpenTracingShim;
-import io.opentelemetry.sdk.OpenTelemetrySdk;
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
-import io.opentracing.Tracer;
import java.lang.invoke.MethodHandles;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
+import org.apache.solr.common.util.NamedList;
import org.apache.solr.core.TracerConfigurator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * OpenTracing TracerConfigurator implementation which exports spans to
OpenTelemetry in OTLP
- * format. This impl re-uses the existing OpenTracing instrumentation through
a shim, and takes care
- * of properly closing the backing Tracer when Solr shuts down.
+ * Tracing TracerConfigurator implementation which exports spans to
OpenTelemetry in OTLP format.
*/
public class OtelTracerConfigurator extends TracerConfigurator {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
- // Copy of environment. Can be overridden by tests
- Map<String, String> currentEnv = System.getenv();
+
+ private final Map<String, String> currentEnv;
+
+ public OtelTracerConfigurator() {
+ this(System.getenv());
+ }
+
+ OtelTracerConfigurator(Map<String, String> currentEnv) {
+ this.currentEnv = currentEnv;
+ }
@Override
public Tracer getTracer() {
+ // TODO remove reliance on global
+ return GlobalOpenTelemetry.getTracer("solr");
+ }
+
+ @Override
+ public void init(NamedList<?> args) {
+ prepareConfiguration();
+ AutoConfiguredOpenTelemetrySdk.initialize();
+ }
+
+ void prepareConfiguration() {
setDefaultIfNotConfigured("OTEL_SERVICE_NAME", "solr");
setDefaultIfNotConfigured("OTEL_TRACES_EXPORTER", "otlp");
setDefaultIfNotConfigured("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc");
setDefaultIfNotConfigured("OTEL_TRACES_SAMPLER", "parentbased_always_on");
- addOtelResourceAttributes(Map.of("host.name", System.getProperty("host")));
+ setDefaultIfNotConfigured("OTEL_PROPAGATORS", "tracecontext,baggage");
+ addOtelResourceAttributes(Map.of("host.name", System.getProperty("host",
"localhost")));
Review Comment:
Sure, it won't be any harm, although I prefer if production code is explicit
and fails fast, i.e. better with `null` as host than faulty `localhost`?
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
