Skip to content
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

Remove ipstats, use graphite instead #7622

Closed
Tracked by #4252
cdrini opened this issue Mar 9, 2023 · 5 comments · Fixed by #7745
Closed
Tracked by #4252

Remove ipstats, use graphite instead #7622

cdrini opened this issue Mar 9, 2023 · 5 comments · Fixed by #7745
Assignees
Labels
Affects: Operations Affects the IA DevOps folks Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] Priority: 1 Do this week, receiving emails, time sensitive, . [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]

Comments

@cdrini
Copy link
Collaborator

cdrini commented Mar 9, 2023

Related to #5144

[[1675987200000, 141032], [1676073600000, 136506], [1676160000000, 121618],...
  • Remove /admin/ipstats endpoint
@cdrini cdrini added Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed] Priority: 2 Important, as time permits. [managed] Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] Affects: Operations Affects the IA DevOps folks labels Mar 9, 2023
@cdrini cdrini added this to the Sprint 2023-03 milestone Mar 9, 2023
@cclauss
Copy link
Contributor

cclauss commented Mar 17, 2023

admin_range__visitors() in openlibrary/admin/numbers.py needs to be rewritten to read the data from Graphite as discussed above.

@cdrini
Copy link
Collaborator Author

cdrini commented Mar 17, 2023

admin_range__visitors() appears to be dead code and can be entirely deleted.

Only this function will need to be updated:

@cache.memoize(engine="memcache", key="admin._get_count_docs", expires=5 * 60)
def _get_count_docs(ndays):
"""Returns the count docs from admin stats database.
This function is memoized to avoid accessing the db for every request.
"""
today = datetime.datetime.utcnow().date()
dates = [today - datetime.timedelta(days=i) for i in range(ndays)]
# we want the dates in reverse order
dates = dates[::-1]
docs = [web.ctx.site.store.get(d.strftime("counts-%Y-%m-%d")) for d in dates]
return [d for d in docs if d]

To add a visitors field to each item from the graphite url listed above.

@cdrini cdrini added Priority: 1 Do this week, receiving emails, time sensitive, . [managed] and removed Priority: 2 Important, as time permits. [managed] labels Mar 27, 2023
@mekarpeles mekarpeles changed the title Remove ipstats, use statsd instead Remove ipstats, use graphite instead Mar 30, 2023
@cclauss
Copy link
Contributor

cclauss commented Apr 1, 2023

% docker logs openlibrary_cron-jobs_1 --follow

2023-04-01 20:16:26 [4096] [openlibrary.admin.stats] [WARNING]   admin_range__visitors - No statistics available
2023-04-01 20:16:26 [4096] [openlibrary.admin.stats] [INFO]   admin_range__works - 842
2023-04-01 20:16:26 [4096] [openlibrary.admin.stats] [INFO] Removing sqlite file used for ipstats
2023-04-01 20:17:24 [openlibrary.dump] split_dump 27,000,000

@mekarpeles
Copy link
Member

#3827 #3838 #7602 #7622

Broken on production (old code)
Working on staging, but off by a factor of 10? 100?

@cdrini
Copy link
Collaborator Author

cdrini commented Jun 5, 2023

Put that bug in its own issue here: #7936

@cdrini cdrini closed this as completed Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects: Operations Affects the IA DevOps folks Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] Priority: 1 Do this week, receiving emails, time sensitive, . [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@mekarpeles @cclauss @cdrini and others