Allow multiple LogStorage with primary and secondaries #417
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 "File Log Storage Factory"; | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| package org.jenkinsci.plugins.workflow.log; | ||
|
|
||
| import hudson.model.Descriptor; | ||
|
|
||
| public abstract class LogStorageFactoryDescriptor<T extends LogStorageFactory> extends Descriptor<LogStorageFactory> {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package org.jenkinsci.plugins.workflow.log.configuration; | ||
|
|
||
| import hudson.Extension; | ||
| import hudson.ExtensionList; | ||
| import java.util.List; | ||
| import java.util.logging.Logger; | ||
| import jenkins.model.GlobalConfiguration; | ||
| import jenkins.model.Jenkins; | ||
| 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.stapler.DataBoundSetter; | ||
|
|
||
| @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(); | ||
| } | ||
|
|
||
| public LogStorageFactory getFactory() { | ||
| return factory; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setFactory(LogStorageFactory factory) { | ||
| this.factory = factory; | ||
| save(); | ||
| } | ||
|
|
||
| public List<LogStorageFactoryDescriptor<?>> getLogStorageFactoryDescriptors() { | ||
| return LogStorageFactory.all(); | ||
| } | ||
|
|
||
| public LogStorageFactoryDescriptor<?> getDefaultLogStorageFactoryDescriptor() { | ||
| return (LogStorageFactoryDescriptor<?>) Jenkins.get().getDescriptor("file"); | ||
| } | ||
|
|
||
| public static PipelineLoggingGlobalConfiguration get() { | ||
| return ExtensionList.lookupSingleton(PipelineLoggingGlobalConfiguration.class); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| 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.PrintStream; | ||
| import java.io.Serial; | ||
| import java.util.List; | ||
| import java.util.stream.Stream; | ||
| import org.jenkinsci.plugins.workflow.log.OutputStreamTaskListener; | ||
|
|
||
| class TeeBuildListener implements BuildListener, OutputStreamTaskListener, AutoCloseable { | ||
|
|
||
| @Serial | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| private final transient TeeLogStorage teeLogStorage; | ||
dwnusbaum marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| private final BuildListener primary; | ||
|
|
||
| private final List<BuildListener> secondaries; | ||
|
|
||
| private transient OutputStream outputStream; | ||
|
|
||
| private transient PrintStream printStream; | ||
|
|
||
dwnusbaum marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| TeeBuildListener(TeeLogStorage teeLogStorage, BuildListener primary, BuildListener... secondaries) { | ||
dwnusbaum marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (!(primary instanceof OutputStreamTaskListener)) { | ||
| throw new ClassCastException("Primary is not an instance of OutputStreamTaskListener: " + primary); | ||
| } | ||
| List.of(secondaries).forEach(secondary -> { | ||
| if (!(secondary instanceof OutputStreamTaskListener)) { | ||
| throw new ClassCastException("Secondary is not an instance of OutputStreamTaskListener: " + secondary); | ||
| } | ||
| }); | ||
jgreffe marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| this.teeLogStorage = teeLogStorage; | ||
| this.primary = primary; | ||
| this.secondaries = List.of(secondaries); | ||
| } | ||
|
|
||
| TeeBuildListener(TeeLogStorage teeLogStorage, TaskListener primary, TaskListener... secondaries) { | ||
| this( | ||
| teeLogStorage, | ||
| (BuildListener) primary, | ||
| Stream.of(secondaries) | ||
| .map(secondary -> (BuildListener) secondary) | ||
| .toArray(BuildListener[]::new)); | ||
| } | ||
|
|
||
| @NonNull | ||
| @Override | ||
| public synchronized OutputStream getOutputStream() { | ||
| if (outputStream == null) { | ||
| outputStream = new TeeOutputStream( | ||
| teeLogStorage, | ||
| ((OutputStreamTaskListener) primary).getOutputStream(), | ||
dwnusbaum marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| secondaries.stream() | ||
| .map(secondary -> ((OutputStreamTaskListener) secondary).getOutputStream()) | ||
| .toArray(OutputStream[]::new)); | ||
| } | ||
| return outputStream; | ||
| } | ||
|
|
||
| @NonNull | ||
| @Override | ||
| public synchronized PrintStream getLogger() { | ||
| if (printStream == null) { | ||
| printStream = new TeePrintStream( | ||
| teeLogStorage, | ||
| primary.getLogger(), | ||
| secondaries.stream().map(TaskListener::getLogger).toArray(PrintStream[]::new)); | ||
| } | ||
| return printStream; | ||
| } | ||
|
|
||
| @Override | ||
| public void close() throws Exception { | ||
|
Member
There was a problem hiding this comment. Just to note: While I was looking at the code yesterday I realized the logic here is a bit confusing. We close the delegate This means that after a call to
Author
There was a problem hiding this comment. If I update with something like: then
Member
There was a problem hiding this comment. I think you specifically need to use
Author
There was a problem hiding this comment.
Author
There was a problem hiding this comment. But now with the additional
Member
There was a problem hiding this comment. Yes, sorry, I was not saying that we definitely need it, only that if we do need it, we need to close the full
Member
There was a problem hiding this comment. 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? |
||
| Exception exception = null; | ||
| if (primary instanceof AutoCloseable) { | ||
| try { | ||
| ((AutoCloseable) primary).close(); | ||
| } catch (Exception e) { | ||
| exception = e; | ||
| } | ||
| } | ||
| for (BuildListener 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; | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.