Introduce ConsoleLogRequestProcessor#6299
Conversation
|
Marked as 19.5.0, because it's not necessary to be part of 19.4.0. /cc @maheshp |
|
@GaneshSPatil @maheshp Should we merge this? By itself, it does nothing. Just provides the ability to write to a job's console log. |
|
@arvindsv we should be able to merge it. Do you want somebody to review it, I haven't looked at it in detail though. |
|
Yes, please. That would be good. Maybe @GaneshSPatil has seen it? He asked me some questions about it. I'll just request a review from him for now. We can change it later. |
.../thoughtworks/go/server/service/plugins/processor/console/v1/ConsoleLogAppendRequest1_0.java
Outdated
Show resolved
Hide resolved
* For use by plugins to write information to the job console log.
7a79de4 to
f9f6bd8
Compare
| private Integer pipelineCounter; | ||
|
|
||
| @Expose | ||
| private String pipelineLabel; |
There was a problem hiding this comment.
@arvindsv -- Can you help me understand the need for pipelineLabel?
IMO, pipeline-name and pipeline-counter should be enough to identify pipeline instance.
There was a problem hiding this comment.
@GaneshSPatil You're right. Yesterday, when I was writing the documentation for it, I was wondering the same. I'll make a change and remove the need for it. Thank you!
There was a problem hiding this comment.
I think I just added it as it was available in the JobIdentifier object. But, that's not a good reason. :)
There was a problem hiding this comment.
Oh, also: ConsoleService#appendToConsoleLog needs a full JobIdentifier.
There was a problem hiding this comment.
Removed in: #6397
I've verified that it works without the pipeline label.
…age-remove-pipeline-label #6299 - Remove need for pipeline label in console log request.
The only change from v1 is that it does not use camel case in its message.
The only change from v1 is that it does not use camel case in its message.
The only change from v1 is that it does not use camel case in its message.
The only change from v1 is that it does not use camel case in its message.
Mainly for use by plugins to write information to the job console log. This is helpful to understand what's going on when an elastic agent is being brought up by a plugin.
Todo:
Update plugin-api documentation. (See: Add documentation: Elastic agent plugins can log messages to console log plugin-api.go.cd#124 and https://plugin-api.gocd.org/current/elastic-agents/#log-messages-to-job-console)
Update next release notes branch. (See: commit)
Update functional tests? Will check to see if any of them test messages.[Doesn't seem to have any and this PR doesn't change anything, anyway]Testing / trying this out:
Part of GoCD config
How does it look?
Something like this:
Review:
It's not a big change.
ConsoleLogRequestProcessoris the main change. Everything else just supports it.Questions people have asked:
My answer has been: Decided not to restrict. The plugins need to know the job identifier to write to their console log. It’s unlikely they’ll write to the wrong job, since they’ll be using a job identifier provided to them. If there’s a malicious plugin, then it has full access to the filesystem, you don’t really need this processor API to do anything.