Add custom proc support to FrontendLogger, simplify FrontendLogController#9117
Conversation
…ller error reporting
… its thing - bind_call calls a specific method implementation, so it doesn't go through the Enhancer implementation
This reverts commit ae086b4.
The method signature is lost if read directly from analytics instance, since it's overridden by AnalyticsEventsEnhancer. We need the original method reference
| if analytics_method.is_a?(Symbol) | ||
| analytics.send( | ||
| analytics_method, | ||
| **hash_from_kwargs(attributes, AnalyticsEvents.instance_method(analytics_method)), |
There was a problem hiding this comment.
9e28cb8 fixes an issue with some specific events in IdV not working. I think this is also what transform_values was helping with previously, since trying to get the method signature from the analytics instance is going to have issues with how IdV::AnalyticsEventsEnhancer overrides the method and loses the signature.
| [3] pry(#<FrontendLogger>)> analytics.method(analytics_method)
=> #<Method: Analytics(Idv::AnalyticsEventsEnhancer)#idv_personal_key_acknowledgment_toggled(**kwargs) /identity-idp/app/services/idv/analytics_events_enhancer.rb:39>
We could also consider moving this back into transform_values, and maybe there's even something where we can consolidate the handling so that the EVENT_MAP hash values are all the same type of callable thing? The issue comes with binding though... maybe we can have a small logic branch to normalize an unbound method to bind it with analytics?
analytics_method = analytics_method.bind(analytics) if analytics_method.is_a?(UnboundMethod)
analytics_method.respond_to?(:call)
analytics_method.call(**hash_from_kwargs(attributes, analytics_method))
else
analytics.track_event("Frontend: #{name}", attributes)
endThere was a problem hiding this comment.
Re: bound vs unbound... I feel like we should have only one or the other? Seems like a lot of handling to manage both
There was a problem hiding this comment.
@zachmargolis How do you feel about the current implementation in this branch?
There was a problem hiding this comment.
I feel good! 🚢
2 participants
I nerd-sniped myself based on this comment and gave it a shot