Skip to content
This repository was archived by the owner on Jul 23, 2025. It is now read-only.

Don't submit protocol as if it were message data #80

Open
philomory wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Don't submit protocol as if it were message data #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,23 @@ GEM
json (1.8.6-java)
jwt (1.5.4)
metaclass (0.0.4)
mini_portile2 (2.1.0)
mini_portile2 (2.4.0)
mocha (1.1.0)
metaclass (~> 0.0.1)
multi_json (1.12.1)
multi_xml (0.5.5)
multipart-post (2.0.0)
nokogiri (1.6.8)
mini_portile2 (~> 2.1.0)
pkg-config (~> 1.1.7)
nokogiri (1.6.8-java)
nokogiri (1.10.1)
mini_portile2 (~> 2.4.0)
nokogiri (1.10.1-java)
nokogiri (1.10.1-x64-mingw32)
mini_portile2 (~> 2.4.0)
oauth2 (1.2.0)
faraday (>= 0.8, < 0.10)
jwt (~> 1.0)
multi_json (~> 1.3)
multi_xml (~> 0.5)
rack (>= 1.2, < 3)
pkg-config (1.1.7)
power_assert (0.3.0)
rack (1.6.4)
rake (11.2.2)
Expand All @@ -62,6 +62,7 @@ GEM
PLATFORMS
java
ruby
x64-mingw32

DEPENDENCIES
jeweler (~> 2.1.1)
Expand All @@ -71,4 +72,4 @@ DEPENDENCIES
test-unit (~> 3.2.0)

BUNDLED WITH
1.14.6
1.16.3
2 changes: 1 addition & 1 deletion lib/gelf.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
require 'digest/md5'

module GELF
SPEC_VERSION = '1.0'
SPEC_VERSION = '1.1'
module Protocol
UDP = 0
TCP = 1
Expand Down
5 changes: 3 additions & 2 deletions lib/gelf/notifier.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,9 @@ def extract_hash(object = nil, args = {})
args['level'] ||= GELF::INFO
{ 'short_message' => object.to_s }
end

