Problem/Motivation
Since 1.0.0-alpha6 (#3630051: Fix the remaining module review findings: API calls on save, endless queue retries, viewer collisions and standalone submodules), Mapillary Logs records API calls in its own table, mapillary_logs_api_call, shown at Reports > Mapillary API calls (/admin/reports/mapillary-api-calls). Each entry has the time, endpoint, query, HTTP status, Graph API error code, duration and message, but not where the call was made from.
The same endpoint is called from several places: field widgets and the import forms in a page request, the queue workers of the sequence cascade and the thumbnail refresh on cron, and Drush commands such as mapillary:thumbnails-refresh. When a run of failed calls shows up in the report, the entry does not say whether a form, cron or a Drush command made it. The system log answers this with its location column, which dblog fills from the request URL.
Steps to reproduce
- Enable
mapillary_logsand check "Log API calls". - Import an image from the import form, then run
drush mapillary:thumbnails-refresh --now. - Open
/admin/reports/mapillary-api-calls: nothing tells the two calls apart except their time.
Proposed resolution
- Add a
locationcolumn tomapillary_logs_api_call:varchar(255), not null, default'', likeendpoint. - Let
MapillaryApiCallLog::record()fill it, the waydblogdoes: in a page request, the path of the current request fromrequest_stack, without the query string, so that tokens or codes in query parameters are not stored; on the command line, the script name and the command, for exampledrush mapillary:thumbnails-refresh, taken from the first argument that is not an option or a site alias. Drush is not a dependency of the module, so the command is read from$_SERVER['argv'], not from Drush classes. MapillaryApiCallEvent, the client methods andMapillaryLogsService::logApiCall()keep their signatures. Passing the location through the event would be more precise, but it would reach every typed method of the client and every caller.- Queue workers have no request of their own: items processed by
drush cronordrush queue:runget the command, items processed by a cron URL get its path. The cron key is part of that path;dblogstores it too, and both reports need the "View site reports" permission. Automated Cron runs after the response of an ordinary page, so its items get the path of that page. This is enough to tell cron from a form or a command. request_stackbecomes an optional last constructor argument ofMapillaryApiCallLog, asMapillaryLogsServicedoes for the API call log: a container built before the update still passes three arguments, and the location then stays empty.- Before
drush updatedbthe column does not exist. A failed insert would abort an open PostgreSQL transaction, sorecord()checks the column once per request, as it does for the table, and leaves it out of the insert while it is missing. - Views data for the column: field, string filter, sort and argument. Add a "Location" column after "Endpoint" to
views.view.mapillary_api_callsinconfig/optional. - Upgrade path:
mapillary_logs_update_11102()adds the column; existing entries keep an empty location. A post update adds the field to an existingviews.view.mapillary_api_callswhen the view exists and has no such field yet, so a view the site changed keeps its changes. - Tests: a kernel test for the location of a call in a page request and on the command line, the insert without the column before the update, and the update path from the 1.0.0-alpha6 schema (table without the column, view without the field), in
ApiCallReportTest. - An "Added" entry under
[Unreleased]inCHANGELOG.md, with a note to rundrush updatedb.
Remaining tasks
- Merge request.
- Review.
- Update rehearsal from 1.0.0-alpha6.
User interface changes
- A "Location" column in the report of Mapillary API calls.
API changes
MapillaryApiCallLog::__construct()gets an optional fourth argument,?RequestStack $requestStack = NULL.- Views data: a
locationfield of themapillary_logs_api_calltable.
Data model changes
- New column
mapillary_logs_api_call.location, added bymapillary_logs_update_11102(). - The view
views.view.mapillary_api_callsgets alocationfield, added by a post update on existing sites.
Issue fork mapillary-3631523
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #5
orkutmuratyilmaz