GitHub Issue #1130: Metrics to track R & Python package usage - #7881
GitHub Issue #1130: Metrics to track R & Python package usage#7881cnathe wants to merge 7 commits into
Conversation
…ecordPackageUsage, and recordSuccessfulRun overrides
…ng via ScriptPackageUsageTracker.record()
…cording via ScriptPackageUsageTracker.record()
…unt of number of times a package was used
|
@labkey-jeckels @labkey-matthewb note that this approach doesn't track anything for Jupyter reports or those that run via Rserve. Thoughts? |
labkey-jeckels
left a comment
labkey-jeckels
left a comment
There was a problem hiding this comment.
I'm not worried about Jupyter since we only have reports on our own cloud servers, not customer instances.
https://www.labkey.org/_mothership/wiki-page.view?name=adhocReport&ids=32119&serverType=customers
See what you think about the suggestions to inject as a shutdown-type handler.
| { | ||
| String packageName; | ||
| while ((packageName = reader.readLine()) != null) | ||
| ScriptPackageUsageTracker.record(language, packageName); |
There was a problem hiding this comment.
Consider truncating long package names so we don't hit the DB limit of VARCHAR(255) for the total metric name.
There was a problem hiding this comment.
For extra protection, could also limit the list we track to 100 or 250 packages.
| if (extensions.isEmpty()) | ||
| throw new ScriptException("There are no file name extensions registered for this ScriptEngine : " + getFactory().getLanguageName()); | ||
|
|
||
| String epilog = getPackageCaptureEpilog(context); |
There was a problem hiding this comment.
I didn't dive deep to fully prove it to myself, but Claude thinks that RserveScriptEngine won't capture the package lists.
| * (via sys.stdlib_module_names), so no Python entry is needed. | ||
| */ | ||
| private static final Map<String, Set<String>> BASE_PACKAGES = Map.of( | ||
| "r", Set.of("base", "compiler", "datasets", "graphics", "grDevices", "methods", "stats", "utils") |
There was a problem hiding this comment.
Claude thinks that these are missing from the list: grid, tools, parallel, splines, stats4, and tcltk
|
|
||
| // Python appended to the end of a user script to capture the loaded modules (top-level names, excluding the standard | ||
| // library). Wrapped in try/except so a capture failure can never break the script run. | ||
| private static final String PACKAGE_CAPTURE_EPILOG = """ |
There was a problem hiding this comment.
This doesn't handle scripts that exit with success via a sys.ext. Consider using atexit.register() in the prologue instead, or possibly a sitecustomize.py.
https://docs.python.org/3/library/site.html
reg.finalizer(globalenv(), f, onexit = TRUE) might work for R.
2 participants
Rationale
https://github.com/LabKey/internal-issues/issues/1130
The change instruments ExternalScriptEngine to record which R packages / Python modules scripts load, so the counts get reported to mothership via SimpleMetricsService. It does this by: (1) appending a language-specific "capture epilog" to the end of the user script that writes loaded packages to a sidecar file; (2) reading that sidecar back after the run and incrementing a per-package counter; and (3) adding a PythonScriptEngine subclass plus manager wiring to detect .py engines. All metric work is wrapped so it can never break script execution.
Related Pull Requests
Changes