Ability to send custom data to logstash #39
Conversation
jakub-bochenski
left a comment
jakub-bochenski
left a comment
There was a problem hiding this comment.
Please add new test cases like e.g. 28b4ecd
We'd like to follow a "single assertion per test" rule for new code
| @@ -0,0 +1,4 @@ | |||
| <div> | |||
| <p>Custom json string to send to Logstash.<br/> | |||
There was a problem hiding this comment.
please change the code to parse and validate the json on input
|
|
||
| @DataBoundConstructor | ||
| public LogstashNotifier(int maxLines, boolean failBuild) { | ||
| public LogstashNotifier(int maxLines, boolean failBuild, String customData) { |
There was a problem hiding this comment.
this will make customData a mandatory argument
this is a no-no as it will break existing usage
please use @DataBoundSetter instead
| protected String displayName; | ||
| protected String fullDisplayName; | ||
| protected String description; | ||
| protected JSONObject custom; |
There was a problem hiding this comment.
BuildData doesn't seem a good place to store this.
This data is not related to a build instead being taken from static configuration. We also need a way to add this that doesn't force a change in so many places (make it more SOLID)
|
Hi, would you check if my script addition in #40 would cover your needs as well any custom needs going forward? With the script it would be possible to modify the JSON object in any way you desire. |
|
I will check #40 If I can make use of it I prefer using it. |
|
@cdivitotawela , looking at your code it seems that you want to add this data to pos-build action. Am I correct? If so, presently the other pull request does not allow it. But I can look at adding the script option to the post build action as well after this one is finalized, shouldn't be hard. Just don't want to bloat the existing pull request even more. |
|
@akostadinov Just to give little background to my requirement. In our environment we create docker images for the applications and push to AWS Container registry and we only use Jenkins pipelines as well. Once the docker build completes we need to store the information about the build in Elasticsearch. Current code push all the build related information to logstash (then to Elasticsearch). We also want to add additional information this the pushed record such as docker image and docker tag, so that we can monitor the job builds via Elasticsearch. While this one of the use cases, I believe there might be many other custom information we would like to store as part of the Jenkins build information in Elasticsearch. Apologies for a poor implementation, as I am not a regular Java developer (more of sysadmin background). I am very happy to see all the feedback. If you can implement a solution which can satisfy the above requirement, I am more than happy to use your solution and change my pipeline code. I also analyse and study more about the code. |
|
@cdivitotawela , that is possible. You would just have code like: Just presently my pull request is about the line-by-line build wrapper and I see in your code that you are using the post-build action. I will add this once the line-by-line is merged. Just want to reduce scope of the pull request as much as possible. And there are still 1-2 open questions in there. |
|
@akostadinov Grand. I will give it a try and see. Thank you. |
|
@cdivitotawela did you find time to check #40? Can we close this one? |
|
Lets reject and close this. I will check the other PR when I get time. Thanks. |
3 participants
Description
Following change allows sending some custom data along with the standard build data. This help us sending additional data to logstash (ex: docker image and tag in docker build). While this change works, there could be number of changes may required to make it better and approved for merge. I expect the community can help by advising the changes required on this.
Change
Introduced additional input parameter customData which should be valid JSON string. CustomData is parsed and added as field custom in logstash input.
Backward Compatibility
CustomData has default value {} which is represents empty JSON string. Need some help from the experts whether this enough or better approach available.
Unit Tests
Currently lack additional unit tests to validate the custom data. Further existing unit tests updated to send the customData string also. Additional advice on this area is highly appreciated.