hash = default_options.merge(self.class.stringify_keys(args.merge(primary_data)))
default_message_data = default_options.dup
default_message_data.delete('protocol')
hash = default_message_data.merge(self.class.stringify_keys(args.merge(primary_data)))
convert_hoptoad_keys_to_graylog2(hash)
set_file_and_line(hash) if @collect_file_and_line
set_timestamp(hash)
Expand Down
47 changes: 27 additions & 20 deletions test/test_notifier.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class TestNotifier < Test::Unit::TestCase
Socket.expects(:gethostname).returns('default_hostname')
n = GELF::Notifier.new
assert_equal [[['localhost', 12201]], 1420], [n.addresses, n.max_chunk_size]
assert_equal( { 'version' => '1.0', 'level' => GELF::UNKNOWN, 'protocol' => 0,
assert_equal( { 'version' => '1.1', 'level' => GELF::UNKNOWN, 'protocol' => 0,
'host' => 'default_hostname', 'facility' => 'gelf-rb' },
n.default_options )
n.addresses, n.max_chunk_size, n.default_options = [['graylog2.org', 7777]], :lan, {:host => 'grayhost'}
Expand All @@ -34,16 +34,16 @@ class TestNotifier < Test::Unit::TestCase
end

should "work with hash" do
hash = @notifier.__send__(:extract_hash, { 'version' => '1.0', 'short_message' => 'message' })
assert_equal '1.0', hash['version']
hash = @notifier.__send__(:extract_hash, { 'version' => '1.1', 'short_message' => 'message' })
assert_equal '1.1', hash['version']
assert_equal 'message', hash['short_message']
end

should "work with any object which responds to #to_hash" do
o = Object.new
o.expects(:to_hash).returns({ 'version' => '1.0', 'short_message' => 'message' })
o.expects(:to_hash).returns({ 'version' => '1.1', 'short_message' => 'message' })
hash = @notifier.__send__(:extract_hash, o)
assert_equal '1.0', hash['version']
assert_equal '1.1', hash['version']
assert_equal 'message', hash['short_message']
end

Expand Down Expand Up @@ -94,11 +94,18 @@ class TestNotifier < Test::Unit::TestCase

should "use default_options" do
@notifier.default_options = {:foo => 'bar', 'short_message' => 'will be hidden by explicit argument', 'host' => 'some_host'}
hash = @notifier.__send__(:extract_hash, { 'version' => '1.0', 'short_message' => 'message' })
hash = @notifier.__send__(:extract_hash, { 'version' => '1.1', 'short_message' => 'message' })
assert_equal 'bar', hash['foo']
assert_equal 'message', hash['short_message']
end

should "not treat protocol option as field data" do
@notifier.default_options = {'protocol' => 0, 'host' => 'some_host', 'version' => '1.1' }
hash = @notifier.__send__(:extract_hash, { 'short_message' => 'foo' })
assert_equal 'some_host', hash['host']
assert_equal nil, hash['protocol']
end

should "be compatible with HoptoadNotifier" do
# https://github.com/thoughtbot/hoptoad_notifier/blob/master/README.rdoc, section Going beyond exceptions
hash = @notifier.__send__(:extract_hash, :error_class => 'Class', :error_message => 'Message')
Expand All @@ -107,21 +114,21 @@ class TestNotifier < Test::Unit::TestCase

should "set file and line" do
line = __LINE__
hash = @notifier.__send__(:extract_hash, { 'version' => '1.0', 'short_message' => 'message' })
hash = @notifier.__send__(:extract_hash, { 'version' => '1.1', 'short_message' => 'message' })
assert_match(/test_notifier.rb/, hash['file'])
assert_equal line + 1, hash['line']
end

should "set timestamp to current time if not set" do
hash = @notifier.__send__(:extract_hash, { 'version' => '1.0', 'short_message' => 'message' })
hash = @notifier.__send__(:extract_hash, { 'version' => '1.1', 'short_message' => 'message' })
assert_instance_of Float, hash['timestamp']
now = Time.now.utc.to_f
assert ((now - 1)..(now + 1)).include?(hash['timestamp'])
end

should "set timestamp to specified time" do
timestamp = 1319799449.13765
hash = @notifier.__send__(:extract_hash, { 'version' => '1.0', 'short_message' => 'message', 'timestamp' => timestamp })
hash = @notifier.__send__(:extract_hash, { 'version' => '1.1', 'short_message' => 'message', 'timestamp' => timestamp })
assert_equal timestamp, hash['timestamp']
end
end
Expand All @@ -145,7 +152,7 @@ class TestNotifier < Test::Unit::TestCase

context "datagrams_from_hash" do
should "not split short data" do
hash = { 'version' => '1.0', 'short_message' => 'message' }
hash = { 'version' => '1.1', 'short_message' => 'message' }
datagrams = @notifier.__send__(:datagrams_from_hash, hash)
assert_equal 1, datagrams.count
assert_instance_of String, datagrams[0]
Expand All @@ -163,7 +170,7 @@ class TestNotifier < Test::Unit::TestCase

should "split long data" do
srand(1) # for stable tests
hash = { 'version' => '1.0', 'short_message' => 'message' }
hash = { 'version' => '1.1', 'short_message' => 'message' }
hash.merge!('something' => (0..3000).map { RANDOM_DATA[rand(RANDOM_DATA.count)] }.join) # or it will be compressed too good
datagrams = @notifier.__send__(:datagrams_from_hash, hash)
assert_equal 2, datagrams.count
Expand All @@ -185,7 +192,7 @@ class MyNotifier < GELF::Notifier; end
@notifier.instance_variable_set('@sender', @sender)

srand(1) # for stable tests
hash = { 'version' => '1.0', 'short_message' => 'message' }
hash = { 'version' => '1.1', 'short_message' => 'message' }
hash.merge!('something' => (0..3000).map { RANDOM_DATA[rand(RANDOM_DATA.count)] }.join) # or it will be compressed too good
datagrams = @notifier.__send__(:datagrams_from_hash, hash)
assert_equal 2, datagrams.count
Expand All @@ -201,7 +208,7 @@ class MyNotifier < GELF::Notifier; end

should "throw an error if more than MAX_CHUNKS will be created" do
srand(1) # for stable tests
hash = { 'version' => '1.0', 'short_message' => 'message' }
hash = { 'version' => '1.1', 'short_message' => 'message' }
hash.merge!('something' => (0..3000).map { RANDOM_DATA[rand(RANDOM_DATA.count)] }.join) # or it will be compressed too good
@notifier.max_chunk_size = 10
@notifier.instance_variable_set('@hash', hash)
Expand All @@ -214,7 +221,7 @@ class MyNotifier < GELF::Notifier; end
context "level threshold" do
setup do
@notifier.level = GELF::WARN
@hash = { 'version' => '1.0', 'short_message' => 'message' }
@hash = { 'version' => '1.1', 'short_message' => 'message' }
end

['debug', 'DEBUG', :debug].each do |l|
Expand Down Expand Up @@ -250,7 +257,7 @@ class MyNotifier < GELF::Notifier; end
should "not send datagrams" do
@sender.expects(:send_datagrams).never
@notifier.expects(:extract_hash).never
@notifier.notify!({ 'version' => '1.0', 'short_message' => 'message' })
@notifier.notify!({ 'version' => '1.1', 'short_message' => 'message' })
end

context "and enabled again" do
Expand All @@ -260,7 +267,7 @@ class MyNotifier < GELF::Notifier; end

should "send datagrams" do
@sender.expects(:send_datagrams)
@notifier.notify!({ 'version' => '1.0', 'short_message' => 'message' })
@notifier.notify!({ 'version' => '1.1', 'short_message' => 'message' })
end
end
end
Expand All @@ -269,11 +276,11 @@ class MyNotifier < GELF::Notifier; end
@sender.expects(:send_datagrams).with do |datagrams|
datagrams.is_a?(Array) && datagrams[0].is_a?(String)
end
@notifier.notify!({ 'version' => '1.0', 'short_message' => 'message' })
@notifier.notify!({ 'version' => '1.1', 'short_message' => 'message' })
end

should "not mutate arguments" do
data = { 'version' => '1.0', 'short_message' => 'message', foo: { bar: "BAZ" } }
data = { 'version' => '1.1', 'short_message' => 'message', foo: { bar: "BAZ" } }
original_hash = data.hash

@sender.expects(:send_datagrams)
Expand All @@ -284,8 +291,8 @@ class MyNotifier < GELF::Notifier; end

GELF::Levels.constants.each do |const|
should "call notify with level #{const} from method name" do
@notifier.expects(:notify_with_level).with(GELF.const_get(const), { 'version' => '1.0', 'short_message' => 'message' })
@notifier.__send__(const.downcase, { 'version' => '1.0', 'short_message' => 'message' })
@notifier.expects(:notify_with_level).with(GELF.const_get(const), { 'version' => '1.1', 'short_message' => 'message' })
@notifier.__send__(const.downcase, { 'version' => '1.1', 'short_message' => 'message' })
end
end

Expand Down