Giter Site home page Giter Site logo

Symfony SDK 5.0.0 about sentry-symfony HOT 33 CLOSED

cleptric avatar cleptric commented on June 7, 2024 8
Symfony SDK 5.0.0

from sentry-symfony.

Comments (33)

cleptric avatar cleptric commented on June 7, 2024 29

Started working on v5 this week, so shouldn't be that much longer šŸ¤ž

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024 20

Yes, March for sure! Iā€˜m actually planning to ship this next week.

from sentry-symfony.

garak avatar garak commented on June 7, 2024 13

I suggest dropping any unsupported Symfony version, so set the minimum Symfony version to ^5.4
Also, supporting Symfony 7 would be nice, since it's about to be released in the next few days (it's in RC currently) and it's already supported in sentry/sentry

from sentry-symfony.

ste93cry avatar ste93cry commented on June 7, 2024 10

Thanks for the constructive feedback, it's always a pleasure to talk with someone so open minded.

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024 9

To be fully transparent, it's a bit of a hectic time at the moment, with my priorities shifting a lot. The PR is almost ready, with the remaining task of adding the newly added options from major version 5.0 of the PHP SDK needing to be completed. Thereafter, we have to update docs and get a new version of our flex recipe published.

from sentry-symfony.

gander avatar gander commented on June 7, 2024 7

Yes, March for sure! Iā€˜m actually planning to ship this next week.

bump after 2 weeks. whats up?

from sentry-symfony.

SchabrechtsK avatar SchabrechtsK commented on June 7, 2024 5

I saw the original time frame was mid-december 2023, I was wondering if there is any progress on this issue?

from sentry-symfony.

sts-rafalmazgaj avatar sts-rafalmazgaj commented on June 7, 2024 4

Can we expect Sentry 5.0 release still in March?

from sentry-symfony.

ste93cry avatar ste93cry commented on June 7, 2024 3

There are a few things I would like to talk about for the next major, personally. The first is a configuration change: I suggest disabling the tracing feature by default: I understand that Sentry wants its users to have everything configured out-of-box so they can get started right away, but the instrumentations are difficult to maintain and operate reliably without causing problems, and a quick search in this repository can prove it. In any case, nothing is sampled without manual user intervention in the configuration to set tracing_sample_rate to a value greater than 0, so in the end instrumentation already does nothing by default. Furthermore, the more integrations we add that are enabled by default, the more we impact the performance of projects without users knowing it: distributed tracing should be opt-in, not opt-out.

The second thing I would like to tackle is that our distributed tracing instrumentations are really hard to maintain because the requirements to allow them to be enabled and to be compatible with the installed dependencies cannot be specified in the composer.json. Basically, we have to resort to checking if some classes or interfaces exists to unlock the enabling of each instrumentation. Instead, a more appropriate approach would be to separate each instrumentation into its own Composer package, each with its own requirements: the package manager is good at solving dependencies and requirements, so we should let it do the job it was born for. The amount of conditionals, class aliases and polyfills tells everything of the current situation.

The third wish is a long-delayed refactoring of how we integrate with Symfony's error and exception handler (#337). We are currently using the core integrations, but they cause problems because the framework has its own handler and it doesn't really expect to be replaced. In the past, due to continued reporting of issues caused by the way we hooked into the error handling mechanism, the Flex recipe was changed to enable the bundle only in the prod environment. This in turn causes other problems, mainly the fact that since nothing is loaded in the rest of the environments, code using any of the services breaks unless the user decides to load the bundle in all environments. It's basically a dog chasing its tail. It's already possible to disable the affected integrations and manually configure Monolog to send its logs to Sentry: that's the right way to integrate with Symfony and we only have to make it the default, somehow.

The fourth thing, no less important, is the elimination of support for previous versions of Symfony. It is becoming increasingly difficult to support old versions, and the maintenance burden increases with each new version released, be it minor or major. We simply don't have the resources to support all of these versions, so letting go of at least Symfony 4.4 would be a great help. For example, to fix the HTTP client distributed tracing instrumentation for Symfony 4.4, I'm spending a huge amount of time to investigate what changed between 3 major versions of the framework, which is simple madness.

from sentry-symfony.

