Log reconnects

[wasabigeek]

Attempt to fix #213

[wasabigeek]

@dblock I only just noticed #229, would this issue/PR still be relevant?

[dblock]

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]

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]

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]

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]

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]

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)

[wasabigeek]

Should I round this to, say, 6 decimal places?

[dblock]

I would round it up to 1 or 2 decimals and just stick an s at the end of it to keep it short.

[dblock]

Here's a neat trick I use for this:

log = [
 "Successfull ... ",
 connected_at ? ... : nil,
 ...
].compact.join(" ")

Maybe nicer?

[dblock]

Merged, thank you.