ENG-5751 remove old keen logic #10992

mkovalua · 2025-02-21T13:19:16Z

Purpose

We don’t use Keen anymore, but we still have a lot of Keen code and config in osf.io. There’s also a legacy column in the node table, keenio_read_key. All that can go. I’m 95% sure we don’t use it anymore, as all metrics logging should be happening through our own metrics endpoints. We probably need to keep the keen js packages, as we still use those to display metrics data pulled from our source.

Changes

Removing old keen code

QA Notes

Please make verification statements inspired by your code and what your code touches.

Verify
Verify

What are the areas of risk?

Any concerns/considerations/questions that development raised?

Documentation

Side Effects

Ticket

https://openscience.atlassian.net/browse/ENG-5751

… needed to be removed in the files)

felliott

This is looking good so far. I've left a couple of notes for things to check on. Please also remove the unneeded keen dependencies for both the python (pyproject.toml) and javascript (package.json) dependency lists.

Thank you!
@felliott

felliott · 2025-02-25T07:11:13Z

osf/management/commands/cumulative_plos_metrics.py

We can remove this file entirely per Eric's comments in Slack

felliott · 2025-02-25T07:12:25Z

website/settings/defaults.py

You can remove references to osf.management.commands.cumulative_plos_metrics

api/nodes/serializers.py

mkovalua · 2025-02-28T15:05:46Z

Hi @felliott , I left some thoughts about possible removal below

osf_metrics.html

Maybe it is needed to remove everything and not render the tab ‘Metrics’ in admin, some thoughts about:

preprints-tab (html) -> renderPreprintMetrics preprint_summary (js) ->
(py)

'preprint_summary' : {
    'mapper': _map_preprint_summary,
    'class': PreprintSummaryReport,
},

(not confident whether it is needed to be deleted (how the data are mapped into DailyReport/PreprintSummaryReport do somehow keen is used (not see it)) though it has 'report_date': _timestamp_to_date(row['keen.timestamp']) in _map_preprint_summary )

addons-tab (html) -> AddonMetrics storage_addon_usage (js) ->
(py) 'storage_addon_usage': reports.StorageAddonUsage,

(not confident whether it is needed to be deleted )

raw-numbers-tab (html) -> renderRawNodeMetrics node_summary (js) -> (py)

'node_summary': {
    'mapper': _map_node_summary,
    'class': NodeSummaryReport,
},

(not confident whether it is needed to be deleted though it has keen.timestamp in _map_node_summary)

institution-tab (html) -> InstitutionMetrics (js) institution_summary ->

 'institution_summary': {
    'mapper': _map_institution_summary,
    'class': InstitutionSummaryReport,
},

(not confident whether it is needed to be deleted though it has keen.timestamp in _map_institution_summary)

user-tab (html) -> no usage in js (looks like it may be removed because it does not render any data from Back end)

downloads-tab (html) -> DownloadMetrics (js) file_stats -> file_stats (WaterButler) (looks like it may be removed because of legacy file_stats)

felliott

Hey @mkovalua!

Looking good so far! I've poked the slack channel about removing/versioning analytics_key. I'll update you when I hear back. In addition to the noted changes, can you also please remove keen-analytics from osf's package.json and yarn.lock files? Thank you!

Cheers,
@felliott

felliott · 2025-03-04T12:26:15Z

admin/static/js/metrics/metrics.es6.js

+    // renderKeenMetric("#number-of-unique-downloads", "metric", uniqueDownloadsQuery,
+    //                  defaultHeight, defaultColor, publicClient);
+
+    // renderDownloadMetrics();


This chart is pulled from internal metrics, so it can be kept in.

felliott · 2025-03-04T12:32:23Z

admin/static/js/metrics/metrics.es6.js

In this file you should also be able to remove client, publicClient, renderKeenClient, and anything involving keen-analysis. keen-dataviz is still used to render charts from our internal metrics, so that should be left alone. Removing keen-analysis from here should allow you to remove it from admin's package.json / yarn.lock too.

felliott · 2025-03-04T12:34:59Z

admin/static/js/metrics/metrics.es6.js

Oh, and I guess axing keen-analysis means NodeLogsByUser can go as well.

felliott · 2025-03-04T12:36:03Z

admin/static/js/metrics/metrics.es6.js

I think you should also be good to delete the commented-out code. If we decide to reimplement these later, we can always dig into the git history.

felliott · 2025-03-04T12:36:58Z

admin/static/js/pages/metrics-page.js

-    };
+    // $('#downloads-tab')[0].onclick = function() {
+    //     Metrics.DownloadMetrics();
+    // };


