Write NewNodeConsoleNote to per-step logs instead of overall log

[dwnusbaum]

This is currently just a prototype I am using to investigate some things. Requires jenkinsci/workflow-api-plugin#296 to fix a test that happens to trigger that bug with this change.

I am working on a (proprietary) Pipeline log viewer that allows users to view logs for an entire stage or branch, by combining the per-step logs for all the steps in the stage/branch. This works fine, but there are a few issues:

• You lose the [Pipeline] <step> lines from NewNodeConsoleNote that give you context about what steps are running, which can be confusing if the stage/branch has a lot of steps.
  • There is no supported way to include only the relevant step announcements using public APIs today, you have to either include or exclude lines from the overall log.
• It is difficult to correctly identify where to put context markers indicating when log output changes from one branch to another (as in the left gutter in the classic console). Given two [Pipeline] <step> lines in a row, there is no exact way to mark their context.
• It is very awkward to try to identify "meaningful" lines from the overall log, e.g. Jenkins restarting, exceptions at the end of a build, etc.

Emitting the NewNodeConsoleNote lines to the per-step logs instead allows all of these issues to be addressed using public APIs. I think it could be useful to the classic console view and other Pipeline log visualizations as well.

Of course there are some drawbacks:

• Visualizations that only show logs for a single step will show at least 1 new log line, i.e. [Pipeline] <step name>, which is at best useless for these visualizations since they already indicate what step is selected in some other way.
• Various changes would need to be made to preserve the show/hide behavior in the classic console view.
• I would not be surprised if this breaks tests in various plugins, but I will check at least all BOM plugins if I decide to pursue this change further.
• Probably some other things I haven't thought of.

Testing done

### Submitter checklist
- [ ] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [ ] Ensure that the pull request title represents the desired changelog entry
- [ ] Please describe what you did
- [ ] Link to relevant issues in GitHub or Jira
- [ ] Link to relevant pull requests, esp. upstream and downstream changes
- [ ] Ensure you have provided tests - that demonstrates feature works or fixes the issue

[dwnusbaum]

This issue is kind of interesting. I am not sure if it has been reported previously, but any console notes that rely on a Run context totally break the per-step log view in the Pipeline steps view. (The notes aren't just skipped, they cause a ClassCastException here and break all log display!)

Perhaps LogStorage.stepLog should really return a AnnotatedLargeText<WorkflowRun> instead? Not sure if that can be changed compatibly at this point. ConsoleAnnotationOutputStream at least could probably be made more robust though.

There do not seem to be many implementations that would trigger the issue, see here and here.

[jglick]

Hmm, does this have anything to do with jenkinsci/jenkins#3662?

[dwnusbaum]

Only tangentially I think, in that the whole API seems wrong. The problem as I see it is that when you create a ConsoleNote, you get to choose an arbitrary type parameter, but when using a ConsoleAnnotationOutputStream, the type parameter is fixed, so code like this is unsound, because the context type of an arbitrary note in the stream is not necessarily T. I think the compiler is only ok with that code because of erasure. If you replace final ConsoleNote a = ... with final ConsoleNote<?> a = ..., then you get the incompatible types: T cannot be converted to CAP#1 error that I would have expected.

Looking at actual usage, I think the context parameters in all associated classes should have just been hard-coded to be Run, and LogStorage.stepLog should have just returned AnnotatedLargeText<WorkflowRun> like overallLog. I guess KK's API design assumption was that you only write notes when you know they will be read with the same context, which doesn't work for Pipeline when we read one log file but return AnnotatedLargeText objects with different contexts.

As far as what we can do compatibly, we could at least catch ClassCastException somewhere in here (I'd have to double-check exactly where the exception gets thrown) so we just skip the note in question. I am not sure if we can get Class<?> witnesses reliably that we could use to pre-check the types instead of just catching exceptions.

[dwnusbaum]

Should be fixable, but I am not going to bother looking into it unless I really want to get this merged.

[dwnusbaum]

For now we are instead going to filter these out when reading the log file for our visualization.