[RFC] add HydrationFactory as extension point for tracking hydrations

[kbond]

This is a proposed extension point to help solve doctrine/DoctrineBundle#109. I'm thinking a HydratorInterface that AbstractHydrator implements should be added but wanted to get some feedback/input before going further.

The doctrine bundle could use this to add hydration times to the timeline:

The doctrine bundle could add something similar to https://github.com/debesha/DoctrineProfileExtraBundle#screenshots to the profiler panel.

You can see an example of how this could be used in DoctrineBundle here: https://github.com/kbond/symfony-reproducer/tree/hydration-profiler-poc (specifically kbond/symfony-reproducer@9a4e91b).

I don't think this causes any performance problems for hydration (when using the DefaultHydratorFactory).

[beberlei]

I am torn, because it will encourage people to overwrite the existing hydrators, which are a bit too complex. Besides tracking the time there are not really many other use cases to use this extension point.

[kbond]

Any other ideas how to achieve? I really think this is an important metric to make available.

Maybe a HydrationLogger - similar to CacheLogger? Maybe a HydrationEvent?

[ostrolucky]

I would be fine with this. This solves one of the inconsistencies between ODM and ORM, as ODM has this option. And I'm not a fan of not having features just because you can abuse them. I think ORM learned this lesson as well, since multiple important features have been deprecated years ago, only to get comeback recently and de-deprecated.

[kbond]

Rebased to 3.4 with the hope of continuing this discussion 😄 🙏🏻

[github-actions]

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

[ostrolucky]

This seems pretty simple and straightforward. Anything against merging it (after rebase and changing target branch ofc) @greg0ire ?

[greg0ire]

I think we should take @beberlei 's concern into account. A HydrationLogger sounds good, although let's maybe not use a name that locks down too much what this is supposed to be used for. Maybe a name that includes "instrumentation", in reference to https://martinfowler.com/articles/domain-oriented-observability.html ?

[ostrolucky]

I don't see how adding some instrumentation hydrator would work, since this hydrator needs to wrap all the other hydrators and needs to be flexible so that bundle can execute custom logic before/after hydration. It's not even clear how would user collect the hydration result if it's in some object they don't instantiate and don't normally have access to.

[kbond]

Maybe a HydrationLogger

I did not have a clear idea here and honestly, I feel like this would add overhead we don't want.