From 02145395443d270b3d55931eb13accd40121ec11 Mon Sep 17 00:00:00 2001 From: Cyrille Le Clerc Date: Fri, 21 Jun 2024 13:02:44 +0200 Subject: [PATCH] Improve javadocs and cleanup code Signed-off-by: Cyrille Le Clerc --- .../JenkinsControllerOpenTelemetry.java | 16 +++--- ...nkinsOpenTelemetryPluginConfiguration.java | 13 +++-- .../init/ServletFilterInitializer.java | 7 ++- .../opentelemetry/CloseableMeterProvider.java | 6 ++- .../opentelemetry/InstrumentationScope.java | 6 ++- .../ReconfigurableEventLoggerProvider.java | 18 ++++--- .../ReconfigurableLoggerProvider.java | 13 ++++- .../ReconfigurableOpenTelemetry.java | 51 ++++++++++++------- .../ReconfigurableTracerProvider.java | 12 ++++- 9 files changed, 92 insertions(+), 50 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/JenkinsControllerOpenTelemetry.java b/src/main/java/io/jenkins/plugins/opentelemetry/JenkinsControllerOpenTelemetry.java index dcf93e86c..cf6db03b6 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/JenkinsControllerOpenTelemetry.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/JenkinsControllerOpenTelemetry.java @@ -14,7 +14,6 @@ import io.jenkins.plugins.opentelemetry.semconv.JenkinsOtelSemanticAttributes; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.incubator.events.EventLogger; -import io.opentelemetry.api.incubator.events.GlobalEventLoggerProvider; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.instrumentation.resources.ProcessResourceProvider; @@ -23,7 +22,6 @@ import javax.annotation.PreDestroy; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; -import java.util.logging.Logger; import java.util.stream.Collectors; /** @@ -32,8 +30,6 @@ @Extension public class JenkinsControllerOpenTelemetry extends ReconfigurableOpenTelemetry implements OpenTelemetry { - private static final Logger LOGGER = Logger.getLogger(JenkinsControllerOpenTelemetry.class.getName()); - /** * See {@code OTEL_JAVA_DISABLED_RESOURCE_PROVIDERS} */ @@ -49,7 +45,7 @@ public class JenkinsControllerOpenTelemetry extends ReconfigurableOpenTelemetry public JenkinsControllerOpenTelemetry() { super(); if (INSTANCE_COUNTER.get() > 0) { - LOGGER.log(Level.WARNING, "More than one instance of JenkinsControllerOpenTelemetry created: " + INSTANCE_COUNTER.get()); + logger.log(Level.WARNING, "More than one instance of JenkinsControllerOpenTelemetry created: " + INSTANCE_COUNTER.get()); } String opentelemetryPluginVersion = OtelUtils.getOpentelemetryPluginVersion(); @@ -72,12 +68,12 @@ public Tracer getDefaultTracer() { } public boolean isLogsEnabled() { - String otelLogsExporter = config.getString("otel.logs.exporter"); + String otelLogsExporter = getConfig().getString("otel.logs.exporter"); return otelLogsExporter != null && !otelLogsExporter.equals("none"); } public boolean isOtelLogsMirrorToDisk() { - String otelLogsExporter = config.getString("otel.logs.mirror_to_disk"); + String otelLogsExporter = getConfig().getString("otel.logs.mirror_to_disk"); return otelLogsExporter != null && otelLogsExporter.equals("true"); } @@ -112,12 +108,12 @@ protected void postOpenTelemetrySdkConfiguration() { .setInstrumentationVersion(opentelemetryPluginVersion) .build(); - LOGGER.log(Level.FINER, () -> "Configure OpenTelemetryLifecycleListeners: " + ExtensionList.lookup(OpenTelemetryLifecycleListener.class).stream().sorted().map(e -> e.getClass().getName()).collect(Collectors.joining(", "))); + logger.log(Level.FINER, () -> "Configure OpenTelemetryLifecycleListeners: " + ExtensionList.lookup(OpenTelemetryLifecycleListener.class).stream().sorted().map(e -> e.getClass().getName()).collect(Collectors.joining(", "))); ExtensionList.lookup(OpenTelemetryLifecycleListener.class).stream() .sorted() .forEachOrdered(otelComponent -> { - otelComponent.afterSdkInitialized(defaultMeter, getOpenTelemetryDelegate().getLogsBridge(), defaultEventLogger, defaultTracer, config); - otelComponent.afterSdkInitialized(getOpenTelemetryDelegate(), config); + otelComponent.afterSdkInitialized(defaultMeter, getOpenTelemetryDelegate().getLogsBridge(), defaultEventLogger, defaultTracer, getConfig()); + otelComponent.afterSdkInitialized(getOpenTelemetryDelegate(), getConfig()); }); } diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/JenkinsOpenTelemetryPluginConfiguration.java b/src/main/java/io/jenkins/plugins/opentelemetry/JenkinsOpenTelemetryPluginConfiguration.java index 5f4727120..53504d584 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/JenkinsOpenTelemetryPluginConfiguration.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/JenkinsOpenTelemetryPluginConfiguration.java @@ -218,16 +218,18 @@ public OpenTelemetryConfiguration toOpenTelemetryConfiguration() { } /** - * Register reconfigurable {@link io.opentelemetry.api.OpenTelemetry} + * Register {@link io.jenkins.plugins.opentelemetry.opentelemetry.ReconfigurableOpenTelemetry} * on {@link io.opentelemetry.api.GlobalOpenTelemetry} * and {@link io.jenkins.plugins.opentelemetry.opentelemetry.ReconfigurableEventLoggerProvider} * on {@link io.opentelemetry.api.incubator.events.GlobalEventLoggerProvider} - * as early as possible in Jenkins lifecycle so any plugin invoking those Global setters will have the + * as early as possible in Jenkins lifecycle so any plugin invoking those Global getters will have the * reconfigurable instance . + * + * TODO can this be refactored into an annotation on the JenkinsControllerOpenTelemetry to force its early instantiation? */ @Initializer(after = InitMilestone.EXTENSIONS_AUGMENTED, before = InitMilestone.SYSTEM_CONFIG_LOADED) public void initializeOpenTelemetryAfterExtensionsAugmented() { - LOGGER.log(Level.INFO, "Initialize Jenkins OpenTelemetry Plugin with a NoOp implementation..."); + LOGGER.log(Level.FINE, "Initialize Jenkins OpenTelemetry Plugin with a NoOp implementation..."); jenkinsControllerOpenTelemetry.configure(Collections.emptyMap(), Resource.empty()); } @@ -238,7 +240,7 @@ public void initializeOpenTelemetryAfterExtensionsAugmented() { @Initializer(after = InitMilestone.SYSTEM_CONFIG_ADAPTED, before = InitMilestone.JOB_LOADED) @SuppressWarnings("MustBeClosedChecker") public void initializeOpenTelemetry() { - LOGGER.log(Level.INFO, "Initialize Jenkins OpenTelemetry Plugin..."); + LOGGER.log(Level.FINE, "Initialize Jenkins OpenTelemetry Plugin..."); OpenTelemetryConfiguration newOpenTelemetryConfiguration = toOpenTelemetryConfiguration(); if (Objects.equals(this.currentOpenTelemetryConfiguration, newOpenTelemetryConfiguration)) { LOGGER.log(Level.FINE, "Configuration didn't change, skip reconfiguration"); @@ -646,9 +648,6 @@ public static JenkinsOpenTelemetryPluginConfiguration get() { * (such as query string or fragment). * A scheme of https indicates a secure connection. *

- * - * @param endpoint - * @return */ public FormValidation doCheckEndpoint(@QueryParameter String endpoint) { if (endpoint == null || endpoint.isEmpty()) { diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/init/ServletFilterInitializer.java b/src/main/java/io/jenkins/plugins/opentelemetry/init/ServletFilterInitializer.java index 099728aea..6a43d5538 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/init/ServletFilterInitializer.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/init/ServletFilterInitializer.java @@ -46,7 +46,9 @@ public void afterSdkInitialized(Meter meter, LoggerProvider loggerProvider, Even traceContextServletFilter = new TraceContextServletFilter(); addToPluginServletFilter(traceContextServletFilter); } else { - logger.log(Level.INFO, () -> "Jenkins Remote Span disabled"); + logger.log(Level.INFO, () -> "Jenkins trace context propagation disabled on inbound HTTP requests (eg. build triggers). " + + "To enable it, set the property " + + JenkinsOtelSemanticAttributes.OTEL_INSTRUMENTATION_JENKINS_REMOTE_SPAN_ENABLED + " to true."); } // TODO support live reload of the config flag boolean jenkinsWebInstrumentationEnabled = Optional.ofNullable(configProperties.getBoolean(JenkinsOtelSemanticAttributes.OTEL_INSTRUMENTATION_JENKINS_WEB_ENABLED)).orElse(true); @@ -55,7 +57,8 @@ public void afterSdkInitialized(Meter meter, LoggerProvider loggerProvider, Even staplerInstrumentationServletFilter = new StaplerInstrumentationServletFilter(tracer); addToPluginServletFilter(staplerInstrumentationServletFilter); } else { - logger.log(Level.INFO, () -> "Jenkins Web instrumentation disabled"); + logger.log(Level.INFO, () -> "Jenkins Web instrumentation disabled. To enable it, set the property " + + JenkinsOtelSemanticAttributes.OTEL_INSTRUMENTATION_JENKINS_WEB_ENABLED + " to true."); } diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/CloseableMeterProvider.java b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/CloseableMeterProvider.java index 0f19b1064..179296688 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/CloseableMeterProvider.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/CloseableMeterProvider.java @@ -38,7 +38,11 @@ import java.util.logging.Level; import java.util.logging.Logger; -public class CloseableMeterProvider implements MeterProvider, Closeable { +/** + * A {@link MeterProvider} that can be closed before the underlying {@link io.opentelemetry.sdk.OpenTelemetrySdk} + * is shut down. + */ +class CloseableMeterProvider implements MeterProvider, Closeable { private final static Logger LOGGER = Logger.getLogger(CloseableMeterProvider.class.getName()); /** diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/InstrumentationScope.java b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/InstrumentationScope.java index 16d0e6d80..742f98db0 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/InstrumentationScope.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/InstrumentationScope.java @@ -9,7 +9,11 @@ import javax.annotation.Nullable; import java.util.Objects; -public class InstrumentationScope { +/** + * OpenTelemetry instrumentation scope, + * data structured used by the {@link ReconfigurableOpenTelemetry} implementation + */ +class InstrumentationScope { @Nonnull final String instrumentationScopeName; @Nullable diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableEventLoggerProvider.java b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableEventLoggerProvider.java index 81ba97de6..a64b27600 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableEventLoggerProvider.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableEventLoggerProvider.java @@ -11,13 +11,22 @@ import io.opentelemetry.api.incubator.events.EventLoggerBuilder; import io.opentelemetry.api.incubator.events.EventLoggerProvider; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -public class ReconfigurableEventLoggerProvider implements EventLoggerProvider { +/** + *

+ * A {@link EventLoggerProvider} that allows to reconfigure the {@link EventLogger}s. + *

+ *

+ * We need reconfigurability because Jenkins supports changing the configuration of the OpenTelemetry params at runtime. + * All instantiated eventLoggers are reconfigured when the configuration changes, when + * {@link ReconfigurableEventLoggerProvider#setDelegate(EventLoggerProvider)} is invoked. + *

+ */ +class ReconfigurableEventLoggerProvider implements EventLoggerProvider { private final ConcurrentMap eventLoggers = new ConcurrentHashMap<>(); private EventLoggerProvider delegate = EventLoggerProvider.noop(); @@ -42,11 +51,6 @@ public void setDelegate(EventLoggerProvider delegateEventLoggerBuilder) { }); } - @VisibleForTesting - protected Map getEventLoggers() { - return eventLoggers; - } - @VisibleForTesting protected static class ReconfigurableEventLogger implements EventLogger { EventLogger delegateEventLogger; diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableLoggerProvider.java b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableLoggerProvider.java index e9f69901e..9b625dde0 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableLoggerProvider.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableLoggerProvider.java @@ -10,14 +10,23 @@ import io.opentelemetry.api.logs.Logger; import io.opentelemetry.api.logs.LoggerBuilder; import io.opentelemetry.api.logs.LoggerProvider; -import io.opentelemetry.api.trace.TracerBuilder; import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -public class ReconfigurableLoggerProvider implements LoggerProvider { +/** + *

+ * A {@link LoggerProvider} that allows to reconfigure the {@link Logger}s. + *

+ *

+ * We need reconfigurability because Jenkins supports changing the configuration of the OpenTelemetry params at runtime. + * All instantiated loggers are reconfigured when the configuration changes, when + * {@link ReconfigurableLoggerProvider#setDelegate(LoggerProvider)} is invoked. + *

+ */ +class ReconfigurableLoggerProvider implements LoggerProvider { private LoggerProvider delegate; private final ConcurrentMap loggers = new ConcurrentHashMap<>(); diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableOpenTelemetry.java b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableOpenTelemetry.java index a894ded55..68f42a0f9 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableOpenTelemetry.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableOpenTelemetry.java @@ -14,6 +14,7 @@ import io.jenkins.plugins.opentelemetry.opentelemetry.autoconfigure.ConfigPropertiesUtils; import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.incubator.events.EventLogger; import io.opentelemetry.api.incubator.events.EventLoggerProvider; import io.opentelemetry.api.incubator.events.GlobalEventLoggerProvider; import io.opentelemetry.api.logs.LoggerProvider; @@ -41,18 +42,30 @@ import java.util.stream.Collectors; /** - * Reconfigurable {@link OpenTelemetry} + *

+ * Reconfigurable {@link EventLoggerProvider} that allows to reconfigure the {@link Tracer}s, + * {@link io.opentelemetry.api.logs.Logger}s, and {@link EventLogger}s. + *

+ *

+ * IMPORTANT: {@link Meter}s are not yet seamlessly reconfigurable yet. + * Please use {@link OpenTelemetryLifecycleListener} to handle the reconfiguration of {@link Meter}s for the moment. + *

+ *

+ * We need reconfigurability because Jenkins supports changing the configuration of the OpenTelemetry params at runtime. + * All instantiated tracers, loggers, and eventLoggers are reconfigured when the configuration changes, when + * {@link ReconfigurableOpenTelemetry#configure(Map, Resource)} is invoked. + *

*/ public class ReconfigurableOpenTelemetry implements OpenTelemetry, Closeable { - private final Logger LOGGER = Logger.getLogger(ReconfigurableOpenTelemetry.class.getName()); - protected transient Resource resource; - protected transient ConfigProperties config; - protected transient OpenTelemetry openTelemetryImpl = OpenTelemetry.noop(); - protected transient CloseableMeterProvider meterProviderImpl = new CloseableMeterProvider(MeterProvider.noop()); - protected final transient ReconfigurableTracerProvider traceProviderImpl = new ReconfigurableTracerProvider(); - protected final transient ReconfigurableEventLoggerProvider eventLoggerProviderImpl = new ReconfigurableEventLoggerProvider(); - protected final transient ReconfigurableLoggerProvider loggerProviderImpl = new ReconfigurableLoggerProvider(); + protected final Logger logger = Logger.getLogger(getClass().getName()); + Resource resource; + ConfigProperties config; + OpenTelemetry openTelemetryImpl = OpenTelemetry.noop(); + CloseableMeterProvider meterProviderImpl = new CloseableMeterProvider(MeterProvider.noop()); + final ReconfigurableTracerProvider traceProviderImpl = new ReconfigurableTracerProvider(); + final ReconfigurableEventLoggerProvider eventLoggerProviderImpl = new ReconfigurableEventLoggerProvider(); + final ReconfigurableLoggerProvider loggerProviderImpl = new ReconfigurableLoggerProvider(); /** * Initialize as NoOp @@ -61,15 +74,15 @@ public ReconfigurableOpenTelemetry() { try { GlobalOpenTelemetry.set(this); } catch (IllegalStateException e) { - LOGGER.log(Level.WARNING, "GlobalOpenTelemetry already set", e); + logger.log(Level.WARNING, "GlobalOpenTelemetry already set", e); } try { GlobalEventLoggerProvider.set(eventLoggerProviderImpl); } catch (IllegalStateException e) { - LOGGER.log(Level.WARNING, "GlobalEventLoggerProvider already set", e); + logger.log(Level.WARNING, "GlobalEventLoggerProvider already set", e); } - LOGGER.log(Level.FINE, () -> "Initialize " + + logger.log(Level.FINE, () -> "Initialize " + "GlobalOpenTelemetry with instance " + Optional.of(GlobalOpenTelemetry.get()).map(ot -> ot + "@" + System.identityHashCode(ot)) + "and " + "GlobalEventLoggerProvide with instance " + Optional.of(GlobalEventLoggerProvider.get()).map(elp -> elp + "@" + System.identityHashCode(elp))); } @@ -81,7 +94,7 @@ public void configure(@NonNull Map openTelemetryProperties, Reso openTelemetryProperties.containsKey("otel.metrics.exporter") || openTelemetryProperties.containsKey("otel.logs.exporter")) { - LOGGER.log(Level.FINE, "initializeOtlp"); + logger.log(Level.FINE, "initializeOtlp"); // OPENTELEMETRY SDK OpenTelemetrySdk openTelemetrySdk = AutoConfiguredOpenTelemetrySdk @@ -118,7 +131,7 @@ public void configure(@NonNull Map openTelemetryProperties, Reso // EVENT LOGGER PROVIDER eventLoggerProviderImpl.setDelegate(SdkEventLoggerProvider.create(openTelemetrySdk.getSdkLoggerProvider())); - LOGGER.log(Level.INFO, () -> "OpenTelemetry initialized: " + OtelUtils.prettyPrintOtelSdkConfig(this.config, this.resource)); + logger.log(Level.INFO, () -> "OpenTelemetry initialized: " + OtelUtils.prettyPrintOtelSdkConfig(this.config, this.resource)); } else { // NO-OP @@ -130,7 +143,7 @@ public void configure(@NonNull Map openTelemetryProperties, Reso this.loggerProviderImpl.setDelegate(LoggerProvider.noop()); this.eventLoggerProviderImpl.setDelegate(EventLoggerProvider.noop()); - LOGGER.log(Level.INFO, "OpenTelemetry initialized as NoOp"); + logger.log(Level.INFO, "OpenTelemetry initialized as NoOp"); } postOpenTelemetrySdkConfiguration(); @@ -138,21 +151,21 @@ public void configure(@NonNull Map openTelemetryProperties, Reso @Override public void close() { - LOGGER.log(Level.FINE, "Shutdown..."); + logger.log(Level.FINE, "Shutdown..."); // METER PROVIDER meterProviderImpl.close(); // OTEL LIFECYCLE LISTENERS - LOGGER.log(Level.FINE, () -> "Shutdown Otel SDK on components: " + ExtensionList.lookup(OpenTelemetryLifecycleListener.class).stream().sorted().map(e -> e.getClass().getName()).collect(Collectors.joining(", "))); + logger.log(Level.FINE, () -> "Shutdown Otel SDK on components: " + ExtensionList.lookup(OpenTelemetryLifecycleListener.class).stream().sorted().map(e -> e.getClass().getName()).collect(Collectors.joining(", "))); ExtensionList.lookup(OpenTelemetryLifecycleListener.class).stream().sorted().forEachOrdered(OpenTelemetryLifecycleListener::beforeSdkShutdown); // OTEL SDK if (this.openTelemetryImpl instanceof OpenTelemetrySdk) { - LOGGER.log(Level.FINE, () -> "Shutdown OTel SDK..."); + logger.log(Level.FINE, () -> "Shutdown OTel SDK..."); CompletableResultCode shutdown = ((OpenTelemetrySdk) this.openTelemetryImpl).shutdown(); if (!shutdown.join(1, TimeUnit.SECONDS).isSuccess()) { - LOGGER.log(Level.WARNING, "Failure to shutdown OTel SDK"); + logger.log(Level.WARNING, "Failure to shutdown OTel SDK"); } } GlobalOpenTelemetry.resetForTest(); diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableTracerProvider.java b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableTracerProvider.java index e45274e44..51beeaa79 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableTracerProvider.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableTracerProvider.java @@ -17,7 +17,17 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -public class ReconfigurableTracerProvider implements TracerProvider { +/** + *

+ * A {@link TracerProvider} that allows to reconfigure the {@link Tracer}s. + *

+ *

+ * We need reconfigurability because Jenkins supports changing the configuration of the OpenTelemetry params at runtime. + * All instantiated tracers are reconfigured when the configuration changes, when + * {@link ReconfigurableTracerProvider#setDelegate(TracerProvider)} is invoked. + *

+ */ +class ReconfigurableTracerProvider implements TracerProvider { private TracerProvider delegate;