honeycombio / beeline-ruby

Legacy instrumentation for your Ruby apps with Honeycomb.
Apache License 2.0
22 stars 32 forks source link

ActionDispatch::Http::Parameters::ParseError with Rack app mounted in Rails #31

Closed qnm closed 5 years ago

qnm commented 5 years ago

I use https://github.com/twitchtv/twirp-ruby as part of a Rails app to provide Twirp RPC support.

I mount the Twirp app inside config/routes.rb as a rack app using mount e.g. mount connections, :at => connections.full_name where connections is a Twirp handler.

The issue I face is that the Rails instrumentation is applied during Twirp/rack requests, and the call to request.param in https://github.com/honeycombio/beeline-ruby/blob/master/lib/honeycomb/integrations/rails.rb#L16 returns the error ActionDispatch::Http::Parameters::ParseError.

One option may be to mount the Twirp app inside config.ru however mounting rack apps in Rails is a supported operation https://apidock.com/rails/ActionDispatch/Routing/Mapper/Base/mount

Thanks in advance for your help.

martin308 commented 5 years ago

@qnm thanks for the report! I'll take a look and see if I can reproduce and fix this issue. I think you're correct in that you should be able to mount racks apps in the standard way and have the beeline support this

martin308 commented 5 years ago

@qnm I've managed to reproduce this! Looking into a fix now

qnm commented 5 years ago

@martin308 Only way I could think of was to supress the exception, which is a pretty blunt mechanism :(

martin308 commented 5 years ago

@qnm I've found a fix! Just trying to write a test to reproduce it and I'll push it up

martin308 commented 5 years ago

@qnm I've pushed up a fix for this here. I wasn't able to reproduce it in a test though. I tried it with a basic twirp handler which reproduced the issue and this fix resolved it. Let me know what you think

martin308 commented 5 years ago

@qnm this has now been released in v1.1.1 of the beeline

qnm commented 5 years ago

Thanks @martin308 I hope to be able to test it today.