Please don't comment this out. The first graph ("Daily Download Counts") is still good.

felliott · 2025-03-04T12:54:39Z

osf/management/commands/cumulative_plos_metrics.py

You can delete this entire file.

felliott · 2025-03-04T12:55:38Z

website/settings/defaults.py

@@ -444,7 +444,7 @@ class CeleryConfig:
        'osf.management.commands.sync_datacite_doi_metadata',
        'osf.management.commands.update_institution_project_counts',
        'osf.management.commands.populate_branched_from',
-        'osf.management.commands.cumulative_plos_metrics',
+        # 'osf.management.commands.cumulative_plos_metrics',


You can delete this.

felliott · 2025-03-04T12:55:46Z

website/settings/defaults.py

@@ -549,7 +549,7 @@ class CeleryConfig:
        'osf.management.commands.delete_legacy_quickfiles_nodes',
        'osf.management.commands.fix_quickfiles_waterbutler_logs',
        'osf.management.commands.sync_doi_metadata',
-        'osf.management.commands.cumulative_plos_metrics',
+        # 'osf.management.commands.cumulative_plos_metrics',


You can delete this

felliott · 2025-03-04T13:03:48Z

osf/migrations/0028_remove_abstractnode_keenio_read_key.py

Just a heads-up: we might need to regenerate this migration if new migrations have gone out since you started work on this.

felliott · 2025-03-04T13:10:12Z

scripts/tests/test_populate_popular_projects_and_registrations.py

If this is only testing popular projects & regs and those come from keen, theny we should be able to delete the entire file.

felliott · 2025-03-04T15:32:20Z

api/nodes/serializers.py

@@ -274,7 +273,6 @@ class NodeSerializer(TaxonomizableSerializerMixin, JSONAPISerializer):
    tags = ValuesListField(attr_name='name', child=ser.CharField(), required=False)
    access_requests_enabled = ShowIfVersion(ser.BooleanField(read_only=False, required=False), min_version='2.0', max_version='2.8')
    node_license = NodeLicenseSerializer(required=False, source='license')
-    analytics_key = ShowIfAdminScopeOrAnonymous(ser.CharField(read_only=True, source='keenio_read_key'))


Per @brianjgeiger's comment, we can set this to '' rather than delete it. If we do that, we don't need to worry about versioning. Maybe add a short comment saying that this can be removed after another version release.

…ely on it and it is not needed to be deleted

felliott

Hey @mkovalua!

Nice work! This is getting closer. I think the last bit is to remove the keen javascript tracking from the legacy pages (those under the website/ directory). To do that:

Your original change to base.mako was correct; we do want to remove the keen ids and keys from base.mako.
website/static/js/keen.js is mostly obsolete. We still use the _logPageview function to log to our internal analytics system. But all the logging to keen can be removed. You can also remove the keen-tracker js library. The keen.helpers.getUniqueId function we use is described here: https://github.com/keen/keen-tracking.js/blob/master/lib/helpers/getUniqueId.js. Feel free to inline that code here if there isn't a better, more standard js way of doing it. A comment in their git history says they got the code from: https://stackoverflow.com/questions/105034/how-do-i-create-a-guid-uuid/2117523#2117523
Please add some code to start migrating the old keenSessionId cookie over to a new name, perhaps osfMetricsSessionId. It looks like that cookie only lives for about 25 minutes, so we can prepare a hotfix to delete any mention of keenSessionId after release.
I guess since we're purging almost all mentions of keen, it would make sense to rename keen.js to metrics.js.
The metrics logging is initiated in website/static/js/pages/base-page.js. If we rename keen.js, that will need to be updated in this file.

Thanks for sticking with this! It's a long process, but we're getting close to the end.

Cheers,
@felliott

…mplementation https://stackoverflow.com/questions/18230217/javascript-generate-a-random-number-within-a-range-using-crypto-getrandomvalues )

felliott

Almost there! Last few nitpicks: there are some files that still mention keen, but don't use it anymore. Can you remove that or rename methods. In most cases you should be able to replace keen with metrics if you are renaming something. Examples include:

admin/metrics/views.py - remove references to KEEN_CREDENTIALS
admin/templates/metrics/osf_metrics.html - remove the keen configuration. You can leave keen-dashboard.css named as it is; we're still using the keen-dataset code, so that's okay for now.
admin/base/settings/defaults.py - remove the keen config vars
website/settings/defaults.py - remove the keen config vars
website/settings/local-ci.py - ditto
website/static/js/metrics.js - rename _createOrUpdateKeenSession()
website/static/js/pages/base-page.js - update mention of KeenIO in comment to "metrics"
website/routes.py - remove the keen config and resolve the keen TODO