ste93cry avatar ste93cry commented on June 7, 2024 3

I don't know what I should look at, besides a documentation page that says that Sentry decided for me that instrumentation must be enabled by default, which again is what I'm saying shouldn't decide on my behalf. Unless you set the traces_sample_rate to a sensible value, those instrumentations are no-op, but are still hooking into the critical paths of the framework and of the dependencies. Exactly, which UX would be degraded more than now by documenting instead that along with the option mentioned before, you also have to set a flag to true for the integrations you are interested in?

from sentry-symfony.

vinceAmstoutz avatar vinceAmstoutz commented on June 7, 2024 2

Any idea when sentry 5 will be released?

from sentry-symfony.

gander avatar gander commented on June 7, 2024 2

Why does the project still require sentry/sdk 3.6 in the composer.json?

#783 (comment)

Because it's still a work in progress?

from sentry-symfony.

gjuric avatar gjuric commented on June 7, 2024 2

Great news, when can we expect a new release to be published?

from sentry-symfony.

Jean85 avatar Jean85 commented on June 7, 2024 1

Sorry I do not have the capacity right now, but I would suggest to ship a fast fix: the test console command is right now not exactly useful, because it does not have a detailed output of the issue if the event is not going out. The new SDK major has a new Logger option that could be leveraged to redirect output to the console while running the test, making it more useful.

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024 1

Most of this sounds reasonable, but this is sadly out of scope. I can only focus on the Symfony SDK for a short amount of time at the moment, hence these are topics more fitting for a v6.0.

However, if you want to work on this, we would need a rough timeline.
I would be fine using monolog by default in v5.0, but this would be a community effort.

Our main objective right now is to bump the PHP SDK and move on.

  • Disabling the tracing feature by default: Most stuff should no op when tracing is disabled. If there is a perf overhead, we should instead fix them. UX is important.

  • Distributed Tracing: Trace Propagation is no longer tied to performance/tracing and is enabled in all SDKs by default, besides JS due to CORS.

  • Each instrumentation into its own Composer package: Fine by me, but only as a mono repo.

  • Symfony 4: We can consider dropping this version, but I would need to check some numbers first.

from sentry-symfony.

tacman avatar tacman commented on June 7, 2024 1

Sure, Symfony 5 will continue to be supported, I question if it's worth the effort to update the Symfony 5 version to SDK 5. If people are on old versions of one library, they shouldn't expect the latest of another.

I don't know if Symfony 5 is holding back the integration of Sentry SDK 5, but if it is, I'd advocate for a new release that only supported PHP ^8.1 and Symfony ^6.4.

from sentry-symfony.

gander avatar gander commented on June 7, 2024 1

Maybe only 5.4?

from sentry-symfony.

gander avatar gander commented on June 7, 2024 1

@cleptric do you need help with any problems? I appreciate that you are actively working on this code.

from sentry-symfony.

Jean85 avatar Jean85 commented on June 7, 2024
  • Each instrumentation into its own Composer package: Fine by me, but only as a mono repo.

This is a no-go because you would lose the main benefit: the separate repo would allow multiple branches to target multiple majors of the related dependencies (i.e. DBAL v2 vs v3)

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024

There are some tools like https://github.com/symplify/monorepo-builder that make this possible. Not going to add more PHP repositories, sorry.

from sentry-symfony.

Jean85 avatar Jean85 commented on June 7, 2024

I'm not sure I understand how such tool would solve our issue. The requirement would be having cross compatibility between our Symfony SDK profiling and multiple versions of other supported libraries at the same time..

from sentry-symfony.

ste93cry avatar ste93cry commented on June 7, 2024

I would be fine using monolog by default in v5.0, but this would be a community effort.

There are some factors to consider to switch by default to Monolog, mainly around the fact that the logger configuration is not known in advance and so the risk of changing/breaking user settings is really high. There needs to be a thorough investigation, but if time is a key factor then I doubt we can make it in time for December. But it's definitely something we'll have to address sooner or later because it's a pain point for all users.

Disabling the tracing feature by default: Most stuff should no op when tracing is disabled. If there is a perf overhead, we should instead fix them. UX is important.

There's always a little overhead, no matter if most things are no-op when tracing is disabled. In most cases, when an instrumentation is disabled, the related services are removed from the container and therefore the performance impact is absolutely 0 simply because they don't even get instantiated. However, since by default everything is enabled, even if the event is eventually discarded, all the collection is still done. But what is more worrying is that the instrumentations hook into critical paths of their dependencies, and the risk of breaking the application is really high. We've had a few cases where things weren't working correctly, so it makes sense to disable instrumentation by default and let users choose, consciously, what they want to use. Right now, once the bundle is installed it hooks into everything it can and the user doesn't even know about it, finding out issues later on.

Each instrumentation into its own Composer package: Fine by me, but only as a mono repo.

Some good examples of what I meant are the transport packages for symfony/messenger: they are independent, they are developed in the monorepo and somehow the changes are replicated in the individual repositories. This ensures that we can support each package independently and that we can rely on Composer to manage dependency compatibility.

Symfony 4: We can consider dropping this version, but I would need to check some numbers first.

I would say that if the only reason for the existence of a v5 of this bundle is to allow the use of the latest SDK, then allowing Symfony 4.x or not doesn't really matter. However, it is not feasible to support all versions forever: at the moment, for every thing we do, be it a bugfix or a new feature, we have to spend a lot of time and energy to ensure that it works on 4 different versions of the framework. I understand that Sentry as a company wants to achieve the widest possible audience, but there are not enough resources to do so here, at least for now.

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024

Ok, then I'll move forward with a slim v5.0. Thanks!

However, since by default everything is enabled, even if the event is eventually discarded, all the collection is still done.

Then we need to add more checks if a transaction is on the scope and return early. We do this in Laravel, and it works fine. Again, to make this very clear, we're not degrading UX because there is a hypothetical problem. If we break users' code, we fix it. If we slow down things too much, we fix it.

from sentry-symfony.

ste93cry avatar ste93cry commented on June 7, 2024

Again, to make this very clear, we're not degrading UX because there is a hypothetical problem.

I don't think that we're degrading any UX: as I explained, the user has to explicitly enable the tracing already by configuring the sample_tracer or the traces_sample_rate options, hence any instrumentation he wants to use already requires a manual intervention to be "enabled". The only thing that would change is that along with setting a sample rate, the user is also in control of which instrumentation he wants to activate in an explicit manner. When and if we will be moving the instrumentations to their own packages, it will be even easier: since it's the user that is in control of installing the package, if he does then it's fine to enable the instrumentation by default. But right now, it makes no sense because it takes away people's free will, and history teaches it's never a good thing.

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024

https://docs.sentry.io/platforms/php/guides/symfony/performance/instrumentation/automatic-instrumentation/

But right now, it makes no sense because it takes away people's free will, and history teaches it's never a good thing.

lol

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024

We are not changing that.

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024

No progress as I had to shift focus onto other things.

from sentry-symfony.

tacman avatar tacman commented on June 7, 2024

Symfony 4: We can consider dropping this version, but I would need to check some numbers first.

It's hard to imagine any benefit to supporting Symfony 4. If you're releasing a new version, I think even dropping Symfony 5 would be okay. Anyone on the older versions can keep using the existing release.

from sentry-symfony.

Shadow-Devil avatar Shadow-Devil commented on June 7, 2024

Please keep Symfony 5 support since it is still maintained until November 2025 (see also: https://symfony.com/releases/5.4) and still used by some projects

from sentry-symfony.

garak avatar garak commented on June 7, 2024

Please keep Symfony 5 support since it is still maintained until November 2025 (see also: https://symfony.com/releases/5.4) and still used by some projects

as mentioned before, support for Symfony 5 will always be granted by older Sentry versions.
There's no need to support it in the last Sentry version.

from sentry-symfony.

newtoframework avatar newtoframework commented on June 7, 2024

Why does the project still require sentry/sdk 3.6 in the composer.json?

from sentry-symfony.

th-lange avatar th-lange commented on June 7, 2024

šŸ™

from sentry-symfony.

cleptric avatar cleptric commented on June 7, 2024

Version 5.0.0 has been released. Please consult the upgrade guide and changelog.

from sentry-symfony.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    šŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. šŸ“ŠšŸ“ˆšŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ā¤ļø Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.