sensu / sensu-transport

The Sensu transport abstraction library.
MIT License
14 stars 19 forks source link

Improve rabbitmq transport logging on connection errors #34

Closed moises-silva closed 7 years ago

moises-silva commented 8 years ago

This should fix issue #30

cwjohnston commented 7 years ago

Closes #30

portertech commented 7 years ago

@moises-silva @cwjohnston I would love to see log lines like https://github.com/sensu/sensu/blob/master/lib/sensu/daemon.rb#L256, using "transport connection error" as the message, then use :error or :reason as context keys.

portertech commented 7 years ago

Otherwise these log events lack context.

cwjohnston commented 7 years ago

@portertech so far this lib hasn't pulled in sensu-logger for testing, using Ruby's built-in Logger class instead. It seems like Logger doesn't accept the same data hash argument Sensu::Logger uses for providing additional context. Should we change the tests to bring in Sensu::Logger?

portertech commented 7 years ago

@cwjohnston yes, let's pull it in without a version constraint.

portertech commented 7 years ago

Fantastic!

1486

cwjohnston commented 7 years ago

@portertech disregarding the one failure here, i think this is good to merge.

I believe the single failed platform in question relates to my accidentally pushing (and subsequently deleting) the branch of same name on this repo. I had meant to push to sangoma/sensu-transport fork to update this pull request, and I guess travis decided they would be the same PR? 🤔

portertech commented 7 years ago

@cwjohnston ok, going to merge this, we can address the build failures if they continue on jruby.