airbnb / interferon

Signaling you about infrastructure or application issues
MIT License
239 stars 44 forks source link

Gracefully shutdown on SIGTERM #7

Closed ziliangpeng closed 8 years ago

ziliangpeng commented 8 years ago

When SIGTERM is received, the program will catch signal and wrap up what it's doing.

to @wyao @igor47

igor47 commented 8 years ago

i think you probably just want to bail out of the work-intensive loops, so i'm going to review it assuming this.

ziliangpeng commented 8 years ago

Yes, I'm trying to break loop or function call that doesn't complete instantly instead of checking every single line.

ziliangpeng commented 8 years ago

Ah, ok, the unless @request_shutdown for update_alerts seems unnecessary, but for read_hosts it can prevent going into the function and printing one line of log 😛

This is not much of a difference to me though, but your suggestion makes the code slightly cleaner. I'll revise the code.

ziliangpeng commented 8 years ago

@igor47 Revised! Please take another look.

Also could you make a gem v0.0.7 for us? Thanks you!

wyao commented 8 years ago

For making/publishing Airbnb gems: https://airbnb.hackpad.com/Publishing-Private-Gems-via-geminabox-a.k.a-Deploying-Gems-78zO5GDwKNY

Be sure to test your change!

wyao commented 8 years ago

@ziliangpeng I have a totally unrelated request. Is there any way you could sneak in this log line: https://github.com/airbnb/interferon/blob/master/lib/interferon/destinations/datadog.rb#L127

Here: https://github.com/airbnb/interferon/blob/master/lib/interferon/destinations/datadog.rb#L116 ?

It would help us debug datadog client errors in the future by just looking at Kibana. I tested it and the output would look something like: " response was #{resp[0]}:'#{resp[1].inspect}'" => " response was 400:'{\"errors\"=>[\"The value provided for parameter 'query' is invalid\"]}'"

ziliangpeng commented 8 years ago

Hey @wyao , I was noticing the same datadog errors during testing today and I'm more than happy to help. I'll get more context from you off here. 😉

I'll probably submit a change in another PR.

ziliangpeng commented 8 years ago

Thanks @wyao, I can build the gem then. @igor47, we might still need you to put it on rubygems.org.

igor47 commented 8 years ago

built and pushed the new gem version to rubygems. thanks for the contribution!