Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2497d7e
Allow multiple LogStorage with primary and secondaries
jgreffe Jul 15, 2025
aefa52d
Additional tests and use cases
jgreffe Jul 16, 2025
2e0d37c
Apply suggestions
jgreffe Jul 17, 2025
88322ca
Apply suggestions
jgreffe Jul 17, 2025
2e2c97b
Switch from a list of factories to simple primary and secondary
jgreffe Jul 17, 2025
dd6e930
Create a specific `LogStorageFactoryDescriptor` for further server-si…
jgreffe Jul 18, 2025
b2a5025
Remove useless methods
jgreffe Jul 18, 2025
bd88176
Remove `TeePrintStream`, remove synchronization, update `mangled` test
jgreffe Jul 18, 2025
9090904
Handle the default descriptor and the TeeLogStorageFactory dropdowns
jgreffe Jul 21, 2025
2137275
Handle exceptions in `TeeOutputStream` and unit test
jgreffe Jul 22, 2025
ed2165c
Handle duplicate factory case
jgreffe Jul 22, 2025
dd5d67d
Handle default factory instance
jgreffe Jul 22, 2025
1512a75
Default factory instance test
jgreffe Jul 23, 2025
3c9d50d
Additional test
jgreffe Jul 23, 2025
1476e12
Fix test on windows
jgreffe Jul 23, 2025
05c1f4d
Fix test on windows
jgreffe Jul 23, 2025
3fe4cc6
Test on windows
jgreffe Jul 23, 2025
da22a49
Properly close the logger
jgreffe Jul 24, 2025
cdc3958
Fix test
jgreffe Jul 24, 2025
8dec700
More robust test
jgreffe Jul 24, 2025
02e97a3
Handle default factory
jgreffe Jul 24, 2025
ee9a44e
Cleanup
jgreffe Jul 28, 2025
d8fec33
UI test for the default factory
jgreffe Jul 28, 2025
423246b
Apply suggestions
jgreffe Jul 29, 2025
6d543ad
Update the default factory configuration
jgreffe Jul 30, 2025
77d2fc3
Display the factory plugin
jgreffe Aug 5, 2025
b707a36
Update the labels
jgreffe Aug 5, 2025
b1e6d01
Fix the tests
jgreffe Aug 5, 2025
2b3f3d6
Add readme.md
jgreffe Aug 5, 2025
8da65e3
Apply suggestions from code review
jgreffe Aug 6, 2025
62b9450
Update javadoc
jgreffe Aug 6, 2025
3782d00
Add `@Restricted` annotation
jgreffe Aug 6, 2025
96d6dae
Change descriptor methods to `isReadWrite` and `isWriteOnly`
jgreffe Aug 6, 2025
365c9e6
Apply suggestions from code review
jgreffe Aug 8, 2025
d93ae64
Update src/main/java/org/jenkinsci/plugins/workflow/log/tee/TeeBuildL…
jgreffe Aug 8, 2025
d83b6c0
Apply suggestions from code review
jgreffe Sep 1, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ Plugin that defines Pipeline API.
A component of [Pipeline
Plugin](https://plugins.jenkins.io/workflow-aggregator).

# JEP-210: External log storage for Pipeline
## Implementation
This plugin provides APIs for [https://github.com/jenkinsci/jep/tree/master/jep/210](JEP-210), which allow plugins to take over Pipeline build logging.
The default logging implementation is the `@Extension LogStorageFactoryDescriptor` with the highest `ordinal` value that implements `LogStorageFactoryDescriptor.getDefaultInstance`.
You can override the default implementation by configuring a logger explicitly under "Pipeline logger" on the Jenkins system configuration page.

## Multiple loggers
In some cases, you may want to use a logging implementation that sends logs to an external system, while also preserving logs in Jenkins for other use cases.
You can accomplish by configuring the "Multiple loggers" implementation in the "Pipeline logger" section on the Jenkins system configuration page.
This implementation allows you to select a "Primary" logger that handles reads and writes, as well as a "Secondary" logger which receives copies of all writes, similarly to the Unix `tee` command.

# Changelog

* For new versions, see [GitHub Releases](https://github.com/jenkinsci/workflow-api-plugin/releases)
Expand Down
5 changes: 5 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,10 @@
<artifactId>apache-httpcomponents-client-4-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.jenkins.configuration-as-code</groupId>
<artifactId>test-harness</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package org.jenkinsci.plugins.workflow.log;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.Descriptor;
import java.io.File;
import org.jenkinsci.Symbol;
import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.Beta;
import org.kohsuke.stapler.DataBoundConstructor;

@Restricted(Beta.class)
public class FileLogStorageFactory implements LogStorageFactory {

@DataBoundConstructor
public FileLogStorageFactory() {}

@Override
public LogStorage forBuild(@NonNull FlowExecutionOwner b) {
try {
return FileLogStorage.forFile(new File(b.getRootDir(), "log"));
} catch (Exception x) {
return new BrokenLogStorage(x);
}
}

@Extension
@Symbol("file")
public static final class DescriptorImpl extends LogStorageFactoryDescriptor<FileLogStorageFactory> {
@NonNull
@Override
public String getDisplayName() {
return "Standard file logger";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
package org.jenkinsci.plugins.workflow.log;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.ExtensionList;
import hudson.console.AnnotatedLargeText;
import hudson.console.ConsoleAnnotationOutputStream;
import hudson.model.BuildListener;
Expand All @@ -39,6 +38,7 @@
import java.util.logging.Logger;
import edu.umd.cs.findbugs.annotations.NonNull;
import org.jenkinsci.plugins.workflow.actions.LogAction;
import org.jenkinsci.plugins.workflow.log.configuration.PipelineLoggingGlobalConfiguration;
import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.steps.StepContext;
Expand Down Expand Up @@ -160,8 +160,9 @@ public interface LogStorage {
*/
static @NonNull LogStorage of(@NonNull FlowExecutionOwner b) {
try {
for (LogStorageFactory factory : ExtensionList.lookup(LogStorageFactory.class)) {
LogStorage storage = factory.forBuild(b);
PipelineLoggingGlobalConfiguration config = PipelineLoggingGlobalConfiguration.get();
if (config.getFactoryOrDefault() != null) {
LogStorage storage = config.getFactoryOrDefault().forBuild(b);
if (storage != null) {
// Pending integration with JEP-207 / JEP-212, this choice is not persisted.
return storage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@

package org.jenkinsci.plugins.workflow.log;

import hudson.ExtensionPoint;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.model.Describable;
import java.util.List;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.Beta;
Expand All @@ -35,13 +37,33 @@
* Factory interface for {@link LogStorage}.
*/
@Restricted(Beta.class)
public interface LogStorageFactory extends ExtensionPoint {
public interface LogStorageFactory extends Describable<LogStorageFactory> {

/**
* Checks whether we should handle a given build.
* When the current factory has been configured or is considered a default factory {@link #getDefaultFactory()}, returns the expected log storage instance to handle the build.
* @param b a build about to start
* @return a mechanism for handling this build, or null to fall back to the next implementation or the default
* @return a mechanism for handling this build, see {@link LogStorage#of(FlowExecutionOwner)}
*/
@CheckForNull LogStorage forBuild(@NonNull FlowExecutionOwner b);

default LogStorageFactoryDescriptor<?> getDescriptor() {
return (LogStorageFactoryDescriptor<?>) Jenkins.get().getDescriptorOrDie(this.getClass());
}

static List<LogStorageFactoryDescriptor<?>> all() {
return Jenkins.get().getDescriptorList(LogStorageFactory.class);
}

/**
* Returns the default {@link LogStorageFactory} based on the descriptor {@code @Extension#ordinal} order and the {@link LogStorageFactoryDescriptor#getDefaultInstance()} implmentations.
*/
static LogStorageFactory getDefaultFactory() {
for (LogStorageFactoryDescriptor<?> descriptor : all()) {
var instance = descriptor.getDefaultInstance();
if (instance != null) {
return instance;
}
}
return new FileLogStorageFactory();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.jenkinsci.plugins.workflow.log;

import hudson.model.Descriptor;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.Beta;

@Restricted(Beta.class)
public abstract class LogStorageFactoryDescriptor<T extends LogStorageFactory> extends Descriptor<LogStorageFactory> {

/**
* Indicates whether the factory supports being used in read/write mode (e.g. as a top-level logger, or as a primary for {@link org.jenkinsci.plugins.workflow.log.tee.TeeLogStorageFactory})
*/
public boolean isReadWrite() {
return true;
}
/**
* Indicates whether the factory supports being used in write-only mode (as a secondary for {@link org.jenkinsci.plugins.workflow.log.tee.TeeLogStorageFactory}).
*/
public boolean isWriteOnly() {
return true;
}

/**
* Allow to define the default factory instance to use if no configuration exists
*/
public LogStorageFactory getDefaultInstance() {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package org.jenkinsci.plugins.workflow.log.configuration;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.ExtensionList;
import java.util.ArrayList;
import java.util.List;
import java.util.logging.Logger;
import jenkins.model.GlobalConfiguration;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.jenkinsci.Symbol;
import org.jenkinsci.plugins.workflow.log.LogStorageFactory;
import org.jenkinsci.plugins.workflow.log.LogStorageFactoryDescriptor;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.Beta;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.StaplerRequest2;

@Extension
@Symbol("pipelineLogging")
@Restricted(Beta.class)
public class PipelineLoggingGlobalConfiguration extends GlobalConfiguration {
private static final Logger LOGGER = Logger.getLogger(PipelineLoggingGlobalConfiguration.class.getName());
private LogStorageFactory factory;

public PipelineLoggingGlobalConfiguration() {
load();
}

/**
* For configuration only. Use {@link #getFactoryOrDefault()} instead.
*/
@Restricted(NoExternalUse.class)
public LogStorageFactory getFactory() {
return factory;
}

@DataBoundSetter
public void setFactory(LogStorageFactory factory) {
this.factory = factory;
save();
}

@Override
public boolean configure(StaplerRequest2 req, JSONObject json) throws FormException {
this.factory = null;
return super.configure(req, json);
}

public LogStorageFactory getFactoryOrDefault() {
if (factory == null) {
return LogStorageFactory.getDefaultFactory();
}
return factory;
}

public List<LogStorageFactoryDescriptor<?>> getLogStorageFactoryDescriptors() {
List<LogStorageFactoryDescriptor<?>> result = new ArrayList<>();
result.add(null); // offer the option to use the default factory without any explicit configuration
result.addAll(getFilteredLogStorageFactoryDescriptors());
return result;
}

private List<LogStorageFactoryDescriptor<?>> getFilteredLogStorageFactoryDescriptors() {
return LogStorageFactory.all().stream()
.filter(LogStorageFactoryDescriptor::isReadWrite)
.toList();
}

public LogStorageFactoryDescriptor<?> getDefaultFactoryDescriptor() {
return LogStorageFactory.getDefaultFactory().getDescriptor();
}

public String getDefaultFactoryPlugin() {
var pluginWrapper = Jenkins.get().getPluginManager().whichPlugin(LogStorageFactory.getDefaultFactory().getClass());
return pluginWrapper != null ? pluginWrapper.getShortName() : "unknown";
}

public static PipelineLoggingGlobalConfiguration get() {
return ExtensionList.lookupSingleton(PipelineLoggingGlobalConfiguration.class);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package org.jenkinsci.plugins.workflow.log.tee;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.model.BuildListener;
import hudson.model.TaskListener;
import java.io.OutputStream;
import java.io.Serial;
import java.util.List;
import org.jenkinsci.plugins.workflow.log.OutputStreamTaskListener;

class TeeBuildListener extends OutputStreamTaskListener.Default
implements BuildListener, OutputStreamTaskListener, AutoCloseable {

@Serial
private static final long serialVersionUID = 1L;

private final TaskListener primary;

private final List<TaskListener> secondaries;

private transient OutputStream outputStream;

TeeBuildListener(TaskListener primary, TaskListener... secondaries) {
this.primary = primary;
this.secondaries = List.of(secondaries);
}

@NonNull
@Override
public synchronized OutputStream getOutputStream() {
if (outputStream == null) {
outputStream = new TeeOutputStream(
OutputStreamTaskListener.getOutputStream(primary),
secondaries.stream()
.map(OutputStreamTaskListener::getOutputStream)
.toArray(OutputStream[]::new));
}
return outputStream;
}

@Override
public void close() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note: While I was looking at the code yesterday I realized the logic here is a bit confusing. We close the delegate TaskListeners, but we don't actually close the TeeOutputStream or the PrintStream returned by getLogger(). At first I thought maybe we could simplify by just calling getLogger().close(), which would eventually close the TeeOutputStream and we could get rid of the rest of the method, but at least BufferedBuildListener and CloudWatchSender have special close methods that do more than just closing the internal streams, so we really do need to try to close the TaskListeners themselves.

This means that after a call to close(), getLogger() and outputStream will still exist as unclosed streams even after their delegates have been been closed, which is a bit weird. We might be able to call getLogger().close() and then close the listeners, but I can't remember if any of the relevant streams have problems with being closed twice. I would maybe look into this a bit just to make sure whether the TeeOutputStream not being closed is ok.

Copy link
Author

@jgreffe jgreffe Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I update with something like:

        Exception exception = null;
        if (outputStream != null) {
            try {
                outputStream.close();
            } catch (IOException e) {
                exception = e;
            }
        }

then TeeLogStorageTest#smokes fails with:

org.opentest4j.MultipleFailuresError: Multiple Failures (2 failures)
	org.junit.ComparisonFailure: expected:<[<a href='http://nowhere.net/'>nikde</a>
]> but was:<[]>
	java.lang.AssertionError: 
Expected: is "starting\none #1\ntwo #1\ntwo #2\ninterrupting\none #2\none #3\npausing\nresuming\none #4\nthree #1\nending\n"
     but: was "starting\none #1\ntwo #1\ntwo #2\ninterrupting\none #2\none #3\npausing\nresuming\none #4\nthree #1\nending\nha:////4AM+Xbq6l0DXt+Aa5ectJ4m2Ny0f1G1cNAXESjnHxoh7AAAAlB+LCAAAAAAAAP9b85aBtbiIQSajNKU4P08vOT+vOD8nVc+jsiC1KCczL9svvyTVzHb1RttJBUeZGJg8GdhyUvPSSzJ8GJhLi3JKGIR8shLLEvVzEvPS9YNLijLz0q0rihik0IxzhtAgwxgggJGJgaGiAMhgLWEQzigpKbDS18/LL89ILUrVy0st0QcAFd2f8JgAAAA=nikde\n"
	...
	Suppressed: org.junit.ComparisonFailure: expected:<[<a href='http://nowhere.net/'>nikde</a>
]> but was:<[]>
		at org.jenkinsci.plugins.workflow.log.LogStorageTestBase.assertLog(LogStorageTestBase.java:324)
		at org.jenkinsci.plugins.workflow.log.LogStorageTestBase.assertStepLog(LogStorageTestBase.java:305)
		at org.jenkinsci.plugins.workflow.log.LogStorageTestBase.smokes(LogStorageTestBase.java:155)

Copy link
Member

@dwnusbaum dwnusbaum Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you specifically need to use getLogger().close(), not outputStream.close, maybe so that the PrintStream flushes its buffer and the stream, but I am not exactly sure why the former works but the latter does not without investigating more deeply. If things are ok without closing the PrintStream or TeeOutputStream itself that might be fine too, I would just check to make sure everything is getting GC'd as expected. Maybe there could be a difference in behavior if you tested writing lines without newlines or something like that, but I do not know.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

@jgreffe jgreffe Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But now with the additional getLogger().close(), the logger is flushed and closed, making TeeOutputStreamTest#primary_fails_close fail, and the close() method is called twice:

Breakpoint reached
	at org.jenkinsci.plugins.workflow.log.tee.TeeOutputStreamTest$4.close(TeeOutputStreamTest.java:118)
	at org.jenkinsci.plugins.workflow.log.tee.RemoteCustomFileLogStorage$Writer.close(RemoteCustomFileLogStorage.java:138)
	at org.jenkinsci.plugins.workflow.log.tee.TeeOutputStream$$Lambda/0x000000d80168a838.apply(Unknown Source:-1)
	at org.jenkinsci.plugins.workflow.log.tee.TeeOutputStream.handleAction(TeeOutputStream.java:45)
	at org.jenkinsci.plugins.workflow.log.tee.TeeOutputStream.close(TeeOutputStream.java:34)
	at java.io.PrintStream.implClose(PrintStream.java:500)
	at java.io.PrintStream.close(PrintStream.java:484)
	at org.jenkinsci.plugins.workflow.log.tee.TeeBuildListener.close(TeeBuildListener.java:51)
...
Breakpoint reached
	at org.jenkinsci.plugins.workflow.log.tee.TeeOutputStreamTest$4.close(TeeOutputStreamTest.java:118)
	at org.jenkinsci.plugins.workflow.log.tee.RemoteCustomFileLogStorage$Writer.close(RemoteCustomFileLogStorage.java:138)
	at org.jenkinsci.plugins.workflow.log.tee.RemoteCustomFileLogStorage$MyListener.close(RemoteCustomFileLogStorage.java:99)
	at org.jenkinsci.plugins.workflow.log.tee.TeeBuildListener.close(TeeBuildListener.java:55)
	at org.jenkinsci.plugins.workflow.log.tee.TeeOutputStreamTest.primary_fails_close(TeeOutputStreamTest.java:123)
	at java.lang.invoke.LambdaForm$DMH/0x000000d801218c00.invokeVirtual(LambdaForm$DMH:-1)
	at java.lang.invoke.LambdaForm$MH/0x000000d801324000.invoke(LambdaForm$MH:-1)
	at java.lang.invoke.Invokers$Holder.invokeExact_MT(Invokers$Holder:-1)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry, I was not saying that we definitely need it, only that if we do need it, we need to close the full PrintStream, not just the OutputStream, and yes it means the stream will be closed twice as I mentioned in #417 (comment). It's just not clear to me whether it is preferable to leave the outer streams open and just let them be cleaned up by GC after the delegate TaskListeners are closed, or to close the PrintStream, causing the TeeOutputStreamTest stream to be closed twice. I will try to investigate in more detail later today.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so now we close the stream twice, but everything seems to be ok IIUC. @jgreffe did you investigate to see if you prefer the behavior with the current code or how you had things prior to adding line 51?

getLogger().close();
Exception exception = null;
if (primary instanceof AutoCloseable) {
try {
((AutoCloseable) primary).close();
} catch (Exception e) {
exception = e;
}
}
for (TaskListener secondary : secondaries) {
if (secondary instanceof AutoCloseable) {
try {
((AutoCloseable) secondary).close();
} catch (Exception e) {
if (exception == null) {
exception = e;
} else {
exception.addSuppressed(e);
}
}
}
}
if (exception != null) {
throw exception;
}
}
}
Loading
Loading