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

Backporting the driver middlewares feature from 3.0 to 2.x #4495

Closed
ste93cry opened this issue Feb 9, 2021 · 5 comments
Closed

Backporting the driver middlewares feature from 3.0 to 2.x #4495

ste93cry opened this issue Feb 9, 2021 · 5 comments
Labels

Comments

@ste93cry
Copy link
Contributor

ste93cry commented Feb 9, 2021

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

#4157 introduced the concept of middlewares to wrap the driver. This extension point is ideal for libraries that supports distributed tracing (OpenTracing, Sentry, etc) because it allows to measure the performances of queries at the lowest level possible. Unfortunately, the feature is present only from DBAL 3.0 onwards. Would be acceptable to backport it (just the middleware part of course, it's not necessary to backport also the moving of the portability layer which I didn't even investigate if it's feasible without BC breaks) to 2.x? Right now, on that version the only possible thing that can be done to wrap the driver is to listen to the postConnect event (which btw happens after a transaction has been started if autocommit is false) and use the reflection to change a private property of the Connection class.

@morozov
Copy link
Member

morozov commented Feb 13, 2021

There's no plan to tag any new 2.x releases with the exception of improving the upgrade path to 3.0. This improvement doesn't fall into the "upgrade path" category.

@ste93cry
Copy link
Contributor Author

ste93cry commented Feb 15, 2021

Understood. Yes this request does not strictly falls into the "upgrade path" category, but it would help in transitioning from 2.x to 3.x mainly because of 3.x supports only PHP >= 7.3. While it's absolutely true that PHP 7.2 is EOL, a lot of people are still using it and libraries like Sentry cannot simply drop its support (although I would really like to do it, trust me). Since there is some work being done to reintroduce support of PHP 7.2 in 2.x, I was hoping to uniformate the two versions in terms of this feature to avoid having to use an hack in the older one to wrap the driver

@morozov
Copy link
Member

morozov commented Feb 15, 2021

If Sentry implements what it's working on only for DBAL 3 which in turn requires PHP 7.3, and this is a useful feature for its users, it should motivate them to upgrade to the PHP and the DBAL versions that support it, so it will be a win-win for everybody. Backporting new features to the old versions and then maintaining multiple versions of everything by each party looks more of a lose-lose to me.

@ste93cry
Copy link
Contributor Author

I tend to agree with your thoughs, but there are a few key points about why we cannot do what you propose:

  • Sentry is a (but not only that) logging library, and as such it must support a wide range of users regardless of their PHP version (unfortunately, not my choice as I'm just a contributor there)
  • The DBAL 3.x is not supported by the ORM yet. Since the feature I'm developing there is for Symfony, and imho who uses Symfony is more likely to also use the ORM, we simply cannot avoid supporting DBAL 2.x. At the same time, we cannot wait for the ORM to support the newest DBAL version because we simply don't know when this will happen

Anyway, I got the point, I don't want to press to backport this feature if you don't think it's worth doing this. I tried, I failed 😄

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants