Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Twirp::Service “around { ... }” Hook #66

Open
dinonuggies1 opened this issue Apr 21, 2021 · 3 comments
Open

New Twirp::Service “around { ... }” Hook #66

dinonuggies1 opened this issue Apr 21, 2021 · 3 comments

Comments

@dinonuggies1
Copy link

dinonuggies1 commented Apr 21, 2021

The twirp-ruby package provides great out of the box hooks (before,on_success,on_error,exception_raised) pre and post the twirp service handler invocation. For most cases, I believe this implementation contains the required functionality to augment the call for a given rpc method, but I have recently discovered a need to yield the execution of an rpc call with Twirp specific routing information.

The Twirp wire protocol uses the POST request method for every rpc call. We have database middleware components in place that translates those Twirp POST request as requiring a :writing connection to the database, which may create unnecessary strain on our DB primaries, when in most cases, only a :reading connection is required, thereby hitting the replicas and reducing pressure on the primaries.

I'm looking for a way to wrap twirp handlers in read-only DB connections utilizing ActiveRecord::Base.connected_to(role: :reading) { ... }

So far, I've managed to come up with an alternative solution by creating a module that selects the appropriate connection role given a list of rpc methods that require :writing connections. This module is then prepended to the Twirp::Service class

  module CallRpcWithDatabaseSelection
    # Override twirp-ruby's `#call_handler` with our own database selection logic.
    def call_handler(env)

        write_connection_methods = @handler.class.write_connections_list
        connection_role = if !write_connection_methods.nil? && write_connection_methods.include?(env[:ruby_method].to_sym)
          :writing
        else
          :reading
        end

        ActiveRecord::Base.connected_to(role: connection_role) do
          # call the definition of #call_handler from the ruby gem
          super
        end
    end
  end

  class Twirp::Service
    prepend CallRpcWithDatabaseSelection
  end

While this accomplishes the functionality I need, it is certainly a bit of a hack.

I was wondering if there was a better/alternative solution that would be recommended to accomplish this task more optimally, or if you think this could be a great use case for an around{ ... } hook or a similar idea that would allow yielding execution of RPC methods with Twirp related information.

@marioizquierdo
Copy link
Collaborator

linking a related issue to use ActiveSupport::Notifications.instrument that would also have benefited from an around { ... } hook: #49

@marioizquierdo
Copy link
Collaborator

marioizquierdo commented Apr 23, 2021

What do you think of this design?

svc = Example::HelloWorldService.new(handler)

svc.before do |rack_env, env|
  env[:beforefoo] = "bar"
end

svc.around do |s, env|
  puts env[:beforefoo] # before hooks were called

  s.call(env) # calls other around hooks, rpc call, and on_success or or_error hooks
  env[:aroundfoo] = "bar"

  puts env[:output] # object returned by rpc_call (env[:output_class] is also available)
  puts env[:twirp_error] # if the output was an error, it is also included here
end

svc.on_success do |env|
  puts env[:successfoo] # on_success and on_error hooks are called after around
end

@dinonuggies1
Copy link
Author

Looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants