Add ActionCable support

[tombruijn]

Support ActionCable by listening for perform_action.action_cable
events. This allows us to create performance and exception samples for
ActionCable events.

Closes #273

To discuss

• How to add parameters to transaction? Add Transaction#params= #295
• Can we store the current request env?
• Register subscribe and unsubscribe events from ActionCable channels

Support

https://app.intercom.io/a/apps/yzor8gyw/respond/inbox/conversation/9011790920
https://app.intercom.io/a/apps/yzor8gyw/respond/inbox/conversation/9039706354

Events of interest

• transmit_subscription_confirmation.action_cable
• transmit_subscription_rejection.action_cable
• broadcast.action_cable - for message transmissions?

[thijsc]

Interesting approach, looks good.

So this would create a transaction for every message you send over the socket right?

[tombruijn]

Yes, this creates a transaction for every message you process.

The actual transmission is part of the transmit.action_cable event, which is triggered separately from perform_action.action_cable. This makes it quite hard to track it in the same transaction. Also, there may be more than one of those events, depending on how many subscribers you have to the channel. So if we would track it, we probably end up with an event for every subscriber. Now that I think about it, probably not that useful to track unless it's something that's very slow, and so far I've only seen it hit 1ms.

[thijsc]

Maybe it makes sense to group all subscribers for a single event in one transaction?

I suggest putting these transactions in their own namespace, that way they don't mess with the normal metrics.

[tombruijn]

I don't know how we could group them together. The transmit.action_cable events are triggered without a parent instrumentation event. Simply listening in on the events wouldn't really work.
It's probably not worth the hassle for a first version unless we hear people really need it.

I can make a separate namespace for it of course. What shall we call it? action_cable or something more generic so that we can use the same name for in Elixir Phoenix? websockets / channels

[thijsc]

I think action_cable is the clearest, there's no need for the name to be the same for different technologies in this case.

[tombruijn]

✅ Namespace updated.

Also updated how we track ActiveSupport events, see the first commit. Necessary to also track the perform_action.action_cable event itself, otherwise you'd only get events that were triggered within that event.

Without the change:

With the change:

[tombruijn]

Pushed a new version of this yesterday evening.

We now track subscribe and unsubscribe events. We also track the request environment, session data, etc. for the requests. However, we (re)use the request data from the original websocket request, because technically there is only one request.

Downside of this new version is that we don't track the parameters for the perform_action.action_cable event anymore. The parameters of the originally websocket request are used here too. We need to be able to set the parameters later on for the transaction and don't have it get overwritten when Transaction#sample_data is called before completing the transaction:

appsignal-ruby/lib/appsignal/transaction.rb

Line 239 in 1a2104c

:params => sanitized_params,

[tombruijn]

Closing in favor of #309. (I had to rebase and clean up my earlier attempt at getting this to work.)

[ipepe]

@tombruijn Any chance to explain why You used class_eval in

appsignal-ruby/lib/appsignal/hooks/action_cable.rb

Line 24 in 8bfcb62

ActionCable::Channel::Base.class_eval do

? I brought issue of handling actioncable exceptions by airbrake here: airbrake/airbrake#885
and we were wondering about chosen technique You used to monkey-patch that class.

[tombruijn]

Hi @ipepe ,

I made the final implementation in PR #309.
I remember wanting to use all the callbacks ActionCable offers, like was done here:

appsignal-ruby/lib/appsignal/hooks/action_cable.rb

Line 57 in 8bfcb62

ActionCable::Channel::Base.set_callback :subscribe, :around, :prepend => true do |channel, inner|

but it was not possible to track the action actions as such.

Why I used class_eval is because our other hooks use it as well. That's because we monkeypatch the classes in the install methods of the hooks rather than require an additional file that overrides the methods. I think it's easier to ensure you're overriding a class's implementation rather than define a new one.

If the Airbrake implementation simply loads a file when something needs to be monkeypatched there's no need for the class_eval approach.

[ipepe]

I guess that explains everything I wanted to know, thank you.