Add ActionCable support #309
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Support ActionCable by overriding the `#perform_action` method and wrapping it in an AppSignal transaction. The `#perform_action` is called by ActionCable once a message is send to the channel. We also track subscribe and unsubscribe events as they can also contain complex logic added by the user, which is something we want to track. To track this two "around" callbacks are added to ActionCable on load.
Otherwise the event timeline would only show up if custom/other instrumentation was added/is active. They would be listed without a related (ActionCable) parent event.
|
Small update: Added instrumentation around callbacks so that they always have one action cable event. |
jeffkreeftmeijer
approved these changes
Jun 8, 2017
| allow(Appsignal::Transaction).to receive(:current).and_return(transaction) | ||
| expect(transaction.ext).to receive(:complete) # and do nothing | ||
|
|
||
| # TODO: Nicer way to stub this without a websocket? |
There was a problem hiding this comment.
Could you remove this TODO and create an issue for it, so it doesn't get lost?
There was a problem hiding this comment.
Ah yes, that TODO. I wanted to fix that before submitting it, but looks like it slipped through. I'll have a looksy
There was a problem hiding this comment.
Explained the comment a bit better, but I'll leave the stub like this for now.
Can't find a way to easily create a websocket class for ActionCable. Instead, I'll leave it like this. Best thing to do is create a higher level integration test, but this will do for now.
|
Looks like this doesn't support standalone mode at the moment: http://edgeguides.rubyonrails.org/action_cable_overview.html#standalone |
Standalone ActionCable servers don't have an ActionDispatch request_id in the request env. Instead, we create our own and add it to the request under a private key. This key is then used to save the request id, also for non-standalone servers, and reused throughout the duration of the websocket. This allows us to track all messages on a websocket under the same request_id. This could make searching for all samples on one websocket connection a lot easier.
|
👌 @tombruijn thanks a lot for this |
|
very nice 🎉 |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support ActionCable by overriding the
#perform_actionmethod andwrapping it in an AppSignal transaction. The
#perform_actionis calledby ActionCable once a message is send to the channel.
We also track subscribe and unsubscribe events as they can also contain
complex logic added by the user, which is something we want to track. To
track this two "around" callbacks are added to ActionCable on load.
I've added an example application here for you to test it against: https://github.com/appsignal/appsignal-examples/tree/rails-5%2Baction-cable
Transactions are tested with the data from the extension as per issue #252
Support
https://app.intercom.io/a/apps/yzor8gyw/respond/inbox/conversation/9011790920
https://app.intercom.io/a/apps/yzor8gyw/respond/inbox/conversation/9039706354
Note: Rebased continuation of #275, which I'll close after creating this. Rebased the branch and created a new PR, because I had to rebase it to clean up some really old work that was extracted to separate PRs and things that really weren't necessary anymore. I also didn't want to rebase the original PR and force push it, in case someone is relying on it, breaking their build.