Thank you!

…n-removal

felliott

Looks good! I made a change to remove the old geoip2 code and merged in the existing feature/pbs-25-05 branch to resolve some poetry.lock conflicts. I think this is ready to merge!

felliott · 2025-03-31T14:03:37Z

@brianjgeiger requesting your review since the target is your branch

brianjgeiger

One question that I suspect I know the answer to. The question might be for @felliott.

brianjgeiger · 2025-03-31T17:13:09Z

scripts/tests/test_populate_popular_projects_and_registrations.py

@@ -1,95 +0,0 @@
-from unittest import mock


I'm not seeing the removal of the script that this tests in this PR. Has it been broken for a while because it relies on Keen, or should this remain in?

@brianjgeiger

the removed test rely on get_keen_activity and it is removed because all the logic inside relied on Keen client response

https://github.com/CenterForOpenScience/osf.io/pull/10992/files/18e6fbd0c5a3dc7180f6e34dc594b7b13c0183ca#diff-ae86e79b3a482fcdd2a3ca8cf0f46852e8d5f716886df902ab2184f240bf15b2L31

if you mean about def activity() the code inside is partially removed and remained because not all logic rely on Keen, the Keen related logic is removed or do you mean something another ? Thanks

FYI:
@felliott

@brianjgeiger, this one is a little weird. The test script does technically test non-keen stuff set by the scripts/populate_popular_projects_and_registrations.py script, but it's very limited. I think that script might be out-of-date. It references POPULAR_LINKS_REGISTRATIONS which is only mentioned in the settings files. It seems to have been superseded by scripts/populate_new_and_noteworthy_projects.py. If that's the case, then we can remove scripts/populate_popular..., remove all references to it, and scrub POPULAR_LINKS_REGISTRATIONS from the default and local-ci settings files. We can also probably delete website.utils.activity and the other methods in that file that it calls.

@mfraezz, as another old-timer do you remember a reason why we kept these scripts?

without

@mock.patch('website.project.utils.get_keen_activity')

(new mock_client is needed to be imitated but it relied on KEEN website.project.utils.get_keen_activity previously )

locally

python3 -m pytest tests/test_populate_popular_projects_and_registrations.py

the test does not passed for now

FYI: @felliott @brianjgeiger

Oh, good spot! Yeah, we need to remove both the @mock.patch and the mock_client from the signature since we're not patching it anymore.

Those scripts were what populated these widgets on the dashboard. I believe we disabled them at one point, due to spam-related reasons. Whether or not we want to retain those widgets and related functionality, especially after the angular migration, is likely a Product question.

The _registrations version of the script fell out of use when that was ported to ember. I believe it was intended to be a follow-up improvement, as the widget for it was retained, but the registrations it contains are specified manually in the configs.

I'm checking with Product to see if we want to keep this functionality with the Angular project, which may inform what we do here.

brianjgeiger · 2025-03-31T17:14:35Z

website/project/utils.py

 def activity():
    """Generate analytics for most popular public projects and registrations.
    Called by `scripts/update_populate_projects_and_registrations`
    """
    Node = apps.get_model('osf.AbstractNode')
    popular_public_projects = []
    popular_public_registrations = []
-    max_projects_to_display = settings.MAX_POPULAR_PROJECTS


I'm guessing the answer to my question above is "It's been broken for a while".

CenterForOpenScience#10992 (comment)

