Switching plugin to use ConsoleAnnotator rather than ConsoleNote #132

jglick · 2018-11-05T22:38:41Z

Supersedes #128, #129, and #130. Fixes JENKINS-11752, JENKINS-39536, and JENKINS-54133. May help with JENKINS-34019 (not tested).

@dblock + @dwnusbaum

jglick · 2018-11-05T22:53:51Z

pom.xml


  <properties>
-    <jenkins.version>2.121.2</jenkins.version>
+    <jenkins.version>2.145</jenkins.version> <!-- to pick up https://github.com/jenkinsci/jenkins/pull/3662 -->


Apparently I should have offered jenkinsci/jenkins#3662 up for backport. At the time it did not seem like a major fix. But in fact this patch will not work without it for freestyle builds or per-step logs (still works for Pipeline whole-build logs): you get a Class for the context and there is no way to look for ColorizedAction.

Since that fix is not in any LTS line yet, I think it would be good to release what we currently have in master for anyone using JEP-210 in recent LTS lines, and then we can release this afterwards as a complete fix for future LTS lines. EDIT: Done with the recently released 0.5.3.

jglick · 2018-11-05T22:56:10Z

src/main/java/hudson/plugins/ansicolor/ColorizedAction.java

+
+/**
+ * Marker for the fact that a build used colorization.
+ * Note that the specific log span(s) are ignored.


Alternately, could go the route of jenkinsci/timestamper-plugin#25 and just have a global option to turn on ANSI coloring (with a specified colormap), and deprecate the step & build wrapper altogether. This would be more in line with how Blue Ocean works: it just assumes that if it sees these escape sequences, it should render them.

jglick · 2018-11-05T22:57:02Z

src/test/java/hudson/plugins/ansicolor/AnsiColorBuildWrapperTest.java

+            System.out.print(html);
+            assertThat(html.replaceAll("<span style=\"display: none\">.+?</span>", ""),
+                allOf(
+                    containsString("[<b><span style=\"color: #1E90FF;\">INFO</span></b>]"),


Checks the subtle case that INFO is wrapped in two tags.

A little awkward how some of the hidden ANSI escape sequences end up nested in other tags, but it works 🤷‍♂️ . i.e. I expected the inner span with style="display: none" to come after the  tag when displaying [INFO] in a colorized Maven build, but that's not the case:

[ \u001B[1;34m INFO  \u001B[m ]

Right, I decided it was not worth the bother to fix.

jglick · 2018-11-05T22:57:52Z

src/test/java/hudson/plugins/ansicolor/AnsiColorBuildWrapperTest.java

+            assertThat(html.replaceAll("<span style=\"display: none\">.+?</span>", ""),
+                allOf(
+                    containsString("[<b><span style=\"color: #1E90FF;\">INFO</span></b>]"),
+                    containsString("<b>--------------&lt; </b><span style=\"color: #00CDCD;\">org.jenkins-ci.plugins:build-token-root</span><b> &gt;---------------</b>")));


Makes sure regular HTML escaping is not messed up, and verifies multiple color sequences in the course of a line.

I have interactively run Maven builds in this branch and it all looks right.

jglick · 2018-11-05T22:58:14Z

src/test/java/hudson/plugins/ansicolor/AnsiColorConsoleLogFilterTest.java

-import org.jvnet.hudson.test.LoggerRule;
-
-/**
- * Checks which kinds of console notes are successfully pregenerated for use in a remoted filter.


No longer relevant.

jglick · 2018-11-05T22:59:24Z

src/test/java/hudson/plugins/ansicolor/AnsiColorStepTest.java

                story.j.assertLogContains("TERM=xterm", run);
                assertTrue("Failed to match color attribute in following HTML log output:\n" + html,
-                        html.matches("(?s).*<span style=\"color: #CD0000;\">red</span>.*"));
+                        html.replaceAll("<span style=\"display: none\">.+?</span>", "").matches("(?s).*<span style=\"color: #CD0000;\">red</span>.*"));


Unlike a ConsoleLogFilter, MarkupText offers no way to delete any of the plain text (in this case the ANSI escapes). You can only suppress its display. A bit nasty but seems to work.

jglick · 2018-11-05T23:00:05Z

src/test/java/hudson/plugins/ansicolor/AnsiHtmlOutputStreamTest.java

    }

-    protected String annotate(String text, AnsiColorMap colorMap) throws IOException {
+    private String annotate(String text, AnsiColorMap colorMap) throws IOException {


Reverting this bit from #130.

jglick · 2018-11-05T23:00:46Z

src/main/java/hudson/plugins/ansicolor/ColorConsoleAnnotator.java

+                if (action != null) {
+                    return new ColorConsoleAnnotator(action.colorMapName, ((Run) context).getCharset().name());
+                }
+            } else if (Jenkins.get().getPlugin("workflow-api") != null && context instanceof FlowNode) {


Did not try to verify behavior when the plugin is not in fact installed. Generally speaking, optional deps are dangerous. I would rather just make it a hard dep.

I agree that making it a normal dependency would be safer, but I did some testing with the plugins not installed and things seem to work fine. New freestyle builds are still colored correctly, and executing Jenkins.get().getPlugin("workflow-api") in the script console returns null, so the short-circuiting behavior here looks fine to me.

jglick · 2018-11-05T23:01:42Z

src/main/java/hudson/plugins/ansicolor/ColorConsoleAnnotator.java

+            class EmitterImpl implements AnsiAttributeElement.Emitter {
+                CountingOutputStream incoming;
+                int adjustment;
+                int lastPoint = -1; // multiple HTML tags may be emitted for one control sequence


The triumph of test-driven development over comprehensibility.

dblock · 2018-11-07T12:47:37Z

@dwnusbaum Can you code review/merge this please? I'm going to try to do less work here ;)

dwnusbaum

The changes look good to me, and I didn't find any issues in my interactive testing.

dwnusbaum · 2018-11-08T18:17:59Z

src/main/java/hudson/plugins/ansicolor/ColorConsoleAnnotator.java

+                if (action != null) {
+                    return new ColorConsoleAnnotator(action.colorMapName, ((Run) context).getCharset().name());
+                }
+            } else if (Jenkins.get().getPlugin("workflow-api") != null && context instanceof FlowNode) {


I agree that making it a normal dependency would be safer, but I did some testing with the plugins not installed and things seem to work fine. New freestyle builds are still colored correctly, and executing Jenkins.get().getPlugin("workflow-api") in the script console returns null, so the short-circuiting behavior here looks fine to me.

dwnusbaum · 2018-11-08T18:29:37Z

src/test/java/hudson/plugins/ansicolor/AnsiColorBuildWrapperTest.java

+            System.out.print(html);
+            assertThat(html.replaceAll("<span style=\"display: none\">.+?</span>", ""),
+                allOf(
+                    containsString("[<b><span style=\"color: #1E90FF;\">INFO</span></b>]"),


A little awkward how some of the hidden ANSI escape sequences end up nested in other tags, but it works 🤷‍♂️ . i.e. I expected the inner span with style="display: none" to come after the  tag when displaying [INFO] in a colorized Maven build, but that's not the case:

[ \u001B[1;34m INFO  \u001B[m ]

dblock · 2018-11-08T19:11:21Z

I hit merge.

reinholdfuereder · 2018-11-22T07:31:14Z

Thanks for the improvements/fixes: besides the aforementioned Jenkins issues I think this also really solves JENKINS-41200, JENKINS-46324 and JENKINS-43107

cmurphy208 · 2020-04-14T16:52:40Z

@jglick Does this cover the pipeline.log file that contains all the output during a pipeline?

Is there a way to completely strip the ansi codes for this pipeline.log file? (or replace them with html tags)?

jglick · 2020-04-14T17:02:45Z

Does this cover the pipeline.log file that contains all the output during a pipeline?

Not sure what exactly you are asking.

Is there a way to completely strip the ansi codes for this pipeline.log file?

Only by displaying the …/console link the Jenkins GUI.

cmurphy208 · 2020-04-14T21:08:50Z

Just to clarify for all other parties:
I was talking about the pipeline/build log accessible via blue ocean UI (i.e. the pop-out button above the comment section). It appears that this was out of the scope of this change according @jglick
I intend to file a blue-ocean plugin bug on his suggestion.
@jglick can you advice the preferred way to have a discussion with your team in the future?

jglick · 2020-04-14T21:48:47Z

Yes Blue Ocean has its own handling of ANSI escapes which was never compatible with the ConsoleNotes formerly used in this plugin—one of the motivations for getting rid of the notes. If Blue Ocean does not process escapes in the full log view, that is a Blue Ocean issue.

jglick added 3 commits November 5, 2018 17:33

Switching plugin to use ConsoleAnnotator rather than ConsoleNote.

a6548c4

This ought to be treated as a larger release.

6127b45

FlowNode annotation was not working.

2c65175

dwnusbaum self-requested a review November 5, 2018 22:49

OK, I am jenkinsci#132.

4aef746

jglick commented Nov 5, 2018

View reviewed changes

kuisathaverat approved these changes Nov 6, 2018

View reviewed changes

dwnusbaum mentioned this pull request Nov 6, 2018

Release v. Next #131

Closed

Merge branch 'master' into ConsoleAnnotator

2f6981e

dblock assigned dwnusbaum Nov 7, 2018

kuisathaverat mentioned this pull request Nov 8, 2018

[Jenkins] Go Agent Pipeline version 0 elastic/apm-agent-go#292

Merged

dwnusbaum approved these changes Nov 8, 2018

View reviewed changes

dblock merged commit 829e6e6 into jenkinsci:master Nov 8, 2018

jglick deleted the ConsoleAnnotator branch November 8, 2018 20:07

jglick mentioned this pull request Nov 15, 2018

sgr0 not supported anymore #135

Closed

dwnusbaum mentioned this pull request Nov 16, 2018

Allow escape sequences to span multiple lines and support default fg/bg colors #137

Merged

jglick mentioned this pull request Nov 19, 2018

Colors are not displayed immediately after upgrading to 0.6.0 #136

Closed

This was referenced Nov 22, 2018

Make it work in Blue Ocean #108

Closed

BO pipeline view "header line" shows escape sequences for echo step #140

Closed

Escape sequences of echo steps in build log show up in notification emails that include last lines of build log with v0.6.0 #141

Closed

jglick mentioned this pull request Nov 30, 2018

[JENKINS-49262] Apply log filters (e.g. Timestamper, AnsiColor) to module builds jenkinsci/maven-plugin#114

Closed

kapoorlakshya mentioned this pull request Jan 3, 2019

Control characters appearing in job output in 0.6.0 #145

Closed

jamiesnape mentioned this pull request Jan 3, 2019

jenkins: Colored output obscures console and plain text views RobotLocomotion/drake#10334

Closed

dwnusbaum mentioned this pull request Dec 9, 2019

Ansicolor 0.6 is corrupting input for warnings-ng for MSBuild #154

Closed

dwnusbaum mentioned this pull request May 11, 2020

Can't colorize a part of of a Jenkins log. #180

Closed

tszmytka mentioned this pull request Sep 21, 2020

ANSI sequences remain in the /consoleText content #205

Closed

This was referenced Apr 15, 2020

[JENKINS-61907] Developer can see ANSI color in full log view jenkinsci/blueocean-plugin#5055

Open

[JENKINS-54875] Pipeline view "header line" shows escape sequences for echo step jenkinsci/blueocean-plugin#4901

Open

Uh oh!

Switching plugin to use ConsoleAnnotator rather than ConsoleNote #132

Switching plugin to use ConsoleAnnotator rather than ConsoleNote #132

Uh oh!

Conversation

jglick commented Nov 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwnusbaum Nov 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwnusbaum Nov 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwnusbaum Nov 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblock commented Nov 7, 2018

Uh oh!

dwnusbaum left a comment

Choose a reason for hiding this comment

Uh oh!

dwnusbaum Nov 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwnusbaum Nov 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblock commented Nov 8, 2018

Uh oh!

reinholdfuereder commented Nov 22, 2018

Uh oh!

cmurphy208 commented Apr 14, 2020

Uh oh!

jglick commented Apr 14, 2020

Uh oh!

cmurphy208 commented Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jglick commented Apr 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jglick commented Nov 5, 2018 •

edited

Loading

dwnusbaum Nov 6, 2018 •

edited

Loading

dwnusbaum Nov 8, 2018 •

edited

Loading

dwnusbaum Nov 8, 2018 •

edited

Loading

dwnusbaum Nov 8, 2018 •

edited

Loading

dwnusbaum Nov 8, 2018 •

edited

Loading

cmurphy208 commented Apr 14, 2020 •

edited

Loading