Skip to content

Commit

Permalink
Improve javadocs and cleanup code
Browse files Browse the repository at this point in the history
Signed-off-by: Cyrille Le Clerc <[email protected]>
  • Loading branch information
cyrille-leclerc committed Jun 21, 2024
1 parent d997f5f commit 0214539
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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}
*/
Expand All @@ -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());

Check warning on line 48 in src/main/java/io/jenkins/plugins/opentelemetry/JenkinsControllerOpenTelemetry.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 48 is not covered by tests
}

String opentelemetryPluginVersion = OtelUtils.getOpentelemetryPluginVersion();
Expand All @@ -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");
}

Expand Down Expand Up @@ -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());
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Check warning on line 228 in src/main/java/io/jenkins/plugins/opentelemetry/JenkinsOpenTelemetryPluginConfiguration.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL: 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());
}

Expand All @@ -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");
Expand Down Expand Up @@ -646,9 +648,6 @@ public static JenkinsOpenTelemetryPluginConfiguration get() {
* (such as query string or fragment).
* A scheme of https indicates a secure connection.
* </p>
*
* @param endpoint
* @return
*/
public FormValidation doCheckEndpoint(@QueryParameter String endpoint) {
if (endpoint == null || endpoint.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Check warning on line 53 in src/main/java/io/jenkins/plugins/opentelemetry/init/ServletFilterInitializer.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL: support live reload of the config flag
boolean jenkinsWebInstrumentationEnabled = Optional.ofNullable(configProperties.getBoolean(JenkinsOtelSemanticAttributes.OTEL_INSTRUMENTATION_JENKINS_WEB_ENABLED)).orElse(true);
Expand All @@ -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 " +

Check warning on line 60 in src/main/java/io/jenkins/plugins/opentelemetry/init/ServletFilterInitializer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 60 is not covered by tests
JenkinsOtelSemanticAttributes.OTEL_INSTRUMENTATION_JENKINS_WEB_ENABLED + " to true.");
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
import javax.annotation.Nullable;
import java.util.Objects;

public class InstrumentationScope {
/**
* <a href="https://opentelemetry.io/docs/concepts/instrumentation-scope/">OpenTelemetry instrumentation scope</a>,
* data structured used by the {@link ReconfigurableOpenTelemetry} implementation
*/
class InstrumentationScope {
@Nonnull
final String instrumentationScopeName;
@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
* <p>
* A {@link EventLoggerProvider} that allows to reconfigure the {@link EventLogger}s.
* </p>
* <p>
* 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.
* </p>
*/
class ReconfigurableEventLoggerProvider implements EventLoggerProvider {

private final ConcurrentMap<InstrumentationScope, ReconfigurableEventLogger> eventLoggers = new ConcurrentHashMap<>();
private EventLoggerProvider delegate = EventLoggerProvider.noop();
Expand All @@ -42,11 +51,6 @@ public void setDelegate(EventLoggerProvider delegateEventLoggerBuilder) {
});
}

@VisibleForTesting
protected Map<InstrumentationScope, ReconfigurableEventLogger> getEventLoggers() {
return eventLoggers;
}

@VisibleForTesting
protected static class ReconfigurableEventLogger implements EventLogger {
EventLogger delegateEventLogger;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
* <p>
* A {@link LoggerProvider} that allows to reconfigure the {@link Logger}s.
* </p>
* <p>
* 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.
* </p>
*/
class ReconfigurableLoggerProvider implements LoggerProvider {
private LoggerProvider delegate;

private final ConcurrentMap<InstrumentationScope, ReconfigurableLogger> loggers = new ConcurrentHashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -41,18 +42,30 @@
import java.util.stream.Collectors;

/**
* Reconfigurable {@link OpenTelemetry}
* <p>
* Reconfigurable {@link EventLoggerProvider} that allows to reconfigure the {@link Tracer}s,
* {@link io.opentelemetry.api.logs.Logger}s, and {@link EventLogger}s.
* </p>
* <p>
* 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.
* </p>
* <p>
* 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.
* </p>
*/
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
Expand All @@ -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);

Check warning on line 82 in src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableOpenTelemetry.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 77-82 are not covered by tests
}

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)));
}
Expand All @@ -81,7 +94,7 @@ public void configure(@NonNull Map<String, String> 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
Expand Down Expand Up @@ -118,7 +131,7 @@ public void configure(@NonNull Map<String, String> 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

Expand All @@ -130,29 +143,29 @@ public void configure(@NonNull Map<String, String> 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();
}

@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");

Check warning on line 168 in src/main/java/io/jenkins/plugins/opentelemetry/opentelemetry/ReconfigurableOpenTelemetry.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 168 is not covered by tests
}
}
GlobalOpenTelemetry.resetForTest();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,17 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

public class ReconfigurableTracerProvider implements TracerProvider {
/**
* <p>
* A {@link TracerProvider} that allows to reconfigure the {@link Tracer}s.
* </p>
* <p>
* 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.
* </p>
*/
class ReconfigurableTracerProvider implements TracerProvider {

private TracerProvider delegate;

Expand Down

0 comments on commit 0214539

Please sign in to comment.