From 4de8ef9772ad67e7e5bc6518a128a9b98a680269 Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 24 Jul 2026 09:59:28 -0500 Subject: [PATCH 01/13] ExternalScriptEngine refactor to allow for getPackageCaptureEpilog, recordPackageUsage, and recordSuccessfulRun overrides --- .../api/reports/ExternalScriptEngine.java | 89 +++++++++++++++++-- 1 file changed, 83 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/reports/ExternalScriptEngine.java b/api/src/org/labkey/api/reports/ExternalScriptEngine.java index 64a79d1a439..9646012c032 100644 --- a/api/src/org/labkey/api/reports/ExternalScriptEngine.java +++ b/api/src/org/labkey/api/reports/ExternalScriptEngine.java @@ -23,6 +23,7 @@ import org.labkey.api.miniprofiler.MiniProfiler; import org.labkey.api.pipeline.PipelineJobService; import org.labkey.api.reader.Readers; +import org.labkey.api.reports.report.ScriptPackageUsageTracker; import org.labkey.api.reports.report.r.ParamReplacementSvc; import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.LabKeyProcessBuilder; @@ -113,15 +114,91 @@ public boolean isBinary(FileLike file) public Object eval(String script, ScriptContext context) throws ScriptException { List extensions = getFactory().getExtensions(); + if (extensions.isEmpty()) + throw new ScriptException("There are no file name extensions registered for this ScriptEngine : " + getFactory().getLanguageName()); + + String epilog = getPackageCaptureEpilog(context); + if (epilog != null) + script = script + "\n" + epilog; - if (!extensions.isEmpty()) + FileLike scriptFile = prepareScriptFile(script, context, extensions); + try { - // write out the script file to disk using the first extension as the default - FileLike scriptFile = writeScriptFile(script, context, extensions); - return eval(scriptFile, context); + Object result = eval(scriptFile, context); + recordSuccessfulRun(context); + return result; + } + finally + { + recordPackageUsage(context); + } + } + + /** + * Prepare the on-disk script file that will be executed. The default writes the script as-is; subclasses (e.g. the + * R engine's knitr handling) may wrap it in a different driver script. + */ + protected FileLike prepareScriptFile(String script, ScriptContext context, List extensions) + { + return writeScriptFile(script, context, extensions); + } + + /** + * GitHub Issue #1130 + * Script appended to the end of the user script (in the same process) that captures the loaded packages/modules and + * writes them, one per line, to a sidecar file in the working directory for {@link #recordPackageUsage} to read + * back. The default returns null (no capture); language-specific engines (e.g. R, Python) override this. Wrapped so + * a capture failure can never break the script run. + */ + protected @Nullable String getPackageCaptureEpilog(ScriptContext context) + { + return null; + } + + /** + * GitHub Issue #1130 + * After a script runs, read back and record the packages loaded by the script. The default does nothing; + * language-specific engines override this, typically delegating to {@link #readPackageSidecar}. Never throws: + * package tracking must not affect script execution. + */ + protected void recordPackageUsage(ScriptContext context) + { + } + + /** + * GitHub Issue #1130 + * Called after a script has run successfully (eval returned without throwing). The default does nothing; + * language-specific engines may override to record success metrics. + */ + protected void recordSuccessfulRun(ScriptContext context) + { + } + + /** + * GitHub Issue #1130 + * Read a sidecar file of package names (one per line) from the working directory and record each under the given + * language in {@link ScriptPackageUsageTracker}. Never throws. A missing file means the script errored before the + * capture epilog ran (or capture was skipped) - nothing to do. + */ + protected void readPackageSidecar(ScriptContext context, String fileName, String language) + { + try + { + FileLike packagesFile = getWorkingDir(context).resolveChild(fileName); + if (!packagesFile.exists()) + return; + + try (BufferedReader reader = Readers.getReader(packagesFile.openInputStream())) + { + String packageName; + while ((packageName = reader.readLine()) != null) + ScriptPackageUsageTracker.record(language, packageName); + } + } + catch (Exception e) + { + LOG.warn("Failed to record " + language + " package usage", e); } - else - throw new ScriptException("There are no file name extensions registered for this ScriptEngine : " + getFactory().getLanguageName()); } protected Object eval(FileLike scriptFile, ScriptContext context) throws ScriptException From d98a36ba5c82e7f1dac44c797a8c1c923d40cdc7 Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 24 Jul 2026 10:00:27 -0500 Subject: [PATCH 02/13] RScriptEngine implementations for tracking package usages and recording via ScriptPackageUsageTracker.record() --- .../api/reports/report/r/RScriptEngine.java | 53 +++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/api/src/org/labkey/api/reports/report/r/RScriptEngine.java b/api/src/org/labkey/api/reports/report/r/RScriptEngine.java index 91846ddfa94..26574247605 100644 --- a/api/src/org/labkey/api/reports/report/r/RScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/r/RScriptEngine.java @@ -16,15 +16,16 @@ package org.labkey.api.reports.report.r; import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.JdbcType; import org.labkey.api.reports.ExternalScriptEngine; import org.labkey.api.reports.ExternalScriptEngineDefinition; +import org.labkey.api.reports.report.ScriptPackageUsageTracker; import org.labkey.vfs.FileLike; import javax.script.Bindings; import javax.script.ScriptContext; import javax.script.ScriptEngineFactory; -import javax.script.ScriptException; import java.util.Arrays; import java.util.List; @@ -35,6 +36,7 @@ */ public class RScriptEngine extends ExternalScriptEngine { + private static final String PACKAGES_FILE = "labkeyRPackages.txt"; public static final String KNITR_FORMAT = "r.script.engine.knitrFormat"; public static final String KNITR_OUTPUT = "r.script.engine.knitrOutput"; public static final String PANDOC_USE_DEFAULT_OUTPUT_FORMAT = "r.script.engine.pandocUseDefaultOutputFormat"; @@ -59,6 +61,7 @@ public ScriptEngineFactory getFactory() return new RScriptEngineFactory(_def); } + @Override protected FileLike prepareScriptFile(String script, ScriptContext context, List extensions) { FileLike scriptFile; @@ -86,18 +89,48 @@ protected FileLike prepareScriptFile(String script, ScriptContext context, List< return scriptFile; } + /** + * R appended to the end of the user script (in the same R session) that captures the set of loaded packages, + * writing them (one per line) to a sidecar file in the working directory for {@link #recordPackageUsage} to read + * back. Wrapped in tryCatch so a capture failure can never break the report or transform run. + */ @Override - public Object eval(String script, ScriptContext context) throws ScriptException + protected @Nullable String getPackageCaptureEpilog(ScriptContext context) { - List extensions = getFactory().getExtensions(); + // For knitr the executed file is a generated wrapper (createKnitrScript), not the user's R, so appending R here + // wouldn't run. We instead record the wrapper's known libraries in recordSuccessfulRun(). + if (getKnitrFormat(context) != RReportDescriptor.KnitrFormat.None) + return null; - if (!extensions.isEmpty()) - { - FileLike scriptFile = prepareScriptFile(script, context, extensions); - return eval(scriptFile, context); - } - else - throw new ScriptException("There are no file name extensions registered for this ScriptEngine : " + getFactory().getLanguageName()); + return """ + # --- LabKey R package usage capture --- + tryCatch({ + writeLines(sort(loadedNamespaces()), "%s") + }, error = function(e) invisible(NULL)) + """.formatted(PACKAGES_FILE); + } + + @Override + protected void recordPackageUsage(ScriptContext context) + { + readPackageSidecar(context, PACKAGES_FILE, "r"); + } + + @Override + protected void recordSuccessfulRun(ScriptContext context) + { + // Non-knitr R runs report their loaded packages via the capture epilog (see getPackageCaptureEpilog). + if (getKnitrFormat(context) == RReportDescriptor.KnitrFormat.None) + return; + + // For knitr, the generated wrapper (createKnitrScript) always loads knitr and, for the markdown+pandoc path, + // rmarkdown; record those as R package usage so they show up alongside other packages. + // NOTE: this only captures the wrapper's libraries, not packages loaded inside the report's own R chunks (e.g. + // ggplot2 used within an .rmd). Fully capturing those would require injecting a loadedNamespaces() write into + // createKnitrScript after the knit()/render() call. + ScriptPackageUsageTracker.record("r", "knitr"); + if (getKnitrFormat(context) == RReportDescriptor.KnitrFormat.Markdown && isPandocEnabled()) + ScriptPackageUsageTracker.record("r", "rmarkdown"); } private boolean isPandocEnabled() From 76cbf5e0675e00586f4a3f23e1b576f7e0f47a0e Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 24 Jul 2026 10:00:46 -0500 Subject: [PATCH 03/13] PythonScriptEngine implementations for tracking package usages and recording via ScriptPackageUsageTracker.record() --- .../report/python/PythonScriptEngine.java | 64 +++++++++++++++++++ .../core/reports/ScriptEngineManagerImpl.java | 15 +++++ 2 files changed, 79 insertions(+) create mode 100644 api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java diff --git a/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java b/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java new file mode 100644 index 00000000000..8d8844d8bb6 --- /dev/null +++ b/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.reports.report.python; + +import org.jetbrains.annotations.Nullable; +import org.labkey.api.reports.ExternalScriptEngine; +import org.labkey.api.reports.ExternalScriptEngineDefinition; + +import javax.script.ScriptContext; + +/** + * Script engine for locally-executed Python scripts (Python assay transform scripts configured as an + * external ".py" engine). Behaves like the base {@link ExternalScriptEngine} except that it appends a capture epilog to + * track which Python modules each script loads; see + * {@link org.labkey.api.reports.report.ScriptPackageUsageTracker}. + */ +public class PythonScriptEngine extends ExternalScriptEngine +{ + private static final String PACKAGES_FILE = "labkeyPythonPackages.txt"; + + // 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 = """ + # --- LabKey Python package usage capture --- + try: + import sys as _lk_sys + _lk_stdlib = set(getattr(_lk_sys, 'stdlib_module_names', ())) + _lk_mods = sorted({m.split('.')[0] for m in list(_lk_sys.modules)} - _lk_stdlib) + with open('%s', 'w') as _lk_f: + _lk_f.write('\\n'.join(m for m in _lk_mods if m and not m.startswith('_'))) + except Exception: + pass + """.formatted(PACKAGES_FILE); + + public PythonScriptEngine(ExternalScriptEngineDefinition def) + { + super(def); + } + + @Override + protected @Nullable String getPackageCaptureEpilog(ScriptContext context) + { + return PACKAGE_CAPTURE_EPILOG; + } + + @Override + protected void recordPackageUsage(ScriptContext context) + { + readPackageSidecar(context, PACKAGES_FILE, "python"); + } +} diff --git a/core/src/org/labkey/core/reports/ScriptEngineManagerImpl.java b/core/src/org/labkey/core/reports/ScriptEngineManagerImpl.java index 7ed2a4ea1b5..33badffd6de 100644 --- a/core/src/org/labkey/core/reports/ScriptEngineManagerImpl.java +++ b/core/src/org/labkey/core/reports/ScriptEngineManagerImpl.java @@ -47,6 +47,7 @@ import org.labkey.api.reports.ExternalScriptEngineDefinition; import org.labkey.api.reports.ExternalScriptEngineFactory; import org.labkey.api.reports.LabKeyScriptEngineManager; +import org.labkey.api.reports.report.python.PythonScriptEngine; import org.labkey.api.reports.report.r.RDockerScriptEngineFactory; import org.labkey.api.reports.report.r.RScriptEngineFactory; import org.labkey.api.reports.report.r.RemoteRNotEnabledException; @@ -348,12 +349,26 @@ else if (def.getType().equals(ExternalScriptEngineDefinition.Type.Jupyter) && !P LOG.error("Jupyter Report engine [{}] requested, but premium module not available/enabled.", def.getName()); throw new PremiumFeatureNotEnabledException("Jupyter Reports are not available. Please talk to your account representative for additional information."); } + else if (isPythonEngine(def)) + return new PythonScriptEngine(def); else return new ExternalScriptEngineFactory(def).getScriptEngine(); } return null; } + // A Python engine is configured as a generic external engine (there is no dedicated Type.Python), so identify it by + // its ".py" file extension. R and Jupyter (.ipynb) are handled by earlier branches, so they won't match here. + private static boolean isPythonEngine(ExternalScriptEngineDefinition def) + { + for (String ext : def.getExtensions()) + { + if ("py".equalsIgnoreCase(ext)) + return true; + } + return false; + } + // Locates any specific engines scoped at either the container or project level for an engine context @Override @Nullable From cea599ace098e489120ff50b76a1fec40e7e51ca Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 24 Jul 2026 10:01:11 -0500 Subject: [PATCH 04/13] ScriptPackageUsageTracker to use SimpleMetricsService to increment count of number of times a package was used --- .../report/ScriptPackageUsageTracker.java | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java diff --git a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java new file mode 100644 index 00000000000..2884c1093b6 --- /dev/null +++ b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.reports.report; + +import org.labkey.api.reports.ExternalScriptEngine; +import org.labkey.api.usageMetrics.SimpleMetricsService; + +import java.util.Map; +import java.util.Set; + +/** + * Tracks which packages/modules are loaded by scripts run on this server (R reports, assay transform scripts, Python + * scripts, and anything else that runs through {@link ExternalScriptEngine}). Populated by a language-specific epilog + * appended to each script that captures the loaded packages and writes them to a sidecar file, which the engine reads + * back after the script runs. Usage is tracked per language (e.g. "r", "python"). + * + * Each load is recorded via {@link SimpleMetricsService}, which persists a cumulative per-package load count across + * restarts and reports it to mothership under "simpleMetricCounts". The package name is the metric name and the feature + * area is "<language>PackageUsage". + */ +public class ScriptPackageUsageTracker +{ + private static final String MODULE_NAME = "API"; + private static final String FEATURE_AREA_SUFFIX = "PackageUsage"; + + /** + * Packages that ship with a given language's runtime and are always present, so aren't interesting as "library + * usage". R's base packages are filtered here; Python's standard library is filtered in the capture epilog itself + * (via sys.stdlib_module_names), so no Python entry is needed. + */ + private static final Map> BASE_PACKAGES = Map.of( + "r", Set.of("base", "compiler", "datasets", "graphics", "grDevices", "methods", "stats", "utils") + ); + + private ScriptPackageUsageTracker() + { + } + + private static boolean isBasePackage(String language, String packageName) + { + return BASE_PACKAGES.getOrDefault(language, Set.of()).contains(packageName); + } + + /** + * Record that the given package was loaded by a script run in the given language (e.g. "r", "python"). Safe to call + * repeatedly; base packages and blank names are ignored. + */ + public static void record(String language, String packageName) + { + if (packageName == null || packageName.isBlank() || isBasePackage(language, packageName)) + return; + + SimpleMetricsService.get().increment(MODULE_NAME, language + FEATURE_AREA_SUFFIX, packageName); + } +} From 4b91d83b5ae9e30c17c9283c711784d876e2e30e Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 24 Jul 2026 10:22:13 -0500 Subject: [PATCH 05/13] add try/catches to make sure we don't faile a valid script execution --- .../api/reports/ExternalScriptEngine.java | 60 +++++++++++++++---- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/api/src/org/labkey/api/reports/ExternalScriptEngine.java b/api/src/org/labkey/api/reports/ExternalScriptEngine.java index 9646012c032..428750e1401 100644 --- a/api/src/org/labkey/api/reports/ExternalScriptEngine.java +++ b/api/src/org/labkey/api/reports/ExternalScriptEngine.java @@ -125,12 +125,30 @@ public Object eval(String script, ScriptContext context) throws ScriptException try { Object result = eval(scriptFile, context); - recordSuccessfulRun(context); + + // Metric tracking must never affect script execution, so swallow any failure here + try + { + recordSuccessfulRun(context); + } + catch (Exception e) + { + LOG.warn("Failed to record successful script run", e); + } + return result; } finally { - recordPackageUsage(context); + // Guard so a metric failure can't mask the script's result or a real exception + try + { + recordPackageUsage(context); + } + catch (Exception e) + { + LOG.warn("Failed to record script package usage", e); + } } } @@ -178,27 +196,45 @@ protected void recordSuccessfulRun(ScriptContext context) * GitHub Issue #1130 * Read a sidecar file of package names (one per line) from the working directory and record each under the given * language in {@link ScriptPackageUsageTracker}. Never throws. A missing file means the script errored before the - * capture epilog ran (or capture was skipped) - nothing to do. + * capture epilog ran (or capture was skipped) - nothing to do. The file is deleted after reading. */ protected void readPackageSidecar(ScriptContext context, String fileName, String language) { + FileLike packagesFile; try { - FileLike packagesFile = getWorkingDir(context).resolveChild(fileName); - if (!packagesFile.exists()) - return; + packagesFile = getWorkingDir(context).resolveChild(fileName); + } + catch (Exception e) + { + LOG.warn("Failed to locate " + language + " package usage sidecar", e); + return; + } - try (BufferedReader reader = Readers.getReader(packagesFile.openInputStream())) - { - String packageName; - while ((packageName = reader.readLine()) != null) - ScriptPackageUsageTracker.record(language, packageName); - } + if (!packagesFile.exists()) + return; + + try (BufferedReader reader = Readers.getReader(packagesFile.openInputStream())) + { + String packageName; + while ((packageName = reader.readLine()) != null) + ScriptPackageUsageTracker.record(language, packageName); } catch (Exception e) { LOG.warn("Failed to record " + language + " package usage", e); } + finally + { + try + { + packagesFile.delete(); + } + catch (Exception e) + { + LOG.warn("Failed to delete " + language + " package usage sidecar", e); + } + } } protected Object eval(FileLike scriptFile, ScriptContext context) throws ScriptException From f79850e3db2f4e3ee6232a42ef966f0be1063436 Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 24 Jul 2026 10:51:27 -0500 Subject: [PATCH 06/13] CR feedback --- .../org/labkey/core/reports/ScriptEngineManagerImpl.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/core/src/org/labkey/core/reports/ScriptEngineManagerImpl.java b/core/src/org/labkey/core/reports/ScriptEngineManagerImpl.java index 33badffd6de..aace2da21d4 100644 --- a/core/src/org/labkey/core/reports/ScriptEngineManagerImpl.java +++ b/core/src/org/labkey/core/reports/ScriptEngineManagerImpl.java @@ -361,12 +361,7 @@ else if (isPythonEngine(def)) // its ".py" file extension. R and Jupyter (.ipynb) are handled by earlier branches, so they won't match here. private static boolean isPythonEngine(ExternalScriptEngineDefinition def) { - for (String ext : def.getExtensions()) - { - if ("py".equalsIgnoreCase(ext)) - return true; - } - return false; + return Arrays.stream(def.getExtensions()).anyMatch("py"::equalsIgnoreCase); } // Locates any specific engines scoped at either the container or project level for an engine context From 42473bb251050188946ca0ab530d97bb37532416 Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 28 Jul 2026 11:38:22 -0500 Subject: [PATCH 07/13] Code review feedback: limit # packages read and truncate long package names --- .../labkey/api/reports/ExternalScriptEngine.java | 16 ++++++++++++++-- .../report/ScriptPackageUsageTracker.java | 14 ++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/reports/ExternalScriptEngine.java b/api/src/org/labkey/api/reports/ExternalScriptEngine.java index 428750e1401..bcc0b8f152d 100644 --- a/api/src/org/labkey/api/reports/ExternalScriptEngine.java +++ b/api/src/org/labkey/api/reports/ExternalScriptEngine.java @@ -84,6 +84,8 @@ public class ExternalScriptEngine extends AbstractScriptEngine implements LabKey public static final String DEFAULT_WORKING_DIRECTORY = "ExternalScript"; private static final Pattern scriptCmdPattern = Pattern.compile("'([^']+)'|\\\"([^\\\"]+)\\\"|(^[^\\s]+)|(\\s[^\\s^'^\\\"]+)"); + private static final int MAX_PACKAGES_PER_RUN = 100; + private FileLike _workingDirectory; protected ExternalScriptEngineDefinition _def; @@ -195,8 +197,9 @@ protected void recordSuccessfulRun(ScriptContext context) /** * GitHub Issue #1130 * Read a sidecar file of package names (one per line) from the working directory and record each under the given - * language in {@link ScriptPackageUsageTracker}. Never throws. A missing file means the script errored before the - * capture epilog ran (or capture was skipped) - nothing to do. The file is deleted after reading. + * language in {@link ScriptPackageUsageTracker}, up to {@link #MAX_PACKAGES_PER_RUN} per run. Never throws. A + * missing file means the script errored before the capture epilog ran (or capture was skipped) - nothing to do. + * The file is deleted after reading. */ protected void readPackageSidecar(ScriptContext context, String fileName, String language) { @@ -217,8 +220,17 @@ protected void readPackageSidecar(ScriptContext context, String fileName, String try (BufferedReader reader = Readers.getReader(packagesFile.openInputStream())) { String packageName; + int recorded = 0; while ((packageName = reader.readLine()) != null) + { + if (recorded >= MAX_PACKAGES_PER_RUN) + { + LOG.warn("Recorded the first {} {} packages for this script run and ignored the rest", MAX_PACKAGES_PER_RUN, language); + break; + } ScriptPackageUsageTracker.record(language, packageName); + recorded++; + } } catch (Exception e) { diff --git a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java index 2884c1093b6..6d3ceb00dba 100644 --- a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java +++ b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java @@ -35,6 +35,7 @@ public class ScriptPackageUsageTracker { private static final String MODULE_NAME = "API"; private static final String FEATURE_AREA_SUFFIX = "PackageUsage"; + private static final int MAX_METRIC_NAME_LENGTH = 255; /** * Packages that ship with a given language's runtime and are always present, so aren't interesting as "library @@ -42,7 +43,7 @@ public class ScriptPackageUsageTracker * (via sys.stdlib_module_names), so no Python entry is needed. */ private static final Map> BASE_PACKAGES = Map.of( - "r", Set.of("base", "compiler", "datasets", "graphics", "grDevices", "methods", "stats", "utils") + "r", Set.of("base", "compiler", "datasets", "graphics", "grDevices", "methods", "stats", "tools", "utils") ); private ScriptPackageUsageTracker() @@ -63,6 +64,15 @@ public static void record(String language, String packageName) if (packageName == null || packageName.isBlank() || isBasePackage(language, packageName)) return; - SimpleMetricsService.get().increment(MODULE_NAME, language + FEATURE_AREA_SUFFIX, packageName); + SimpleMetricsService.get().increment(MODULE_NAME, language + FEATURE_AREA_SUFFIX, truncateMetricName(packageName)); + } + + /** + * The package name is used as the metric name, and the names come from whatever the script actually loaded rather + * than from a fixed list, so cap the length at what the DB column holds. + */ + private static String truncateMetricName(String packageName) + { + return packageName.length() <= MAX_METRIC_NAME_LENGTH ? packageName : packageName.substring(0, MAX_METRIC_NAME_LENGTH); } } From e2f06e48d275ad31c3417597b18dacab705a605f Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 28 Jul 2026 11:39:07 -0500 Subject: [PATCH 08/13] RserveScriptEngine implementation of package capture (given that it has its own eval() override) --- .../reports/report/r/RserveScriptEngine.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java b/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java index b8d284e5e08..c5b06512665 100644 --- a/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java @@ -314,6 +314,11 @@ public Object eval(String script, ScriptContext context) throws ScriptException LOG.info("Reusing RServe connection in use: {}", rh.isInUse()); } + // GitHub Issue #1130 + String epilog = getPackageCaptureEpilog(context); + if (epilog != null) + script = script + "\n" + epilog; + FileLike scriptFile = prepareScriptFile(script, context, extensions); try @@ -339,6 +344,16 @@ public Object eval(String script, ScriptContext context) throws ScriptException // no logging here, because this is a no-op by default copyWorkingDirectoryFromRemote(rconn); + // GitHub Issue #1130: Metric tracking must never affect script execution, so swallow any failure here + try + { + recordSuccessfulRun(context); + } + catch (Exception e) + { + LOG.warn("Failed to record successful script run", e); + } + return output; } catch (IOException|RserveException x) @@ -348,6 +363,16 @@ public Object eval(String script, ScriptContext context) throws ScriptException finally { closeConnection(rconn, rh); + + // GitHub Issue #1130: Guard so a metric failure can't mask the script's result or a real exception + try + { + recordPackageUsage(context); + } + catch (Exception e) + { + LOG.warn("Failed to record script package usage", e); + } } } From 2f781f96f88808b63d7dc5d5b2d078e1d18a0db1 Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 28 Jul 2026 13:20:39 -0500 Subject: [PATCH 09/13] change to prepend instead of append script so that we can capture package usage on exit/failure --- .../api/reports/ExternalScriptEngine.java | 18 +++++------ .../report/ScriptPackageUsageTracker.java | 8 ++--- .../report/python/PythonScriptEngine.java | 30 ++++++++++++------- .../api/reports/report/r/RScriptEngine.java | 27 ++++++++++------- .../reports/report/r/RserveScriptEngine.java | 28 +++++++++++++++-- 5 files changed, 73 insertions(+), 38 deletions(-) diff --git a/api/src/org/labkey/api/reports/ExternalScriptEngine.java b/api/src/org/labkey/api/reports/ExternalScriptEngine.java index bcc0b8f152d..96fd8fc4cd4 100644 --- a/api/src/org/labkey/api/reports/ExternalScriptEngine.java +++ b/api/src/org/labkey/api/reports/ExternalScriptEngine.java @@ -119,9 +119,9 @@ public Object eval(String script, ScriptContext context) throws ScriptException if (extensions.isEmpty()) throw new ScriptException("There are no file name extensions registered for this ScriptEngine : " + getFactory().getLanguageName()); - String epilog = getPackageCaptureEpilog(context); - if (epilog != null) - script = script + "\n" + epilog; + String packageCapture = getPackageCaptureProlog(context); + if (packageCapture != null) + script = packageCapture + "\n" + script; FileLike scriptFile = prepareScriptFile(script, context, extensions); try @@ -165,12 +165,12 @@ protected FileLike prepareScriptFile(String script, ScriptContext context, List< /** * GitHub Issue #1130 - * Script appended to the end of the user script (in the same process) that captures the loaded packages/modules and - * writes them, one per line, to a sidecar file in the working directory for {@link #recordPackageUsage} to read - * back. The default returns null (no capture); language-specific engines (e.g. R, Python) override this. Wrapped so - * a capture failure can never break the script run. + * Script prepended to the user script (in the same process) that registers an exit hook to capture the loaded + * packages/modules, writing them one per line to a sidecar file in the working directory for + * {@link #recordPackageUsage} to read back. The default returns null (no capture); language-specific + * engines (e.g. R, Python) override this. Wrapped so a capture failure can never break the script run. */ - protected @Nullable String getPackageCaptureEpilog(ScriptContext context) + protected @Nullable String getPackageCaptureProlog(ScriptContext context) { return null; } @@ -198,7 +198,7 @@ protected void recordSuccessfulRun(ScriptContext context) * GitHub Issue #1130 * Read a sidecar file of package names (one per line) from the working directory and record each under the given * language in {@link ScriptPackageUsageTracker}, up to {@link #MAX_PACKAGES_PER_RUN} per run. Never throws. A - * missing file means the script errored before the capture epilog ran (or capture was skipped) - nothing to do. + * missing file means the capture code never ran (or was skipped) - nothing to do. * The file is deleted after reading. */ protected void readPackageSidecar(ScriptContext context, String fileName, String language) diff --git a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java index 6d3ceb00dba..bbd7e415c00 100644 --- a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java +++ b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java @@ -23,9 +23,9 @@ /** * Tracks which packages/modules are loaded by scripts run on this server (R reports, assay transform scripts, Python - * scripts, and anything else that runs through {@link ExternalScriptEngine}). Populated by a language-specific epilog - * appended to each script that captures the loaded packages and writes them to a sidecar file, which the engine reads - * back after the script runs. Usage is tracked per language (e.g. "r", "python"). + * scripts, and anything else that runs through {@link ExternalScriptEngine}). Populated by a language-specific prolog + * prepended to each script that registers an exit hook to write the loaded packages to a sidecar file, which the engine + * reads back after the script runs. Usage is tracked per language (e.g. "r", "python"). * * Each load is recorded via {@link SimpleMetricsService}, which persists a cumulative per-package load count across * restarts and reports it to mothership under "simpleMetricCounts". The package name is the metric name and the feature @@ -39,7 +39,7 @@ public class ScriptPackageUsageTracker /** * Packages that ship with a given language's runtime and are always present, so aren't interesting as "library - * usage". R's base packages are filtered here; Python's standard library is filtered in the capture epilog itself + * usage". R's base packages are filtered here; Python's standard library is filtered in the capture prolog itself * (via sys.stdlib_module_names), so no Python entry is needed. */ private static final Map> BASE_PACKAGES = Map.of( diff --git a/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java b/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java index 8d8844d8bb6..88736e56a40 100644 --- a/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java @@ -23,7 +23,7 @@ /** * Script engine for locally-executed Python scripts (Python assay transform scripts configured as an - * external ".py" engine). Behaves like the base {@link ExternalScriptEngine} except that it appends a capture epilog to + * external ".py" engine). Behaves like the base {@link ExternalScriptEngine} except that it prepends a capture prolog to * track which Python modules each script loads; see * {@link org.labkey.api.reports.report.ScriptPackageUsageTracker}. */ @@ -31,16 +31,24 @@ public class PythonScriptEngine extends ExternalScriptEngine { private static final String PACKAGES_FILE = "labkeyPythonPackages.txt"; - // 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 = """ + // Python prepended to a user script that registers an atexit hook to capture the loaded modules (top-level names, + // excluding the standard library). atexit runs on interpreter shutdown, including after an uncaught exception, so + // we also capture usage for scripts that fail. try/except means a capture failure can never break + // the script run, and registering the hook inside the try keeps a capture error out of the script's own traceback. + private static final String PACKAGE_CAPTURE_PROLOG = """ # --- LabKey Python package usage capture --- try: - import sys as _lk_sys - _lk_stdlib = set(getattr(_lk_sys, 'stdlib_module_names', ())) - _lk_mods = sorted({m.split('.')[0] for m in list(_lk_sys.modules)} - _lk_stdlib) - with open('%s', 'w') as _lk_f: - _lk_f.write('\\n'.join(m for m in _lk_mods if m and not m.startswith('_'))) + import atexit as _lk_atexit, os as _lk_os, sys as _lk_sys + _lk_packages_file = _lk_os.path.join(_lk_os.getcwd(), '%s') + def _lk_write_packages(): + try: + _lk_stdlib = set(getattr(_lk_sys, 'stdlib_module_names', ())) + _lk_mods = sorted({m.split('.')[0] for m in list(_lk_sys.modules)} - _lk_stdlib) + with open(_lk_packages_file, 'w') as _lk_f: + _lk_f.write('\\n'.join(m for m in _lk_mods if m and not m.startswith('_'))) + except Exception: + pass + _lk_atexit.register(_lk_write_packages) except Exception: pass """.formatted(PACKAGES_FILE); @@ -51,9 +59,9 @@ public PythonScriptEngine(ExternalScriptEngineDefinition def) } @Override - protected @Nullable String getPackageCaptureEpilog(ScriptContext context) + protected @Nullable String getPackageCaptureProlog(ScriptContext context) { - return PACKAGE_CAPTURE_EPILOG; + return PACKAGE_CAPTURE_PROLOG; } @Override diff --git a/api/src/org/labkey/api/reports/report/r/RScriptEngine.java b/api/src/org/labkey/api/reports/report/r/RScriptEngine.java index 26574247605..e16f2de2692 100644 --- a/api/src/org/labkey/api/reports/report/r/RScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/r/RScriptEngine.java @@ -36,7 +36,7 @@ */ public class RScriptEngine extends ExternalScriptEngine { - private static final String PACKAGES_FILE = "labkeyRPackages.txt"; + protected static final String PACKAGES_FILE = "labkeyRPackages.txt"; public static final String KNITR_FORMAT = "r.script.engine.knitrFormat"; public static final String KNITR_OUTPUT = "r.script.engine.knitrOutput"; public static final String PANDOC_USE_DEFAULT_OUTPUT_FORMAT = "r.script.engine.pandocUseDefaultOutputFormat"; @@ -90,23 +90,28 @@ protected FileLike prepareScriptFile(String script, ScriptContext context, List< } /** - * R appended to the end of the user script (in the same R session) that captures the set of loaded packages, - * writing them (one per line) to a sidecar file in the working directory for {@link #recordPackageUsage} to read - * back. Wrapped in tryCatch so a capture failure can never break the report or transform run. + * R prepended to the user script (in the same R session) that registers a finalizer on the global environment to + * write the set of loaded packages (one per line) to a sidecar file for {@link #recordPackageUsage} to read back. + * reg.finalizer(onexit = TRUE) runs as R shuts down, including when the script errors out ("Execution halted"), so + * we capture usage for failed runs and scripts calling q() too. Note: tryCatch means a capture failure can never + * break the report or transform run */ @Override - protected @Nullable String getPackageCaptureEpilog(ScriptContext context) + protected @Nullable String getPackageCaptureProlog(ScriptContext context) { - // For knitr the executed file is a generated wrapper (createKnitrScript), not the user's R, so appending R here - // wouldn't run. We instead record the wrapper's known libraries in recordSuccessfulRun(). + // For knitr the user's script is written as the knitr input (.rhtml/.rmd), where prepended bare R would render + // as text rather than run. We instead record the generated wrapper's known libraries in recordSuccessfulRun(). if (getKnitrFormat(context) != RReportDescriptor.KnitrFormat.None) return null; return """ # --- LabKey R package usage capture --- - tryCatch({ - writeLines(sort(loadedNamespaces()), "%s") - }, error = function(e) invisible(NULL)) + local({ + labkeyPackagesFile <- file.path(getwd(), "%s") + invisible(reg.finalizer(globalenv(), function(e) { + tryCatch(writeLines(sort(loadedNamespaces()), labkeyPackagesFile), error = function(e) invisible(NULL)) + }, onexit = TRUE)) + }) """.formatted(PACKAGES_FILE); } @@ -119,7 +124,7 @@ protected void recordPackageUsage(ScriptContext context) @Override protected void recordSuccessfulRun(ScriptContext context) { - // Non-knitr R runs report their loaded packages via the capture epilog (see getPackageCaptureEpilog). + // Non-knitr R runs report their loaded packages via the capture prolog (see getPackageCaptureProlog). if (getKnitrFormat(context) == RReportDescriptor.KnitrFormat.None) return; diff --git a/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java b/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java index c5b06512665..95a295e53b5 100644 --- a/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java @@ -19,6 +19,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.xmlbeans.impl.common.IOUtil; +import org.jetbrains.annotations.Nullable; import org.labkey.api.pipeline.file.PathMapper; import org.labkey.api.reports.ExternalScriptEngineDefinition; import org.labkey.api.reports.LabKeyScriptEngineManager; @@ -289,6 +290,24 @@ protected void copyWorkingDirectoryFromRemote(RConnection rconn) throws IOExcept } + /** + * GitHub Issue #1130 + * Writes the loaded packages directly rather than registering the finalizer {@link RScriptEngine} uses, because our + * eval() appends this instead of prepending it. + */ + @Override + protected @Nullable String getPackageCaptureProlog(ScriptContext context) + { + // As in RScriptEngine, knitr runs a generated wrapper and reports its libraries via recordSuccessfulRun(). + if (getKnitrFormat(context) != RReportDescriptor.KnitrFormat.None) + return null; + + return """ + # --- LabKey R package usage capture --- + tryCatch(writeLines(sort(loadedNamespaces()), file.path(getwd(), "%s")), error = function(e) invisible(NULL)) + """.formatted(PACKAGES_FILE); + } + @Override public Object eval(String script, ScriptContext context) throws ScriptException { @@ -315,9 +334,12 @@ public Object eval(String script, ScriptContext context) throws ScriptException } // GitHub Issue #1130 - String epilog = getPackageCaptureEpilog(context); - if (epilog != null) - script = script + "\n" + epilog; + // Appended here, unlike ExternalScriptEngine.eval() which prepends it: the R session on the remote outlives the + // script, so an exit hook wouldn't run until after copyWorkingDirectoryFromRemote() below. + // The tradeoff is that a script that errors out reports no package usage. + String packageCapture = getPackageCaptureProlog(context); + if (packageCapture != null) + script = script + "\n" + packageCapture; FileLike scriptFile = prepareScriptFile(script, context, extensions); From 802cbfa01d2701081a2499fbd9cc2fdc02ce247f Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 28 Jul 2026 15:08:35 -0500 Subject: [PATCH 10/13] revert back to epilog instead of prolog --- .../api/reports/ExternalScriptEngine.java | 70 ++++++++----------- .../report/ScriptPackageUsageTracker.java | 13 ++-- .../report/python/PythonScriptEngine.java | 30 +++----- .../api/reports/report/r/RScriptEngine.java | 32 +++------ .../reports/report/r/RserveScriptEngine.java | 54 ++------------ 5 files changed, 65 insertions(+), 134 deletions(-) diff --git a/api/src/org/labkey/api/reports/ExternalScriptEngine.java b/api/src/org/labkey/api/reports/ExternalScriptEngine.java index 96fd8fc4cd4..2521bf2c656 100644 --- a/api/src/org/labkey/api/reports/ExternalScriptEngine.java +++ b/api/src/org/labkey/api/reports/ExternalScriptEngine.java @@ -119,39 +119,13 @@ public Object eval(String script, ScriptContext context) throws ScriptException if (extensions.isEmpty()) throw new ScriptException("There are no file name extensions registered for this ScriptEngine : " + getFactory().getLanguageName()); - String packageCapture = getPackageCaptureProlog(context); - if (packageCapture != null) - script = packageCapture + "\n" + script; + FileLike scriptFile = prepareScriptFile(appendPackageCaptureEpilog(script, context), context, extensions); + Object result = eval(scriptFile, context); - FileLike scriptFile = prepareScriptFile(script, context, extensions); - try - { - Object result = eval(scriptFile, context); - - // Metric tracking must never affect script execution, so swallow any failure here - try - { - recordSuccessfulRun(context); - } - catch (Exception e) - { - LOG.warn("Failed to record successful script run", e); - } + // Only reached when the script succeeded; a failed run reports no package usage + recordPackageUsage(context); - return result; - } - finally - { - // Guard so a metric failure can't mask the script's result or a real exception - try - { - recordPackageUsage(context); - } - catch (Exception e) - { - LOG.warn("Failed to record script package usage", e); - } - } + return result; } /** @@ -165,32 +139,44 @@ protected FileLike prepareScriptFile(String script, ScriptContext context, List< /** * GitHub Issue #1130 - * Script prepended to the user script (in the same process) that registers an exit hook to capture the loaded + * Script appended to the end of the user script (running in the same process) that captures the loaded * packages/modules, writing them one per line to a sidecar file in the working directory for - * {@link #recordPackageUsage} to read back. The default returns null (no capture); language-specific - * engines (e.g. R, Python) override this. Wrapped so a capture failure can never break the script run. + * {@link #recordPackageUsage} to read back. The default returns null (no capture); language-specific engines + * (e.g. R, Python) override this. */ - protected @Nullable String getPackageCaptureProlog(ScriptContext context) + protected @Nullable String getPackageCaptureEpilog(ScriptContext context) { return null; } /** * GitHub Issue #1130 - * After a script runs, read back and record the packages loaded by the script. The default does nothing; - * language-specific engines override this, typically delegating to {@link #readPackageSidecar}. Never throws: - * package tracking must not affect script execution. + * Append this engine's package capture epilog, if it has one, to the end of the given script. + * Never throws: package tracking must not affect script execution. */ - protected void recordPackageUsage(ScriptContext context) + protected String appendPackageCaptureEpilog(String script, ScriptContext context) { + try + { + String epilog = getPackageCaptureEpilog(context); + if (epilog != null) + return script + "\n" + epilog; + } + catch (Exception e) + { + LOG.warn("Failed to build the script package capture epilog", e); + } + + return script; } /** * GitHub Issue #1130 - * Called after a script has run successfully (eval returned without throwing). The default does nothing; - * language-specific engines may override to record success metrics. + * Called after a script has run successfully (eval returned without throwing), to record the packages it loaded. + * The default does nothing; language-specific engines override this, typically delegating to + * {@link #readPackageSidecar}. */ - protected void recordSuccessfulRun(ScriptContext context) + protected void recordPackageUsage(ScriptContext context) { } diff --git a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java index bbd7e415c00..e49aec76526 100644 --- a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java +++ b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java @@ -23,9 +23,12 @@ /** * Tracks which packages/modules are loaded by scripts run on this server (R reports, assay transform scripts, Python - * scripts, and anything else that runs through {@link ExternalScriptEngine}). Populated by a language-specific prolog - * prepended to each script that registers an exit hook to write the loaded packages to a sidecar file, which the engine - * reads back after the script runs. Usage is tracked per language (e.g. "r", "python"). + * scripts, and anything else that runs through {@link ExternalScriptEngine}). Populated by a language-specific epilog + * appended to each script that writes the loaded packages to a sidecar file, which the engine reads back after the + * script has run successfully. Usage is tracked per language (e.g. "r", "python"). + *

+ * Note that this only sees scripts that ran to completion: a script that fails, or that exits early via q() or + * sys.exit(), never reaches the epilog and so reports nothing. Counts are a lower bound. * * Each load is recorded via {@link SimpleMetricsService}, which persists a cumulative per-package load count across * restarts and reports it to mothership under "simpleMetricCounts". The package name is the metric name and the feature @@ -39,11 +42,11 @@ public class ScriptPackageUsageTracker /** * Packages that ship with a given language's runtime and are always present, so aren't interesting as "library - * usage". R's base packages are filtered here; Python's standard library is filtered in the capture prolog itself + * usage". R's base packages are filtered here; Python's standard library is filtered in the capture epilog itself * (via sys.stdlib_module_names), so no Python entry is needed. */ private static final Map> BASE_PACKAGES = Map.of( - "r", Set.of("base", "compiler", "datasets", "graphics", "grDevices", "methods", "stats", "tools", "utils") + "r", Set.of("base", "compiler", "datasets", "graphics", "grDevices", "grid", "methods", "parallel", "splines", "stats", "stats4", "tcltk", "tools", "utils") ); private ScriptPackageUsageTracker() diff --git a/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java b/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java index 88736e56a40..f8d14b6dca3 100644 --- a/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java @@ -23,7 +23,7 @@ /** * Script engine for locally-executed Python scripts (Python assay transform scripts configured as an - * external ".py" engine). Behaves like the base {@link ExternalScriptEngine} except that it prepends a capture prolog to + * external ".py" engine). Behaves like the base {@link ExternalScriptEngine} except that it appends a capture epilog to * track which Python modules each script loads; see * {@link org.labkey.api.reports.report.ScriptPackageUsageTracker}. */ @@ -31,24 +31,16 @@ public class PythonScriptEngine extends ExternalScriptEngine { private static final String PACKAGES_FILE = "labkeyPythonPackages.txt"; - // Python prepended to a user script that registers an atexit hook to capture the loaded modules (top-level names, - // excluding the standard library). atexit runs on interpreter shutdown, including after an uncaught exception, so - // we also capture usage for scripts that fail. try/except means a capture failure can never break - // the script run, and registering the hook inside the try keeps a capture error out of the script's own traceback. - private static final String PACKAGE_CAPTURE_PROLOG = """ + // Python appended to a user script to capture the loaded modules (top-level names, excluding the standard library + // and private names). try/except means a capture failure can never break the script run. + private static final String PACKAGE_CAPTURE_EPILOG = """ # --- LabKey Python package usage capture --- try: - import atexit as _lk_atexit, os as _lk_os, sys as _lk_sys - _lk_packages_file = _lk_os.path.join(_lk_os.getcwd(), '%s') - def _lk_write_packages(): - try: - _lk_stdlib = set(getattr(_lk_sys, 'stdlib_module_names', ())) - _lk_mods = sorted({m.split('.')[0] for m in list(_lk_sys.modules)} - _lk_stdlib) - with open(_lk_packages_file, 'w') as _lk_f: - _lk_f.write('\\n'.join(m for m in _lk_mods if m and not m.startswith('_'))) - except Exception: - pass - _lk_atexit.register(_lk_write_packages) + import os as _lk_os, sys as _lk_sys + _lk_stdlib = set(getattr(_lk_sys, 'stdlib_module_names', ())) + _lk_mods = sorted({m.split('.')[0] for m in list(_lk_sys.modules)} - _lk_stdlib) + with open(_lk_os.path.join(_lk_os.getcwd(), '%s'), 'w') as _lk_f: + _lk_f.write('\\n'.join(m for m in _lk_mods if m and not m.startswith('_'))) except Exception: pass """.formatted(PACKAGES_FILE); @@ -59,9 +51,9 @@ public PythonScriptEngine(ExternalScriptEngineDefinition def) } @Override - protected @Nullable String getPackageCaptureProlog(ScriptContext context) + protected @Nullable String getPackageCaptureEpilog(ScriptContext context) { - return PACKAGE_CAPTURE_PROLOG; + return PACKAGE_CAPTURE_EPILOG; } @Override diff --git a/api/src/org/labkey/api/reports/report/r/RScriptEngine.java b/api/src/org/labkey/api/reports/report/r/RScriptEngine.java index e16f2de2692..d95b08ca682 100644 --- a/api/src/org/labkey/api/reports/report/r/RScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/r/RScriptEngine.java @@ -90,43 +90,33 @@ protected FileLike prepareScriptFile(String script, ScriptContext context, List< } /** - * R prepended to the user script (in the same R session) that registers a finalizer on the global environment to - * write the set of loaded packages (one per line) to a sidecar file for {@link #recordPackageUsage} to read back. - * reg.finalizer(onexit = TRUE) runs as R shuts down, including when the script errors out ("Execution halted"), so - * we capture usage for failed runs and scripts calling q() too. Note: tryCatch means a capture failure can never - * break the report or transform run + * R appended to the end of the user script (running in the same R session) that writes the set of loaded packages, + * one per line, to a sidecar file for {@link #recordPackageUsage} to read back. Notes: tryCatch means a capture + * failure can never break the report or transform run. */ @Override - protected @Nullable String getPackageCaptureProlog(ScriptContext context) + protected @Nullable String getPackageCaptureEpilog(ScriptContext context) { - // For knitr the user's script is written as the knitr input (.rhtml/.rmd), where prepended bare R would render - // as text rather than run. We instead record the generated wrapper's known libraries in recordSuccessfulRun(). + // For knitr the user's script is written as the knitr input (.rhtml/.rmd), where appended bare R would render + // as text rather than run. We instead record the generated wrapper's known libraries in recordPackageUsage(). if (getKnitrFormat(context) != RReportDescriptor.KnitrFormat.None) return null; return """ # --- LabKey R package usage capture --- - local({ - labkeyPackagesFile <- file.path(getwd(), "%s") - invisible(reg.finalizer(globalenv(), function(e) { - tryCatch(writeLines(sort(loadedNamespaces()), labkeyPackagesFile), error = function(e) invisible(NULL)) - }, onexit = TRUE)) - }) + tryCatch(writeLines(sort(loadedNamespaces()), file.path(getwd(), "%s")), error = function(e) invisible(NULL)) """.formatted(PACKAGES_FILE); } @Override protected void recordPackageUsage(ScriptContext context) { - readPackageSidecar(context, PACKAGES_FILE, "r"); - } - - @Override - protected void recordSuccessfulRun(ScriptContext context) - { - // Non-knitr R runs report their loaded packages via the capture prolog (see getPackageCaptureProlog). + // Non-knitr R runs report their loaded packages via the capture epilog (see getPackageCaptureEpilog). if (getKnitrFormat(context) == RReportDescriptor.KnitrFormat.None) + { + readPackageSidecar(context, PACKAGES_FILE, "r"); return; + } // For knitr, the generated wrapper (createKnitrScript) always loads knitr and, for the markdown+pandoc path, // rmarkdown; record those as R package usage so they show up alongside other packages. diff --git a/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java b/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java index 95a295e53b5..8c32b59cb77 100644 --- a/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/r/RserveScriptEngine.java @@ -19,7 +19,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.xmlbeans.impl.common.IOUtil; -import org.jetbrains.annotations.Nullable; import org.labkey.api.pipeline.file.PathMapper; import org.labkey.api.reports.ExternalScriptEngineDefinition; import org.labkey.api.reports.LabKeyScriptEngineManager; @@ -290,24 +289,6 @@ protected void copyWorkingDirectoryFromRemote(RConnection rconn) throws IOExcept } - /** - * GitHub Issue #1130 - * Writes the loaded packages directly rather than registering the finalizer {@link RScriptEngine} uses, because our - * eval() appends this instead of prepending it. - */ - @Override - protected @Nullable String getPackageCaptureProlog(ScriptContext context) - { - // As in RScriptEngine, knitr runs a generated wrapper and reports its libraries via recordSuccessfulRun(). - if (getKnitrFormat(context) != RReportDescriptor.KnitrFormat.None) - return null; - - return """ - # --- LabKey R package usage capture --- - tryCatch(writeLines(sort(loadedNamespaces()), file.path(getwd(), "%s")), error = function(e) invisible(NULL)) - """.formatted(PACKAGES_FILE); - } - @Override public Object eval(String script, ScriptContext context) throws ScriptException { @@ -333,15 +314,10 @@ public Object eval(String script, ScriptContext context) throws ScriptException LOG.info("Reusing RServe connection in use: {}", rh.isInUse()); } - // GitHub Issue #1130 - // Appended here, unlike ExternalScriptEngine.eval() which prepends it: the R session on the remote outlives the - // script, so an exit hook wouldn't run until after copyWorkingDirectoryFromRemote() below. - // The tradeoff is that a script that errors out reports no package usage. - String packageCapture = getPackageCaptureProlog(context); - if (packageCapture != null) - script = script + "\n" + packageCapture; - - FileLike scriptFile = prepareScriptFile(script, context, extensions); + // GitHub Issue #1130: capture the packages the script loaded (see RScriptEngine.getPackageCaptureEpilog). This + // has to run inside the script rather than as an exit hook, because the R session on the remote outlives the + // script and so wouldn't shut down until after copyWorkingDirectoryFromRemote() below. + FileLike scriptFile = prepareScriptFile(appendPackageCaptureEpilog(script, context), context, extensions); try { @@ -366,15 +342,9 @@ public Object eval(String script, ScriptContext context) throws ScriptException // no logging here, because this is a no-op by default copyWorkingDirectoryFromRemote(rconn); - // GitHub Issue #1130: Metric tracking must never affect script execution, so swallow any failure here - try - { - recordSuccessfulRun(context); - } - catch (Exception e) - { - LOG.warn("Failed to record successful script run", e); - } + // GitHub Issue #1130: only reached when the script succeeded, and has to follow the copy above so the + // sidecar file the epilog wrote on the remote is available locally + recordPackageUsage(context); return output; } @@ -385,16 +355,6 @@ public Object eval(String script, ScriptContext context) throws ScriptException finally { closeConnection(rconn, rh); - - // GitHub Issue #1130: Guard so a metric failure can't mask the script's result or a real exception - try - { - recordPackageUsage(context); - } - catch (Exception e) - { - LOG.warn("Failed to record script package usage", e); - } } } From c17e00fccdc457d8dc135408bd7b0f6f28c574f8 Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 28 Jul 2026 15:28:54 -0500 Subject: [PATCH 11/13] change to python epilog to track packages --- .../reports/report/python/PythonScriptEngine.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java b/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java index f8d14b6dca3..cf8c830c1ff 100644 --- a/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java +++ b/api/src/org/labkey/api/reports/report/python/PythonScriptEngine.java @@ -24,23 +24,24 @@ /** * Script engine for locally-executed Python scripts (Python assay transform scripts configured as an * external ".py" engine). Behaves like the base {@link ExternalScriptEngine} except that it appends a capture epilog to - * track which Python modules each script loads; see + * track which Python packages each script loads; see * {@link org.labkey.api.reports.report.ScriptPackageUsageTracker}. */ public class PythonScriptEngine extends ExternalScriptEngine { private static final String PACKAGES_FILE = "labkeyPythonPackages.txt"; - // Python appended to a user script to capture the loaded modules (top-level names, excluding the standard library - // and private names). try/except means a capture failure can never break the script run. + // Python appended to a user script to capture the packages it loaded. + // try/except means a capture failure can never break the script run. private static final String PACKAGE_CAPTURE_EPILOG = """ # --- LabKey Python package usage capture --- try: - import os as _lk_os, sys as _lk_sys - _lk_stdlib = set(getattr(_lk_sys, 'stdlib_module_names', ())) - _lk_mods = sorted({m.split('.')[0] for m in list(_lk_sys.modules)} - _lk_stdlib) + import importlib.metadata as _lk_md, os as _lk_os, sys as _lk_sys + _lk_dists = _lk_md.packages_distributions() + _lk_tops = {m.split('.')[0] for m in list(_lk_sys.modules)} - set(_lk_sys.stdlib_module_names) + _lk_names = sorted({d for t in _lk_tops if t and not t.startswith('_') for d in _lk_dists.get(t, ())}) with open(_lk_os.path.join(_lk_os.getcwd(), '%s'), 'w') as _lk_f: - _lk_f.write('\\n'.join(m for m in _lk_mods if m and not m.startswith('_'))) + _lk_f.write('\\n'.join(_lk_names)) except Exception: pass """.formatted(PACKAGES_FILE); From 8dacba119d01a99f5dedeafb16190237ac9b94c6 Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 28 Jul 2026 15:29:17 -0500 Subject: [PATCH 12/13] change to python epilog to track packages --- .../labkey/api/reports/report/ScriptPackageUsageTracker.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java index e49aec76526..fbba74f0c52 100644 --- a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java +++ b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java @@ -42,8 +42,8 @@ public class ScriptPackageUsageTracker /** * Packages that ship with a given language's runtime and are always present, so aren't interesting as "library - * usage". R's base packages are filtered here; Python's standard library is filtered in the capture epilog itself - * (via sys.stdlib_module_names), so no Python entry is needed. + * usage". R's base packages are filtered here; Python needs no entry because the capture epilog only reports names + * that map to an installed distribution, which the standard library never does. */ private static final Map> BASE_PACKAGES = Map.of( "r", Set.of("base", "compiler", "datasets", "graphics", "grDevices", "grid", "methods", "parallel", "splines", "stats", "stats4", "tcltk", "tools", "utils") From f0aa7cc8da459639207c0f216e9d3781a293a4e0 Mon Sep 17 00:00:00 2001 From: cnathe Date: Tue, 28 Jul 2026 16:02:16 -0500 Subject: [PATCH 13/13] claude cr feedback --- .../labkey/api/reports/ExternalScriptEngine.java | 2 +- .../reports/report/ScriptPackageUsageTracker.java | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/reports/ExternalScriptEngine.java b/api/src/org/labkey/api/reports/ExternalScriptEngine.java index 2521bf2c656..aa61a37d6b9 100644 --- a/api/src/org/labkey/api/reports/ExternalScriptEngine.java +++ b/api/src/org/labkey/api/reports/ExternalScriptEngine.java @@ -84,7 +84,7 @@ public class ExternalScriptEngine extends AbstractScriptEngine implements LabKey public static final String DEFAULT_WORKING_DIRECTORY = "ExternalScript"; private static final Pattern scriptCmdPattern = Pattern.compile("'([^']+)'|\\\"([^\\\"]+)\\\"|(^[^\\s]+)|(\\s[^\\s^'^\\\"]+)"); - private static final int MAX_PACKAGES_PER_RUN = 100; + private static final int MAX_PACKAGES_PER_RUN = 250; private FileLike _workingDirectory; diff --git a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java index fbba74f0c52..f47ba5cf2ce 100644 --- a/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java +++ b/api/src/org/labkey/api/reports/report/ScriptPackageUsageTracker.java @@ -15,8 +15,10 @@ */ package org.labkey.api.reports.report; +import org.apache.logging.log4j.Logger; import org.labkey.api.reports.ExternalScriptEngine; import org.labkey.api.usageMetrics.SimpleMetricsService; +import org.labkey.api.util.logging.LogHelper; import java.util.Map; import java.util.Set; @@ -36,6 +38,8 @@ */ public class ScriptPackageUsageTracker { + private static final Logger LOG = LogHelper.getLogger(ScriptPackageUsageTracker.class, "Tracks R & Python package usage by server-side scripts"); + private static final String MODULE_NAME = "API"; private static final String FEATURE_AREA_SUFFIX = "PackageUsage"; private static final int MAX_METRIC_NAME_LENGTH = 255; @@ -67,7 +71,14 @@ public static void record(String language, String packageName) if (packageName == null || packageName.isBlank() || isBasePackage(language, packageName)) return; - SimpleMetricsService.get().increment(MODULE_NAME, language + FEATURE_AREA_SUFFIX, truncateMetricName(packageName)); + try + { + SimpleMetricsService.get().increment(MODULE_NAME, language + FEATURE_AREA_SUFFIX, truncateMetricName(packageName)); + } + catch (Exception e) + { + LOG.warn("Failed to record {} package usage for '{}'", language, packageName, e); + } } /**