search

perftools / php-profiler

A PHP profiling library based on XHGUI Data Collector
MIT License
147 stars 26 forks source link

Set ProfilingId from Profiler? #39

Closed glensc closed 4 years ago

glensc commented 4 years ago

In mongo saver there's getLastProfilingId method:

that has a side-effect, that if this saver is called in the application side, the id would be re-used for multiple profilings.

I don't know if that's deliberate or not, but using other savers this functionality is not present.

Question:

Having it profiler side could overwrite (accidentally or deliberately) existing profiling. I don't think saving the same id twice will merge the results?

Having it set at the profiler side, allows the profiler to control the id value.

and if reading the importer section:

it says importing the same profile twice creates duplicate profiling, so the duplicate id sent from profiler is not overwriting other profilers? is there any point then having value re-used? set from profiler?

cc @perftools/maintainers

glensc commented 4 years ago

Considering this issue:

I think there should be no way for the profiler (saver) to specify _id, i.e every submit should be accepted as new profiling submit.

glensc commented 4 years ago

Unit tests in xhgui assume the _id can be set by fixture, and therefore the tests fail because PDO driver doesn't allow it:

1) XHGui\Test\Controller\RunTest::testCallgraph
Exception: No profile data found.
glensc commented 4 years ago

Allow id-override in pdo saver:

similarly in mongo saver:

but the override is there just to allow testing, at least for pdo.

markstory commented 4 years ago

It would be simpler if the clients didn't generate ids. That would allow the server and each storage implementation to generate the appropriate ID as required. It also prevents duplicate IDs from being created.

glensc commented 4 years ago

thanks, @markstory I've come to the same conclusion, just haven't got around to implementing it so.

glensc commented 4 years ago

also, the import method should return the id in the response:

glensc commented 4 years ago

ignoring Id from saver if one is provided:

glensc commented 4 years ago

Providing id from profiler is no longer supported with latest xhgui (it's ignored).

glensc commented 3 years ago

The getLastProfilingId() was added in:

it was a mistake and is removed by now.

a better approach, if need to identify requests and profilers, is to use UNIQUE_ID in env: