Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ElasticSearchDao to support LogStash #22

Merged
merged 1 commit into from
Jun 8, 2017

Conversation

rebnridgway
Copy link

This allows ElasticSearchDao to push data to a LogStash instance set up with the http input plugin instead of an ElasticSearch instance. They expose the same API, but LogStash returns 200 while ElasticSearch returns 201.

@diginc
Copy link

diginc commented Jan 19, 2017

A colleague of mine actually just implemented this exact same thing for the exact same reason; in a local SNAPSHOT build. Good timing +1

@sunshineo
Copy link

What is the benefit of sending to LogStash? Does LogStash do something specific?

@rebnridgway
Copy link
Author

rebnridgway commented Feb 14, 2017

LogStash is Elastic's configurable data-processing and queuing software. Having LogStash feeding logs into Elasticsearch is a very common pattern, it adds another layer of protection in case of failure or overload and it can preprocess data before it is stored.

LogStash presents the same interface as Elasticsearch, but it returns 200 instead of 201, hence this change.

@sunshineo
Copy link

@rebnridgway I see. Thank you very much for the explanation. Our volume is very low. I guess we will continue to hit ES directly.

@jakub-bochenski
Copy link

jakub-bochenski commented Mar 8, 2017

@jesusaurus @R-Gerard Is this project dead? I see last PR was integrated almost a year ago..

We'd prefer to pull a new version from Jenkins update center, instead of building a fork locally..

@R-Gerard
Copy link
Member

R-Gerard commented Mar 8, 2017

@jakub-bochenski I marked this plugin with the adopt-this-plugin tag on the Jenkins wiki last year (i.e. looking for a new maintainer). I'd like to help out more, but I haven't had much time for side projects lately.

@jakub-bochenski
Copy link

@R-Gerard I can see it now, please excuse my banner-blindness :)

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jakub-bochenski
Copy link

jakub-bochenski commented May 10, 2017

@oleg-nenashev I'm new to jenkins plugin maintenance: is there anything preventing us from merging this?
Is there some additional process between approving a MR and merging?

@oleg-nenashev
Copy link
Member

@jakub-bochenski No special organization-wide process. Plugin maintainers are eligible to define the process they prefer.

This allows ElasticSearchDao to push data to a LogStash instance set up with the http input plugin instead of an ElasticSearch instance.
They expose a similiar API.
@jakub-bochenski
Copy link

@oleg-nenashev I've changed the code to simply accept any success (2xx) code, this should be more robust.
WDYT?

@vjm
Copy link

vjm commented May 15, 2017

Can we get this merged?

@diginc
Copy link

diginc commented May 15, 2017

Has anyone besides the committer tested a snapshot version of 9ce0c79 ? The change happened after the code approval.

VJM, I'm curious too, do we need a (foster/permanent) maintainer to get this merged or are there special cases when CloudBees merges unofficial plugin PRs due to popular demand :)

@oleg-nenashev
Copy link
Member

@diginc Jenkins project is independent from CloudBees. There is no official/unofficial PRs as well there is no official/unofficial plugins in the community.

Sometimes Jenkins "core team" goes forward and releases changes in non-maintained without taking ownership , but it happens only in the case of severe regressions and 0-day security issues. For all other changes there is a "adopt a plugin" community process: https://wiki.jenkins-ci.org/display/JENKINS/Adopt+a+Plugin

@jakub-bochenski
Copy link

@diginc I was planning to merge this soon as we have been using this version for a few months now.

I'm not sure what exactly is your concern? Do you see some issues with the recent update -- it's functionally the same as far as 200 and 201 codes are concerned, the only difference is for status codes from 202-299 range.

@diginc
Copy link

diginc commented May 30, 2017

On the surface the latest refactored code looks good but I haven't actually tested so I was just asking who had tested/ran the refactored commit in production

@jakub-bochenski jakub-bochenski merged commit 5078fcc into jenkinsci:master Jun 8, 2017
@vjm
Copy link

vjm commented Jun 9, 2017

@jakub-bochenski @hkbao when do we expect to see this new plugin version out here: https://wiki.jenkins-ci.org/display/JENKINS/Logstash+Plugin ?

@jakub-bochenski
Copy link

@vjm I was intending to try and do a release by the end of this month, depending on the status of other PRs currently active.

In the meantime I'd like to encourage you to try out the master build and test if it works for you before the release.

@vjm
Copy link

vjm commented Jun 14, 2017

@jakub-bochenski we will try it out and let you know! thanks!

@vjm
Copy link

vjm commented Jun 14, 2017

@jakub-bochenski I am not seeing the option from the dropdown after installing the master build.

I am able to see version 1.2.1 in the plugins page.

Any suggestions?

@jakub-bochenski
Copy link

jakub-bochenski commented Jun 14, 2017

@vjm there are no new options introduced here, this just changes the message-posting logic to accept any 2xx codes instead of only 200 (you should be able to use ES backend to post to Logstash now)

@vjm
Copy link

vjm commented Jun 27, 2017

Ok, trying that but not getting any feedback in the UI. @jakub-bochenski I am not seeing anything come through in logstash. Is there a way to validate that this output is actually getting sent somewhere out of Jenkins?

@jakub-bochenski
Copy link

jakub-bochenski commented Jun 27, 2017

@vjm I'm using master build now to push to logstash, this is the config I'm using
screenshot_20170627_114909
Remember you need to add either a wrapper or publisher to the job you are interested in.

The plugin is throwing exceptions in case of errors and those should be visible in Jenkins log

@vjm
Copy link

vjm commented Jun 27, 2017 via email

@jakub-bochenski
Copy link

@vjm you need #28 for that

@vjm
Copy link

vjm commented Jun 27, 2017

@jakub-bochenski any idea when that will be in the master build I can test?

@jakub-bochenski
Copy link

jakub-bochenski commented Jun 27, 2017

@vjm I've rebased the PR from master, you can grab the .hpi file from CI build, but be quick as the retention is only a few days

Please let me know about the results, I'm also testing this on our staging Jenkins instance, but the more eyes the better

@vjm
Copy link

vjm commented Jun 27, 2017 via email

@vjm
Copy link

vjm commented Jul 2, 2017

org.apache.http.NoHttpResponseException: logstash-external-tcp-xxx.xxxxx.com:1234 failed to respond

any suggestions? i can properly send data to this endpoint in other methods.

@vjm vjm mentioned this pull request Jul 2, 2017
@vjm
Copy link

vjm commented Jul 2, 2017

@jakub-bochenski I opened #29 to help address these issues by actually creating a separate Logstash provider DAO.

@jakub-bochenski
Copy link

I'm soon going on vacation for next two weeks and I won't be able to push a new release with this changes -- sorry about that, but some other last minute tasks got in the way.

I will try to do a release the first thing when I come back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants