-
Notifications
You must be signed in to change notification settings - Fork 0
Update OtelLogStorageFactory to use last workflow-api changes
#1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,8 +60,8 @@ | |
| <properties> | ||
| <revision>3</revision> | ||
| <changelist>999999-SNAPSHOT</changelist> | ||
| <jenkins.baseline>2.479</jenkins.baseline> | ||
| <jenkins.version>${jenkins.baseline}.3</jenkins.version> | ||
| <jenkins.baseline>2.504</jenkins.baseline> | ||
| <jenkins.version>${jenkins.baseline}.1</jenkins.version> | ||
| <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo> | ||
| <opentelemetry.version>1.49.0</opentelemetry.version> | ||
| <jenkins-opentelemetry.version>1.49.0.75.v66006f513b_1f</jenkins-opentelemetry.version> | ||
|
|
@@ -149,6 +149,22 @@ | |
| <artifactId>parsson</artifactId> | ||
| <version>1.1.7</version> | ||
| </dependency> | ||
| <dependency> | ||
| <!-- depends on https://github.com/jenkinsci/workflow-api-plugin/pull/417 --> | ||
| <!-- TODO remove when in bom --> | ||
| <groupId>org.jenkins-ci.plugins.workflow</groupId> | ||
| <artifactId>workflow-api</artifactId> | ||
| <version>1404.vb_1e6d0166a_33</version> | ||
| </dependency> | ||
| <dependency> | ||
| <!-- depends on https://github.com/jenkinsci/workflow-api-plugin/pull/417 --> | ||
| <!-- TODO remove when in bom --> | ||
| <groupId>org.jenkins-ci.plugins.workflow</groupId> | ||
| <artifactId>workflow-api</artifactId> | ||
| <version>1404.vb_1e6d0166a_33</version> | ||
| <scope>test</scope> | ||
| <classifier>tests</classifier> | ||
| </dependency> | ||
| </dependencies> | ||
| </dependencyManagement> | ||
| <dependencies> | ||
|
|
@@ -176,6 +192,16 @@ | |
| <groupId>org.apache.httpcomponents.client5</groupId> | ||
| <artifactId>httpclient5</artifactId> | ||
| </exclusion> | ||
| <exclusion> | ||
| <!-- provided by org.jenkins-ci.plugins:jackson2-api --> | ||
| <groupId>com.fasterxml.jackson.core</groupId> | ||
| <artifactId>jackson-core</artifactId> | ||
| </exclusion> | ||
| <exclusion> | ||
| <!-- provided by org.jenkins-ci.plugins:jackson2-api --> | ||
| <groupId>com.fasterxml.jackson.core</groupId> | ||
| <artifactId>jackson-databind</artifactId> | ||
| </exclusion> | ||
| </exclusions> | ||
| </dependency> | ||
| <dependency> | ||
|
|
@@ -525,10 +551,6 @@ | |
| <version>1.21.3</version> | ||
| <scope>test</scope> | ||
| <exclusions> | ||
| <exclusion> | ||
| <groupId>org.apache.commons</groupId> | ||
| <artifactId>commons-compress</artifactId> | ||
| </exclusion> | ||
|
Comment on lines
-528
to
-531
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to remove this exclusion due to https://issues.jenkins.io/browse/JENKINS-73355. |
||
| <exclusion> | ||
| <groupId>org.slf4j</groupId> | ||
| <artifactId>slf4j-api</artifactId> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,6 @@ | |
| import edu.umd.cs.findbugs.annotations.NonNull; | ||
| import edu.umd.cs.findbugs.annotations.Nullable; | ||
| import hudson.Extension; | ||
| import hudson.ExtensionList; | ||
| import hudson.model.Queue; | ||
| import hudson.model.Run; | ||
| import io.jenkins.plugins.opentelemetry.JenkinsControllerOpenTelemetry; | ||
|
|
@@ -19,19 +18,19 @@ | |
| import java.io.IOException; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import javax.annotation.PostConstruct; | ||
| import javax.inject.Inject; | ||
| import org.jenkinsci.Symbol; | ||
| import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner; | ||
| import org.jenkinsci.plugins.workflow.log.BrokenLogStorage; | ||
| import org.jenkinsci.plugins.workflow.log.LogStorage; | ||
| import org.jenkinsci.plugins.workflow.log.LogStorageFactory; | ||
| import org.jenkinsci.plugins.workflow.log.LogStorageFactoryDescriptor; | ||
| import org.kohsuke.stapler.DataBoundConstructor; | ||
|
|
||
| /** | ||
| * Binds Otel Logs to Pipeline logs. | ||
| * <p> | ||
| * See <a href="https://github.com/jenkinsci/pipeline-cloudwatch-logs-plugin/blob/pipeline-cloudwatch-logs-0.2/src/main/java/io/jenkins/plugins/pipeline_cloudwatch_logs/PipelineBridge.java">Pipeline Cloudwatch Logs - PipelineBridge</a> | ||
| */ | ||
| @Extension | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the Describable/Descriptor pattern, the |
||
| public final class OtelLogStorageFactory implements LogStorageFactory, OpenTelemetryLifecycleListener { | ||
|
|
||
| private static final Logger logger = Logger.getLogger(OtelLogStorageFactory.class.getName()); | ||
|
|
@@ -41,16 +40,17 @@ public final class OtelLogStorageFactory implements LogStorageFactory, OpenTelem | |
| System.setProperty("org.jenkinsci.plugins.workflow.steps.durable_task.DurableTaskStep.USE_WATCHING", "true"); | ||
| } | ||
|
|
||
| @Inject | ||
| JenkinsControllerOpenTelemetry jenkinsControllerOpenTelemetry; | ||
| private final JenkinsControllerOpenTelemetry jenkinsControllerOpenTelemetry; | ||
|
|
||
| @Nullable | ||
| private OtelTraceService otelTraceService; | ||
| private final OtelTraceService otelTraceService; | ||
|
|
||
| private Tracer tracer; | ||
| private final Tracer tracer; | ||
|
|
||
| static OtelLogStorageFactory get() { | ||
| return ExtensionList.lookupSingleton(OtelLogStorageFactory.class); | ||
| @DataBoundConstructor | ||
| public OtelLogStorageFactory() { | ||
| jenkinsControllerOpenTelemetry = JenkinsControllerOpenTelemetry.get(); | ||
| otelTraceService = OtelTraceService.get(); | ||
| tracer = jenkinsControllerOpenTelemetry.getDefaultTracer(); | ||
|
Comment on lines
+51
to
+53
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's instantiate everything once in the constructor without |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -62,7 +62,7 @@ static OtelLogStorageFactory get() { | |
| @Override | ||
| public LogStorage forBuild(@NonNull final FlowExecutionOwner owner) { | ||
| LogStorage ret = null; | ||
| if (!getJenkinsControllerOpenTelemetry().isLogsEnabled()) { | ||
| if (!jenkinsControllerOpenTelemetry.isLogsEnabled()) { | ||
| logger.log(Level.FINE, () -> "OTel Logs disabled"); | ||
| return ret; | ||
| } | ||
|
|
@@ -86,35 +86,23 @@ private LogStorage forExec(@NonNull Queue.Executable exec) { | |
| if (exec instanceof Run<?, ?> run && run.getAction(MonitoringAction.class) != null) { | ||
| // it's a pipeline with monitoring data | ||
| logger.log(Level.FINEST, () -> "forExec(" + run + ")"); | ||
| ret = new OtelLogStorage(run, getOtelTraceService(), tracer); | ||
| ret = new OtelLogStorage(run, otelTraceService, tracer); | ||
| } | ||
| return ret; | ||
| } | ||
|
|
||
| /** | ||
| * Workaround dependency injection problem. @Inject doesn't work here | ||
| */ | ||
| @NonNull | ||
| private JenkinsControllerOpenTelemetry getJenkinsControllerOpenTelemetry() { | ||
| if (jenkinsControllerOpenTelemetry == null) { | ||
| jenkinsControllerOpenTelemetry = JenkinsControllerOpenTelemetry.get(); | ||
| @Extension() | ||
| @Symbol("opentelemetry") | ||
| public static final class DescriptorImpl extends LogStorageFactoryDescriptor<OtelLogStorageFactory> { | ||
|
Comment on lines
+94
to
+96
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Describable/Descriptor pattern, the symbol "opentelemetry" is used for CasC configuration. |
||
| @NonNull | ||
| @Override | ||
| public String getDisplayName() { | ||
| return "Open Telemetry"; | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Displayed in the new "Pipeline logging" global UI configuration, see jenkinsci/workflow-api-plugin#417 |
||
| } | ||
| return jenkinsControllerOpenTelemetry; | ||
| } | ||
|
Comment on lines
-94
to
-103
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Useless internal private method |
||
|
|
||
| /** | ||
| * Workaround dependency injection problem. @Inject doesn't work here | ||
| */ | ||
| @NonNull | ||
| private OtelTraceService getOtelTraceService() { | ||
| if (otelTraceService == null) { | ||
| otelTraceService = OtelTraceService.get(); | ||
| @Override | ||
| public LogStorageFactory getDefaultInstance() { | ||
| return new OtelLogStorageFactory(); | ||
| } | ||
| return otelTraceService; | ||
| } | ||
|
|
||
| @PostConstruct | ||
| public void postConstruct() { | ||
| this.tracer = jenkinsControllerOpenTelemetry.getDefaultTracer(); | ||
| } | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Useless internal private methods. |
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix some "require upper bounds" maven issues.