Allow a callable object as the :patterns option of the Trace middleware

[hibariya]

I'd like to pass a Proc as the patterns option for the Trace middleware because I sometimes want to filter classes by a bit different conditions like filtering by the file path (TracePoint#path) rather than module name matching. This change allows users to pass an object that responds to :call as the :patterns option similar to Orthoses::RBSPrototypeRuntime.

[hibariya]

I should have added this option in the previous PR 🙇

[ksss]

@hibariya Thank you for contributing.

The patterns option in Orthoses#Trace was originally added due to performance problems when tracing all objects.

This PR looks to want to do more advanced class/module/method filtering.
This is an issue for the entire build stack, not just a single middleware.

How about to use Orthoses::Filter?

[hibariya]

@ksss Sorry for the very late reaction 🙇 Got it, I'll try using Orthoses::Filter first! Let me close this PR for now.

[hibariya]

Hi, @ksss 👋 I'd like to reopen this since I think we need something to improve the performance of this middleware.

First, I tried Orthoses::Filter to filter out all the classes and modules defined outside of the Rails.root. I'm running my application's RSpec test suite in the run block, and it usually takes around 10 minutes. However, when I tried the Orthoses::Filter approach, it takes much longer time (I waited around an hour and gave up in the middle of the process). I believe it's because TracePoint was tracking all the method calls in the process even the ones that will eventually be filtered out with Orthoses::Filter later.

Then I tried another approach: load all classes and modules defined under Rails.root in advance and specify it as the patterns: option. With this approach, only necessary method calls will be tracked. As I expected, it was better, but was still super slow. I assume it's because even if Targetable#target? prevents unnecessary tracking, the method itself (Targetable#target?) will be called every time and it slows down the entire process (besides, the patterns: array not so small since it contains all module names defined in the Rails app).

Since I believe that with the current Targetable implementation, there's not much room for optimization, to solve the problem above, I think there are a couple of ways:

1. Allow Proc for the patterns: option like this PR:
2. Introduce if: option like Orthoses::Mixin and Orthoses::Constant: it looks more consistent and readable than using patterns:
3. Cache the result of Targetable#target? internally: I haven't tried it yet, but I think it should solve the issue. As a user of this lib, I prefer the other two, but I will try it if you prefer.

What do you think?

[hibariya]

Forgot to reopen 🙏

[ksss]

@hibariya Thank you for the research.
I see your use case now. You're likely trying Orthoses::Trace on test cases in a Rails application, aren't you?
I have two concerns about this change.

1. Orthoses::Trace is designed with small-scale cases, such as testing gems, in mind. Even if output is possible in large-scale cases, the output patterns would likely become so overwhelming that it would ultimately be impractical to use. The output of Orthoses::Trace is only meant to serve as a reference and will likely require manual corrections.
2. Exposing the TracePoint object to users feels overly dependent on the implementation.

[hibariya]

Hi @ksss, sorry for the late reply. Yes, I'd like to generate RBS for existing apps.

The output of Orthoses::Trace is only meant to serve as a reference and will likely require manual corrections.

Yes, as you mentioned, I don't think using the generated RBS as-is for type-checking is practical (at least at this point). That said, it's still useful for supplemental documentation purposes even today. Improving the middleware for this purpose does not always contradict the original design.

Exposing the TracePoint object to users feels overly dependent on the implementation.

Good point. How about removing the TracePoint from the parameters and keep it the same as today (Targetable#target(name)) while accepting something callable (like Proc) as the @patterns?

[ksss]

Good point. How about removing the TracePoint from the parameters and keep it the same as today (Targetable#target(name)) while accepting something callable (like Proc) as the @patterns?

Looks good!
How about the name trace_point_filter?

[hibariya]

How about the name trace_point_filter?

gotcha!

[ksss]

やられたいことが分からなくなったんですが、TracePointオブジェクトが使いたかったんじゃないんですか？
この使い方ならpatternsで十分満たせるように思います。

[hibariya]

私も書いててよくわからなくなっていました。上で背景含めてだらだら書いて長くなってしまっていたんですが、要は少し大きめのアプリケーションで走らせてみたら現実的な時間で終わらなかった (でも走らせたい) ので、 target? を速くしたいというのがこのPRのモチベーションです (このPRを最初に書いた時点では他にもモチベがあって、 TracePoint オブジェクトにも触れるようにしていたのですが、それは他で解決できたので現時点では不要になりました)。

そういう動機を考えると、別途新しい option を導入するよりは、やはり pattern が Proc を取れるほうが自然な気はします。もうひとつ別の方法としては、Proc の中でやることはだいたい cache なので、それをこの middleware の中でやってしまい、API はそのままにする、というのもあるかなと思います。