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

Question / feature request: hooks around call? #111

Open
blocknotes opened this issue Mar 22, 2023 · 6 comments
Open

Question / feature request: hooks around call? #111

blocknotes opened this issue Mar 22, 2023 · 6 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@blocknotes
Copy link

Hey :)
I wonder if there is a way to execute some callback code before / after / around the call method of an actor.

My use case is the following:

  • I have an actor which plays actors in a sequence (is this a playbook? an orchestrator? something else?)
  • My actors inherit from ApplicationActor (which simply has include ServiceActor::Base)
  • I need to log the execution of each actor.

I tried to add to ApplicationActor the following:

  class << self
    def call(**args)
      ActiveSupport::Notifications.instrument("ApplicationActor", extra: self) do
        super
      end
    end
  end

but it gets executed only on the call of the "main" actor.

Any idea?

@sunny
Copy link
Owner

sunny commented Mar 22, 2023

Hi there!

I’m afraid there’s currently no built-in instrumentation or hooks.

You could try and override the _call method on actors. But, as its name suggests, it’s an internal API, so there’s no guarantee that overriding it will stay in place.

class ApplicationActor
  def _call
    ActiveSupport::Notifications.instrument("ApplicationActor", extra: self) do
      super
    end
  end
end

I’d be happy to accept a pull-request to add ActiveSupport instrumentation to actor-rails or some kind of hook syntax such as around_call … in this repo if you’d be willing to try that out 🙏🏻

@blocknotes
Copy link
Author

Thank you for your feedback @sunny - I'll think about proposing something to you.
I'm not a big fan of callbacks but having something easy for instrumentation seems a nice value to me.

@blocknotes
Copy link
Author

I don't have a good solution for now to avoid using the private interface method _call.

Currently I'm following the @sunny hint.
Adding here some code if it can help someone else:

# app/lib/instrumentable_actor.rb
module InstrumentableActor
  # NOTE: rely on a private method _call
  def _call(**_args)
    extra = { class: self.class, args: result.to_h }
    ActiveSupport::Notifications.instrument("ApplicationActor", extra:) do
      super
    end
  end
end

# app/actors/application_actor.rb
class ApplicationActor
  include ServiceActor::Base
  include InstrumentableActor
end

# config/initializers/notifications.rb
ActiveSupport::Notifications.subscribe("ApplicationActor") do |event, start, finish, id, payload|
  Rails.logger.info do
    extra = payload[:extra]
    "[#{id}] #{event} - #{extra[:class]} #{extra[:args]} - start: #{start} - finish: #{finish}"
  end
end

The logs have this form:

[081cd796c6441ff4e6db] ApplicationActor - MyActor {:par1=>"some value"} - start: 2023-03-30 16:37:55 +0200 - finish: 2023-03-30 16:37:55 +0200

@sunny
Copy link
Owner

sunny commented Mar 30, 2023

Perhaps the ActiveSupport::Notifications.instrument call you have could be added directly to ServiceActor::Base inside the service_actor-rails gem so that you don’t have to rely on an internal method.

Would you be willing to try a pull-request over there with that change?

@blocknotes
Copy link
Author

Hey @sunny.
I could try but how could I avoid the _call method?
I tried also directly prepending the instrumentation on the call instances but I got some issues and it seems a workaround to me (also because the descendants must be loaded ahead also in development with eager load or manually requiring the child actor classes).

@sunny
Copy link
Owner

sunny commented Apr 3, 2023

I think it’s fine for actor-rails to use actor’s internal _call method. Although they are two different gems, their develpment is tightly linked so it’s much less of an issue than if the method is overridden in your app.

Another option would be to introduce a callback mechanic in actor so that actor-rails doesn’t rely on _call. For example this could be an around class method that can accept a block. That would be ideal and allow more use-cases but it might take longer to build since it would mean changes to both actor and actor-rails.

@sunny sunny added the enhancement New feature or request label Apr 24, 2023
@sunny sunny added the help wanted Extra attention is needed label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants