search

slack-ruby / slack-ruby-bot

The easiest way to write a Slack bot in Ruby.
MIT License
1.12k stars 187 forks source link

Log reconnects #244

Closed wasabigeek closed 4 years ago

wasabigeek commented 4 years ago

Attempt to fix https://github.com/slack-ruby/slack-ruby-bot/issues/213

wasabigeek commented 4 years ago

@dblock I only just noticed https://github.com/slack-ruby/slack-ruby-bot/pull/229, would this issue/PR still be relevant?

dblock commented 4 years ago

It's related/similar. This PR is good though, let's finish it!

I'd like to change connected to connected_at, and make it consistent as an attribute like logger, save the timestamp when the reconnect happens. This will allow us to track reconnects after reconnects instead of just 2 states.

Optional, if possible, when the bot goes offline let's log its uptime using this info. Do note that initialize shouldn't set connected_at because that's not an actual connection yet, you want to do that after the bot actually connects.

wasabigeek commented 4 years ago

Optional, if possible, when the bot goes offline let's log its uptime using this info

To clarify, uptime = time when App#stop! is called - time when slack confirmed that bot was first connected? Or is there a more accurate measure?

dblock commented 4 years ago

Optional, if possible, when the bot goes offline let's log its uptime using this info

To clarify, uptime = time when App#stop! is called - time when slack confirmed that bot was first connected? Or is there a more accurate measure?

Ideally you want the time difference between confirmation (callback) of connect and disconnect. Also, make sure to read https://blog.dnsimple.com/2018/03/elapsed-time-with-ruby-the-right-way/ :)

wasabigeek commented 4 years ago

~So time of reconnect - time of last (re)connect? (edit: just read the article, TIL, will take note of that! Conceptually this representation is still helpful for me) Still somewhat an approximation if I understood correctly (since we have to wait for the hook to be called), is that ok?~

I realise I read your comment wrongly. Struggling to find out the time of disconnect - is there an event I can listen to (is goodbye the one to look for)? As far as I understood all the detection of a disconnect is in the Client library. Unless an approximation is fine (i.e. time of reconnect - time of last (re)connect)

dblock commented 4 years ago

I think approximation is fine, but maybe later feel free to raise an event from the client library if it's not already there when the bot detects a disconnect. It would be good to "know" how long it took to reconnect, plus it can be hours, where we'll be reporting wrong "uptime". So I guess we shouldn't call it uptime, but just time since last successful connection.

wasabigeek commented 4 years ago

Do you think "Successfully ... after #{connected_at - self.connected_at}s" would be a bit confusing, since it's not clear that the time is from last connection till this reconnection? (I would have interpreted it as the time from initiating the current connection to the confirmation of that connection)

dblock commented 4 years ago

Merged, thank you.