Closed JeremyEinfeld closed 7 years ago
Hi @JeremyEinfeld, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?
I have updated the e-mail addresses under my profile.
I haven't reviewed yet, but I notice the travis test failed.
The Travis error does not seem to be due to my changes:
1) LogStash::Outputs::Gelf#send sends the generated event to gelf
Failure/Error: @logger.debug(["Sending GELF event", m])
NameError:
no method 'debug' for arguments (org.jruby.RubyArray,org.jruby.RubyHash) on Java::OrgApacheLoggingLog4jCore::Logger
available overloads:
(org.apache.logging.log4j.Marker,java.lang.String,org.apache.logging.log4j.util.Supplier[])
(java.lang.String,java.lang.Object[])
(org.apache.logging.log4j.Marker,java.lang.String,java.lang.Object[])
(java.lang.String,org.apache.logging.log4j.util.Supplier[])
# ./vendor/bundle/jruby/1.9/gems/logstash-core-5.0.0.pre.beta1-java/lib/logstash/logging/logger.rb:41:in `debug'
# ./lib/logstash/outputs/gelf.rb:193:in `receive'
# ./spec/outputs/gelf_spec.rb:32:in `(root)'
# ./vendor/bundle/jruby/1.9/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `(root)'
@jordansissel please advice
@logger.debug(["Sending GELF event", m])
This should probably be changed to:
@logger.debug("Sending GELF event", event => m)
oh, hmm.. the stack trace says this:
./vendor/bundle/jruby/1.9/gems/logstash-core-5.0.0.pre.beta1-java/lib/logstash/logging/logger.rb:41:in `deb
It is targeting a prereelease of Logstash 5? This seems incorrect.
@JeremyEinfeld please change the code according to Jordan @jordansissel can you take care of the ci error?
Changing @logger.debug(["Sending GELF event", m])
to @logger.debug("Sending GELF event", event => m)
is outside the scope of my pull request, see https://github.com/logstash-plugins/logstash-output-gelf/issues/20
I have synced my fork with the master, so the build now reflects 3bf9a2f8bffe5058a6376ad7038cdf7edb0c226b and 15746abeef708185bca77ffc8c32b75d082c56ee and completes successfully. No changes were made to my commit.
I have updated my commit to use upper-case protocol values, allowing the removal of the need for a .upcase
and allowing the documentation to use the proper capitalization of the abbreviations without causing confusion about what is expected.
LGTM
Jordan Sissel merged this into the following branches!
Branch | Commits |
---|---|
master | 29bd5d088aa871cbd6555c75b94f64fc19dae47b |
logstash gelf output v3.0.0 is now published.
@JeremyEinfeld thank you for this work :)
For those landing on this from Google, the TCP option for logstash's gelf output plugin has been available since v3.0.0 of the plugin. There's a (currently undocumented) option you can add to the output plugin protocol => "TCP"
that switches it to TCP instead of UDP.
No tests yet, but I wanted to get this out there for those that are seeking it.
A caveat: this opens a single, long-running TCP connection to Graylog, preventing a standard TCP load balancer from balancing the load. For our environment, we added an option to open multiple parallel connections which can then be distributed among our Graylog servers via HAProxy: 4e4a4a0a7944f6a3d32cf11c63c78966a579b42d (some variation of which might be considered for logstash-output-gelf).
This is a solution for logstash-plugins/logstash-output-gelf#1