* ENG-5751 remove old keen logic * add migration * remove legacy keen from metrics .js (maybe another keen logic is also needed to be removed in the files) * remove dashboard keen (it is not used in any views) * ENG-5751 code updates * ENG-5751 remove keen dependency from poetry * ENG-5751 code updates * code updates * ENG-5751 resolve PR comments * ENG-5751 add default '' analytics_key * ENG-5751 remove base_analytics.html * add keen-dashboards.css * fix/ENG-5751 it seems 'keen' is used in base mako files and keen js rely on it and it is not needed to be deleted * fix/ENG-5751 resolve PR comments (not confident about 'getUniqueId' implementation https://stackoverflow.com/questions/18230217/javascript-generate-a-random-number-within-a-range-using-crypto-getrandomvalues ) * remove keen-tracking * update getUniqueId * fix/ENG-5751 PR comments related updates * fix/ENG-5751 PR comments related updates * remove geoip from deps and location code from app * PR comment related updates #10992 (comment) * fix migrations conflicts * a few tidy-ups: * Relocate code changes in api.nodes.serializers to be more compact when squished. * Really delete `cumulative_plos_metrics.py`. Per Eric's comments on Slack, we no longer need this. * Remove `cumulative_plos_metrics.py`-related config from website.settings.defaults.py * Add attributive comments for borrowed code in metrics library. --------- Co-authored-by: Fitz Elliott <[email protected]>

mkovalua added 4 commits February 21, 2025 15:15

ENG-5751 remove old keen logic

ee7c18a

add migration

873a011

remove legacy keen from metrics .js (maybe another keen logic is also…

f20d0e7

… needed to be removed in the files)

remove dashboard keen (it is not used in any views)

b6cd855

felliott requested changes Feb 25, 2025

View reviewed changes

mkovalua added 4 commits February 28, 2025 12:09

ENG-5751 code updates

5c2255e

ENG-5751 remove keen dependency from poetry

f081bd2

ENG-5751 code updates

4df563c

code updates

4cf4dc7

mkovalua requested a review from felliott February 28, 2025 15:19

felliott requested changes Mar 4, 2025

View reviewed changes

felliott reviewed Mar 4, 2025

View reviewed changes

mkovalua added 4 commits March 4, 2025 19:10

ENG-5751 resolve PR comments

3c86081

ENG-5751 add default '' analytics_key

096f519

ENG-5751 remove base_analytics.html

1e2eb30

add keen-dashboards.css

5456bca

mkovalua requested a review from felliott March 5, 2025 13:30

mkovalua marked this pull request as ready for review March 6, 2025 20:38

mkovalua added 2 commits March 6, 2025 22:52

Merge branch 'feature/b-and-i-25-01' into fix/ENG-5751

801c95f

fix/ENG-5751 it seems 'keen' is used in base mako files and keen js r…

0fe5548

…ely on it and it is not needed to be deleted

mkovalua changed the base branch from feature/b-and-i-25-01 to feature/pbs-25-05 March 11, 2025 20:23

felliott requested changes Mar 13, 2025

View reviewed changes

mkovalua added 3 commits March 14, 2025 00:23

fix/ENG-5751 resolve PR comments (not confident about 'getUniqueId' i…

51f23d7

…mplementation https://stackoverflow.com/questions/18230217/javascript-generate-a-random-number-within-a-range-using-crypto-getrandomvalues )

remove keen-tracking

530c57e

update getUniqueId

822b829

mkovalua requested a review from felliott March 14, 2025 13:07

felliott requested changes Mar 24, 2025

View reviewed changes

fix/ENG-5751 PR comments related updates

de3b4aa

mkovalua requested a review from felliott March 24, 2025 19:53

mkovalua and others added 3 commits March 24, 2025 23:11

fix/ENG-5751 PR comments related updates

af03db3

Merge remote-tracking branch 'cos/feature/pbs-25-05' into feature/kee…

35ad648

…n-removal

remove geoip from deps and location code from app

18e6fbd

felliott force-pushed the fix/ENG-5751 branch from e450503 to 18e6fbd Compare March 31, 2025 09:10

felliott approved these changes Mar 31, 2025

View reviewed changes

felliott requested a review from brianjgeiger March 31, 2025 14:03

brianjgeiger requested changes Mar 31, 2025

View reviewed changes

mkovalua requested review from felliott and brianjgeiger March 31, 2025 21:09

brianjgeiger deleted the branch CenterForOpenScience:feature/pbs-25-05 April 7, 2025 21:20

brianjgeiger closed this Apr 7, 2025

mkovalua mentioned this pull request Apr 8, 2025

[ENG-5751] remove old keen logic #11082

Merged

mkovalua added a commit to mkovalua/osf.io that referenced this pull request Apr 8, 2025

PR comment related updates

baa253f

CenterForOpenScience#10992 (comment)

ENG-5751 remove old keen logic #10992

ENG-5751 remove old keen logic #10992

Uh oh!

Conversation

mkovalua commented Feb 21, 2025

Purpose

Changes

QA Notes

Documentation

Side Effects

Ticket

Uh oh!

felliott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mkovalua commented Feb 28, 2025

Uh oh!

felliott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felliott left a comment

Choose a reason for hiding this comment

Uh oh!

felliott left a comment

Choose a reason for hiding this comment

Uh oh!

felliott left a comment

Choose a reason for hiding this comment

Uh oh!

felliott commented Mar 31, 2025

Uh oh!

brianjgeiger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkovalua Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felliott Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfraezz Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

mkovalua Apr 1, 2025 •

edited

Loading

felliott Apr 2, 2025 •

edited

Loading

mfraezz Apr 3, 2025 •

edited

Loading