Write select participation scores to statsd#13073
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds hourly participation metric collection (lists created, star ratings, and reading log events/unique users) and publishes those counts to StatsD as part of the existing openlibrary/admin/stats.py cron workflow when ndays == 1.
Changes:
- Add a new
openlibrary/admin/vitals.pymodule to query hourly participation counts from Postgres and emit them as StatsD gauges. - Hook the new participation-score gathering + StatsD publishing into the existing admin stats cron script for the hourly run.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| openlibrary/admin/vitals.py | New participation metric queries and StatsD gauge emission logic |
| openlibrary/admin/stats.py | Invokes participation metric gathering/emission during the hourly (ndays == 1) cron execution |
|
prefix of |
| totals = oldb.query(total_books_logged_query) | ||
| results["reading_logs"] = {normalize_shelf_name(i.bookshelf_name): i.cnt for i in totals} | ||
|
|
||
| distinct_readers_logging_query = """ |
There was a problem hiding this comment.
Is there any way for some of these queries to live on the core/models themselves (e.g. classmethod)?
There was a problem hiding this comment.
I think so. I'll place the queries in the appropriate openlibrary.core modules.
There was a problem hiding this comment.
I moved the ratings query and the reading log query to Ratings and Bookshelves, respectively.
I'm not sure where to put the list count query. We get list data from Infogami, so there is no existing openlibrary.core module for list-related DB queries (happy to create one, if wanted).
We could add this function to numbers.py, though it may be a bit out-of-place there. If we do that, it may make sense to move the remaining functions from vitals.py to stats.py.
I've no strong opinions either way...
- Fix type hints - Cast `ndays` to `int` before comparing to another number - Add `stats.ol.` prefix to all statsd event names - Safely load the config file's contents - Address `RUF015` violations
e3f8361 to
af6d285
Compare
for more information, see https://pre-commit.ci
|
Looks much cleaner, thank you for leading this! |
Advances #12281
Gathers and writes the following statistics to
statsd:Reading log event counts are broken down by shelf name.
These stats will only be gathered/persisted during the hourly store counts cron jobs (where
ndays == 1).Data returned by the new function will look something like this:
Technical
Testing
Screenshot
Stakeholders