-
Notifications
You must be signed in to change notification settings - Fork 40
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
We are missing info #48
Comments
Thanks for spotting this David, Will try get to this ASAP. |
Does it in end up in the error log if you use MDL_PERFTOLOG? |
Copied from the Tracker, sorry I assumed incorrectly that it was a performance issue when it was not: IMO then we must stop relying on footer information, I'm afraid. Unless I'm wrong, output could be already close, some variables undefined and all sort of problems. So this change will require, for sure a lot of testing (different web servers, cgi...). Perhaps time to look for another communication channel different from page footer to gather all that information. All this said with a pinch of salt, be warned. I've not facts defending my fear, just the feeling it can be dangerous. Ciao :-) |
Yes @danpoltawski, it should as core_shutdown_manager executes all subscribed shutdown functions, and, after that, writes to the error_log and stops the profiler https://github.com/moodle/moodle/blob/master/lib/classes/shutdown_manager.php#L103 |
I would add my vote to Dan's proposal of parsing web server error logs, but I don't think that we should restrict it to apache, probably there are already open source tools that takes care of the different logs format used by the different web servers. |
Dirties patch ever to test event monitor dmonllao/moodle@89564fd catching all data, testing it now |
(pasting from #48) I'm currently testing an alternative to, instead of using MDL_PERFTOFOOT var + MDL_PERF_TEST (the one we created to catch data from redirects) remove all references to MDL_PERF_TEST and keep a simple echo $performanceinfo['txt'] at the end of the shutdown function based on MDL_PERF_TEST, the same place where we write to error_log. MDL_PERF_TEST is only set in this tool's tests (as a collateral damage we remove test testing frameworks references in codebase). Probably @danpoltawski can comment about it as he has experience working with apache logs, but this solution would work exactly like reading from apache logs but less trouble for what I can see. Using this alternative we will catch everything, including redirections. @danpoltawski, do you see any problem using this approach? |
Now a single MDL_PERF_TEST is cool enough to display all the data we need in a shutdown function, if we would keep the other vars too we would have duplicated results and even worst, we would be picking the first ones, the footer ones.
As JMeter don't know about pretty UI we can just parse the txt data sent to error logs.
I'm sad :( https://tracker.moodle.org/browse/MDL-47900?focusedCommentId=320129&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-320129
We process the MDL_PERF data displayed on the page footer to know about how things went, but that's not the end of the page scripts, standard log db write-per-page-request is not counted here, and the same happens now when trying to test the events monitor; these db writes are executed before closing the page request in a register_shutdown_function() We are missing all this data and we want it, I am thinking on another register_shutdown_function() (ensuring that is the last registered function to be added) that will output whatever it is that we can parse later from this tool if MDL_PERF_TEST is enabled, and move from parsing the footer html like we currently do to parse this.
The text was updated successfully, but these errors were encountered: