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

Xhprof is abonond, document why is still a prod requirement #20317

Open
mikkeschiren opened this issue Feb 3, 2023 · 9 comments
Open

Xhprof is abonond, document why is still a prod requirement #20317

mikkeschiren opened this issue Feb 3, 2023 · 9 comments
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.

Comments

@mikkeschiren
Copy link
Contributor

lox/xhprof is an abandoned project, and have no updates since 2015.
There should be documentation why this still is needed as a prod requirement.

@mikkeschiren mikkeschiren added the To Triage An issue awaiting triage by a Matomo core team member label Feb 3, 2023
@sgiehl
Copy link
Member

sgiehl commented Feb 3, 2023

@tsteur would you mind commenting this one? I'm actually not sure if the requirement ist still needed. Unless someone wants to debug a productive system I don't think it's needed or am I wrong?

@tsteur
Copy link
Member

tsteur commented Feb 5, 2023

As long as it is possible to still collect data on production, then we could remove it and let people know they will need to view them locally. Looking at #17939 it may be required to profile locally?

@sgiehl
Copy link
Member

sgiehl commented Feb 6, 2023

@tsteur We could remove that from core maybe and provide that dependency with a plugin instead. That way everyone who wants to debug could install the plugin.

@tsteur
Copy link
Member

tsteur commented Feb 6, 2023

That could work. Or maybe there's an alternative package we can use that's being updated? Just thinking that might be simpler for us and for users. Also then we won't need to worry re adding support for all the different profiling like --xhprof via a plugin or so. No big preference though, just a thought.

@sgiehl
Copy link
Member

sgiehl commented Feb 7, 2023

Couldn't find any other package that is still maintained. Moving it to a plugin sound like the most viable solution for me. I'll put it to prio queue, so we can consider doing that at some point.

@sgiehl sgiehl added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. and removed To Triage An issue awaiting triage by a Matomo core team member labels Feb 7, 2023
@sgiehl sgiehl added this to the For Prioritization milestone Feb 7, 2023
@mikkeschiren
Copy link
Contributor Author

As, I understand it, xhprof, doesn't work in PHP8, it should be removed from core, for those want to run it, and are still on PHP 7, it should be a plugin.

@tsteur
Copy link
Member

tsteur commented Feb 9, 2023

FYI xhprof and tideways profiler do work with PHP 8, we used it just two weeks ago

@textagroup
Copy link
Contributor

Found the following fork which is being kept up to date the fork does not have a composer.json file and is not listed on packagist but I have raised a PR to add a composer.json file to the fork.

Have also created the following branch of matomo which replaces the current version of xhprof.
However I have not tested this I will start by seeing if the PHPUnit test pass first of all.

@michalkleiner
Copy link
Contributor

We are also on track to providing ddev configuration for Matomo and ddev already comes with xhprof support which uses the fork Kirk found, so I would probably wait a bit until that's finalised, allowing us to perhaps save some time and not double up on the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

No branches or pull requests

5 participants