diff --git a/src/main/java/jenkins/plugins/logstash/LogstashConsoleLogFilter.java b/src/main/java/jenkins/plugins/logstash/LogstashConsoleLogFilter.java index 54a6ba38..dae097a1 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashConsoleLogFilter.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashConsoleLogFilter.java @@ -3,10 +3,11 @@ import java.io.IOException; import java.io.OutputStream; import java.io.Serializable; +import java.util.logging.Level; +import java.util.logging.Logger; import hudson.Extension; import hudson.console.ConsoleLogFilter; -import hudson.model.AbstractBuild; import hudson.model.AbstractProject; import hudson.model.Run; @@ -14,39 +15,70 @@ public class LogstashConsoleLogFilter extends ConsoleLogFilter implements Serializable { + private static Logger LOGGER = Logger.getLogger(LogstashConsoleLogFilter.class.getName()); private transient Run run; - public LogstashConsoleLogFilter() {}; + + public LogstashConsoleLogFilter() + {}; public LogstashConsoleLogFilter(Run run) { this.run = run; } + private static final long serialVersionUID = 1L; @Override public OutputStream decorateLogger(Run build, OutputStream logger) throws IOException, InterruptedException { - if (build != null && build instanceof AbstractBuild) + /* + * Currently pipeline is not supporting global ConsoleLogFilters. So even when we have it enabled globally + * we would not log to an indexer without the logstash step. + * But when pipeline supports global ConsoleLogFilters we don't want to log twice when we have the step in + * the pipeline. + * With the LogstashMarkerRunAction we can detect if it is enabled globally and if pipeline is supporting + * global ConsoleLogFilters. + * The LogstashMarkerRunAction will be only attached to a WorkflowRun when pipeline supports global + * ConsoleLogFilters (JENKINS-45693). And the assumption is that the marker action is attached before the + * logstashStep is initialized. + * This marker action will also disable the notifier. + * + */ + + /* + * A pipeline step uses the constructor which sets run. + */ + if (run != null) { - if (isLogstashEnabled(build)) - { - LogstashWriter logstash = getLogStashWriter(build, logger); - return new LogstashOutputStream(logger, logstash); - } - else + if (run.getAction(LogstashMarkerAction.class) != null) { + LOGGER.log(Level.FINEST, "Logstash is enabled globally. No need to decorate the logger another time for {0}", + run.toString()); return logger; } + return getLogstashOutputStream(run, logger); } - if (run != null) - { - LogstashWriter logstash = getLogStashWriter(run, logger); - return new LogstashOutputStream(logger, logstash); - } - else + + /* + * Not pipeline step so @{code build} should be set. + * + */ + if (isLogstashEnabled(build)) { - return logger; + if (build.getAction(LogstashMarkerAction.class) == null) + { + build.addAction(new LogstashMarkerAction()); + } + return getLogstashOutputStream(build, logger); } + + return logger; + } + + private LogstashOutputStream getLogstashOutputStream(Run run, OutputStream logger) + { + LogstashWriter logstash = getLogStashWriter(run, logger); + return new LogstashOutputStream(logger, logstash); } LogstashWriter getLogStashWriter(Run build, OutputStream errorStream) @@ -54,13 +86,27 @@ LogstashWriter getLogStashWriter(Run build, OutputStream errorStream) return new LogstashWriter(build, errorStream, null, build.getCharset()); } - private boolean isLogstashEnabled(Run build) + private boolean isLogstashEnabledGlobally() { LogstashConfiguration configuration = LogstashConfiguration.getInstance(); if (configuration.isEnableGlobally()) { return true; } + return false; + } + + private boolean isLogstashEnabled(Run build) + { + if (build == null) + { + return false; + } + + if (isLogstashEnabledGlobally()) + { + return true; + } if (build.getParent() instanceof AbstractProject) { @@ -72,5 +118,4 @@ private boolean isLogstashEnabled(Run build) } return false; } - } diff --git a/src/main/java/jenkins/plugins/logstash/LogstashMarkerAction.java b/src/main/java/jenkins/plugins/logstash/LogstashMarkerAction.java new file mode 100644 index 00000000..51874f85 --- /dev/null +++ b/src/main/java/jenkins/plugins/logstash/LogstashMarkerAction.java @@ -0,0 +1,17 @@ +package jenkins.plugins.logstash; + +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +import hudson.Extension; +import hudson.model.InvisibleAction; + +/* + * A marker for Runs if logstash is enabled globally or set via JobProperty + * When enabled globally or per JobProperty it doesn't make sense to send the data another time via a logstash step in a pipeline or + * the logstashSend notifier + */ +@Extension +@Restricted(NoExternalUse.class) +public class LogstashMarkerAction extends InvisibleAction +{} \ No newline at end of file diff --git a/src/main/java/jenkins/plugins/logstash/LogstashNotifier.java b/src/main/java/jenkins/plugins/logstash/LogstashNotifier.java index d8bb88a4..60a3f6cf 100644 --- a/src/main/java/jenkins/plugins/logstash/LogstashNotifier.java +++ b/src/main/java/jenkins/plugins/logstash/LogstashNotifier.java @@ -88,6 +88,11 @@ public void perform(Run run, FilePath workspace, Launcher launcher, } private boolean perform(Run run, TaskListener listener) { + // globally enabled and active. No need to send the data another time. + if (run.getAction(LogstashMarkerAction.class) != null) + { + return true; + } PrintStream errorPrintStream = listener.getLogger(); LogstashWriter logstash = getLogStashWriter(run, errorPrintStream, listener); logstash.writeBuildLog(maxLines); diff --git a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java index f8210aa3..d0b55e03 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashIntegrationTest.java @@ -284,4 +284,44 @@ public void ansiColorAnnotationsAreNotIncluded() throws Exception JSONObject firstLine = dataLines.get(0); assertThat(firstLine.getJSONArray("message").get(0).toString(),not(startsWith("[8mha"))); } + + @Test + public void enabledGloballyNotifierWillNotSend() throws Exception + { + when(logstashConfiguration.isEnableGlobally()).thenReturn(true); + project.getPublishersList().add(new LogstashNotifier(10, false)); + + QueueTaskFuture f = project.scheduleBuild2(0); + + FreeStyleBuild build = f.get(); + assertThat(build.getResult(), equalTo(Result.SUCCESS)); + List dataLines = memoryDao.getOutput(); + assertThat(dataLines.size(), is(4)); + JSONObject firstLine = dataLines.get(0); + JSONObject lastLine = dataLines.get(dataLines.size()-1); + JSONObject data = firstLine.getJSONObject("data"); + assertThat(data.getString("buildHost"),equalTo("Jenkins")); + assertThat(data.getString("buildLabel"),equalTo("master")); + assertThat(lastLine.getJSONArray("message").get(0).toString(),equalTo("Finished: SUCCESS")); + } + + @Test + public void enabledJobPropertyNotifierWillNotSend() throws Exception + { + project.addProperty(new LogstashJobProperty()); + project.getPublishersList().add(new LogstashNotifier(10, false)); + + QueueTaskFuture f = project.scheduleBuild2(0); + + FreeStyleBuild build = f.get(); + assertThat(build.getResult(), equalTo(Result.SUCCESS)); + List dataLines = memoryDao.getOutput(); + assertThat(dataLines.size(), is(4)); + JSONObject firstLine = dataLines.get(0); + JSONObject lastLine = dataLines.get(dataLines.size()-1); + JSONObject data = firstLine.getJSONObject("data"); + assertThat(data.getString("buildHost"),equalTo("Jenkins")); + assertThat(data.getString("buildLabel"),equalTo("master")); + assertThat(lastLine.getJSONArray("message").get(0).toString(),equalTo("Finished: SUCCESS")); + } } diff --git a/src/test/java/jenkins/plugins/logstash/LogstashNotifierTest.java b/src/test/java/jenkins/plugins/logstash/LogstashNotifierTest.java index 9769748a..3184260f 100644 --- a/src/test/java/jenkins/plugins/logstash/LogstashNotifierTest.java +++ b/src/test/java/jenkins/plugins/logstash/LogstashNotifierTest.java @@ -7,6 +7,7 @@ import hudson.model.BuildListener; import hudson.model.AbstractBuild; import hudson.model.AbstractProject; +import hudson.model.Action; import hudson.model.TaskListener; import hudson.model.Run; import hudson.model.Result; @@ -54,6 +55,7 @@ LogstashWriter getLogStashWriter(Run run, OutputStream errorStream, TaskLi } static class MockRun

, R extends AbstractBuild> extends Run { + Result result; MockRun(P job) throws IOException { @@ -120,6 +122,7 @@ public void performSuccess() throws Exception { verify(mockListener).getLogger(); verify(mockWriter).writeBuildLog(3); verify(mockWriter).isConnectionBroken(); + verify(mockBuild).getAction(LogstashMarkerAction.class); assertEquals("Errors were written", "", errorBuffer.toString()); @@ -156,6 +159,7 @@ public void performBadWriterDoNotFailBuild() throws Exception { verify(mockListener).getLogger(); verify(mockWriter).writeBuildLog(3); verify(mockWriter).isConnectionBroken(); + verify(mockBuild).getAction(LogstashMarkerAction.class); assertEquals("Error was not written", "Mocked Constructor failure", errorBuffer.toString()); } @@ -197,6 +201,7 @@ public void performBadWriterDoFailBuild() throws Exception { verify(mockListener).getLogger(); verify(mockWriter).writeBuildLog(3); verify(mockWriter, times(2)).isConnectionBroken(); + verify(mockBuild).getAction(LogstashMarkerAction.class); assertEquals("Error was not written", "Mocked Constructor failure", errorBuffer.toString()); } @@ -248,6 +253,7 @@ public Object answer(InvocationOnMock invocationOnMock) throws Throwable { verify(mockListener).getLogger(); verify(mockWriter).writeBuildLog(3); verify(mockWriter, times(3)).isConnectionBroken(); + verify(mockBuild).getAction(LogstashMarkerAction.class); assertThat("Wrong error message", errorBuffer.toString(), containsString(errorMsg)); } @@ -270,6 +276,7 @@ public void performAllLines() throws Exception { verify(mockListener).getLogger(); verify(mockWriter).writeBuildLog(-1); verify(mockWriter, times(2)).isConnectionBroken(); + verify(mockBuild).getAction(LogstashMarkerAction.class); assertEquals("Errors were written", "", errorBuffer.toString()); } @@ -292,6 +299,7 @@ public void performZeroLines() throws Exception { verify(mockListener).getLogger(); verify(mockWriter).writeBuildLog(0); verify(mockWriter, times(2)).isConnectionBroken(); + verify(mockBuild).getAction(LogstashMarkerAction.class); assertEquals("Errors were written", "", errorBuffer.toString()); }