Skip to content
This repository has been archived by the owner on Sep 10, 2021. It is now read-only.

log slowest query times #210

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

log slowest query times #210

wants to merge 6 commits into from

Conversation

mgrauer
Copy link
Contributor

@mgrauer mgrauer commented Mar 10, 2016

@mgrauer
Copy link
Contributor Author

mgrauer commented Mar 11, 2016

I generalized this to all requests, will log the count of queries and time of queries if you include a query param profiler.

@mgrauer
Copy link
Contributor Author

mgrauer commented Mar 12, 2016

This is moving towards something we could consider putting into production.

It only enables the DB profiler if a profiler query param exists, and only logs the query times and query counts if a profiler query param exists, so it should have minimal impact on performance when not invoked.

There is an additional query param that reduces the logging output. So if ?profiler&profilerFiveSlowest is passed, it will only log the five slowest queries and times.

Zend_Registry::get('logger')->info('Profiler for '.$_SERVER['REQUEST_URI']);
$db = Zend_Db_Table::getDefaultAdapter();
$profiler = $db->getProfiler();
$profile = '';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer used

@mgrauer mgrauer changed the title DEBUG: do not merge; log community view query profiles log slowest query times Mar 19, 2016
@mgrauer
Copy link
Contributor Author

mgrauer commented Mar 19, 2016

@cpatrick @jamiesnape

Do you think this adds something worth getting in? If so I can clean up the history and get it ready to merge.

If not, I'll kill it and delete the branch so we don't have it hanging around.

IMO, it is doesn't add much overhead and can enable a useful debugging tool, so I favor putting it in. My feelings won't be hurt if you think otherwise.

@cpatrick
Copy link
Contributor

@mgrauer Thanks for the ping on this. I think as long as it doesn't seem like an avenue for Denial of Service or for discovery of system information (I don't think it does), then LGTM. Thanks!

@mgrauer
Copy link
Contributor Author

mgrauer commented Mar 19, 2016

I don't think it is an avenue for DOS either, it just might make DOS a slight bit easier, but you would have to already be trying to do a DOS.

I'll wait for @jamiesnape to weigh in since there are security implications possible.

@jamiesnape
Copy link
Contributor

Not sure about this, to be honest. Let me think about it and get back to you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants