state-machines / state_machines

Adds support for creating state machines for attributes on any Ruby class
https://github.com/state-machines/state_machines
MIT License
814 stars 91 forks source link

Add transition & branch debug information #90

Closed joaolrpaulo closed 9 months ago

joaolrpaulo commented 1 year ago

Enrich information about the where the transition executed was defined for easier traceability in runtime.

This will enable me and my team to know where the transition was called during runtime, in order to provide better metrics/logs.

NOTE: Tests added & nothing broke during the implementation.

Snippet of our current usage:

state_machine initial: created do 
    event :finish do
        transition created: :finished ## <- The objective is for the log to point to this line.
    end

    before_transition do |_, transition|
        puts "#{transition.event}: Moving from #{transition.from} to #{transition.to} (defined_in: #{transition.defined_in})"
    end
end
joaolrpaulo commented 1 year ago

@seuros Can we get a couple of 👀 on this? 🙏🏼

We are currently blocked internally because of this. Thanks in advance

joaolrpaulo commented 1 year ago

Hello @seuros

Are you ok with this change ? Is there any chance we can get feedback / get this merged ?

Best

joaolrpaulo commented 1 year ago

Any feedback here?

joaolrpaulo commented 1 year ago

@seuros Any feedback? 🤔

seuros commented 1 year ago

@joaolrpaulo , I will review it this weekend max. Sorry for long delay.

A quick review, it seem that everything is correct. I will test it in a live application

joaolrpaulo commented 1 year ago

We are using this code in production since day 1 of opening this PR. No issues found so far. (Actually this code has been in production for over 8 years, since it was ported from a fork that we made of https://github.com/pluginaweek/state_machine).

seuros commented 1 year ago

@joaolrpaulo I understand, but the runtime memory usage changed since you are transporting a bunch of debug string for each transition. People that profile their code will see a big change in the next version.

So my proposal here :

wdyt ?

joaolrpaulo commented 1 year ago
  1. The use case is explained in the PR description, we would like to have a way to expose information about the place were a certain transition was called.
    • This helps us follow the usage of the state machine since we have hundreds of branches & transition, that could have caused a piece of code to be called.
  2. The proposal here is to have this code in a dedicated repo other than here?

I believe that your proposals, are totally valid.

If you do not mind me asking, what was the performance hit of this change?