[Hotfix] Add perf monitoring trace for file encryption time #3496

avazirna · 2026-01-16T13:38:17Z

Product Description

This PR is the first in a series related to the retirement of CommCareSessionService. One of the options is to encrypt files using Android keystore facilities. To ensure that this change won't negatively impact CommCare performance, we decided to start measuring files encryption times to better assess this impact.

Ticket: https://dimagi.atlassian.net/browse/SAAS-19065

Original PR: #3490

Safety Assurance

Safety story

This was designed to be a null-safe operation and has no impact to the user.

Labels and Review

Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

coderabbitai · 2026-01-16T13:42:45Z

📝 Walkthrough

Walkthrough

This PR adds performance monitoring instrumentation for file encryption and storage operations. It introduces new trace name and attribute constants (TRACE_FILE_ENCRYPTION_TIME, ATTR_FILE_SIZE_BYTES, ATTR_FILE_TYPE) in the analytics module, then instruments three file-related classes (EncryptionIO, HybridFileBackedSqlStorage, SaveToDiskTask) to capture performance metrics around encryption and I/O operations, including file size and type attributes. A new XML extension constant is also exposed in FileUtil.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Add perf monitoring trace for form loading time #3333: Adds new performance-trace constants to CCPerfMonitoring and integrates Firebase tracing across file operation codepaths
Add perf monitoring trace for case list loading time #3297: Modifies performance monitoring infrastructure by adding constants to CCPerfMonitoring and introducing Firebase performance trace instrumentation
Add perf monitoring trace for app sync duration #3332: Adds new performance-tracing constants to CCPerfMonitoring and introduces Firebase trace instrumentation for file operations

Suggested labels

skip-integration-tests

Suggested reviewers

Jignesh-dimagi
shubham1g5
OrangeAndGreen

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and clearly summarizes the main change: adding performance monitoring traces for file encryption time measurement.
Description check	✅ Passed	The description includes Product Description with context and ticket link, Safety Assurance with safety story, and Labels and Review checklist, though Technical Summary section with design rationale and Automated test coverage details are missing.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch hotfix_add-file-cryptographic-operations-perf-monitoring-trace

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@app/src/org/commcare/models/encryption/EncryptionIO.java`:
- Around line 19-21: EncryptionIO currently imports java.nio.file.Files and
java.nio.file.Paths and uses Files.size(Paths.get(sourceFilePath)); replace
these with java.io.File usage to avoid API 26-only classes: remove the
java.nio.file.Files and java.nio.file.Paths imports, import java.io.File
instead, and change the size check to use new File(sourceFilePath).length()
wherever Files.size(Paths.get(...)) is used (e.g., the call in EncryptionIO that
references sourceFilePath); keep behavior identical otherwise and handle the
long return type from File.length().

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5bb211f and fe50e49.

📒 Files selected for processing (5)

app/src/org/commcare/google/services/analytics/CCPerfMonitoring.kt
app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java
app/src/org/commcare/models/encryption/EncryptionIO.java
app/src/org/commcare/tasks/SaveToDiskTask.java
app/src/org/commcare/utils/FileUtil.java

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-12-10T16:39:05.007Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3448
File: app/src/org/commcare/gis/EntityMapUtils.kt:214-234
Timestamp: 2025-12-10T16:39:05.007Z
Learning: In Kotlin files (here: app/src/org/commcare/gis/EntityMapUtils.kt), follow fail-fast guidance: treat conditions that indicate a programmer error or broken invariant as exceptions rather than defensive handling. If a condition represents a bug (e.g., internal API assumptions like array/list size mismatches), crash early to alert developers instead of attempting to recover. Use clear runtime exceptions (e.g., IllegalStateException, IllegalArgumentException) or assertions where appropriate, and avoid swallowing or masking bugs. Ensure invariants are checked and failures are surfaced with actionable messages for quick debugging.

Applied to files:

app/src/org/commcare/google/services/analytics/CCPerfMonitoring.kt

📚 Learning: 2025-05-22T14:28:35.959Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.

Applied to files:

app/src/org/commcare/tasks/SaveToDiskTask.java

🧬 Code graph analysis (3)

app/src/org/commcare/models/encryption/EncryptionIO.java (1)

app/src/org/commcare/google/services/analytics/CCPerfMonitoring.kt (2)

startTracing (29-42)

stopTracing (44-51)

app/src/org/commcare/tasks/SaveToDiskTask.java (2)

app/src/org/commcare/utils/FileUtil.java (1)

FileUtil (65-968)

app/src/org/commcare/google/services/analytics/CCPerfMonitoring.kt (2)

startTracing (29-42)

stopTracing (44-51)

app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java (3)

app/src/org/commcare/CommCareApplication.java (1)

CommCareApplication (158-1255)

app/src/org/commcare/google/services/analytics/CCPerfMonitoring.kt (2)

startTracing (29-42)

stopTracing (44-51)

app/src/org/commcare/utils/HashUtils.kt (1)

toString (13-13)

🔇 Additional comments (4)

app/src/org/commcare/utils/FileUtil.java (1)

70-70: LGTM — shared XML extension constant is clean and useful.
Line 70 looks good and improves consistency across callers.

app/src/org/commcare/google/services/analytics/CCPerfMonitoring.kt (1)

15-27: LGTM — new trace and attribute constants are clear and consistent.
Lines 15 and 26–27 are straightforward additions.

app/src/org/commcare/models/database/HybridFileBackedSqlStorage.java (1)

338-359: Nice, safe tracing integration in the file-write path.
Line 338–359 cleanly captures size/type attributes and stops tracing in a finally path.

app/src/org/commcare/tasks/SaveToDiskTask.java (1)

266-281: LGTM — tracing around XML write is consistent and low-risk.
Line 266–281 correctly tags size and type and stops the trace in the finally path.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

coderabbitai · 2026-01-16T13:42:48Z

app/src/org/commcare/models/encryption/EncryptionIO.java

+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.security.InvalidKeyException;


⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "EncryptionIO.java" -type f

Repository: dimagi/commcare-android

Length of output: 125

🏁 Script executed:

find . -name "build.gradle" -o -name "AndroidManifest.xml" | head -20

Repository: dimagi/commcare-android

Length of output: 270

🏁 Script executed:

head -n 60 ./app/src/org/commcare/models/encryption/EncryptionIO.java | cat -n

Repository: dimagi/commcare-android

Length of output: 2726

🏁 Script executed:

grep -A 10 -B 2 "minSdk" ./app/build.gradle

Repository: dimagi/commcare-android

Length of output: 484

🏁 Script executed:

grep -n "import java.io.File" ./app/src/org/commcare/models/encryption/EncryptionIO.java

Repository: dimagi/commcare-android

Length of output: 202

Replace java.nio.file APIs with java.io.File to ensure compatibility with minSdk 23.
Lines 19–20 import java.nio.file.Files and java.nio.file.Paths, which are only available from API 26. Line 51 uses Files.size(Paths.get(sourceFilePath)). Since the app's minSdkVersion is 23, this will crash at runtime on Android 6.x devices (API 23–25) with NoClassDefFoundError. Replace with new File(sourceFilePath).length().

🛠️ Suggested fix

-import java.nio.file.Files; -import java.nio.file.Paths; import java.security.InvalidKeyException;

- attrs.put(CCPerfMonitoring.ATTR_FILE_SIZE_BYTES, Long.toString(Files.size(Paths.get(sourceFilePath)))); + attrs.put(CCPerfMonitoring.ATTR_FILE_SIZE_BYTES, Long.toString(new File(sourceFilePath).length()));

🤖 Prompt for AI Agents

In `@app/src/org/commcare/models/encryption/EncryptionIO.java` around lines 19 - 21, EncryptionIO currently imports java.nio.file.Files and java.nio.file.Paths and uses Files.size(Paths.get(sourceFilePath)); replace these with java.io.File usage to avoid API 26-only classes: remove the java.nio.file.Files and java.nio.file.Paths imports, import java.io.File instead, and change the size check to use new File(sourceFilePath).length() wherever Files.size(Paths.get(...)) is used (e.g., the call in EncryptionIO that references sourceFilePath); keep behavior identical otherwise and handle the long return type from File.length().

shubham1g5 · 2026-01-16T14:23:41Z

@avazirna One other question -> Given the main goal of this is to compare performance pre-post the change to use Android keystore for encryption, it might be better to put the file size in buckets for ease of analysis. Not sure if that's easy to do on Firebase Performance UI or bigquery.

Jignesh-dimagi · 2026-01-19T04:40:35Z

app/src/org/commcare/tasks/SaveToDiskTask.java

+            try {
+                Map<String, String> attrs = new HashMap<>();
+                attrs.put(CCPerfMonitoring.ATTR_FILE_SIZE_BYTES, Long.toString(payload.getLength()));
+                attrs.put(CCPerfMonitoring.ATTR_FILE_TYPE, XML_EXTENSION);
+                CCPerfMonitoring.INSTANCE.stopTracing(trace, attrs);
+            } catch (Exception e) {
+                Logger.exception("Failed to stop tracing ", e);
+            }


@avazirna Can we abstract this function in some util as code is getting repeated?

Yes, great point

avazirna added 3 commits January 16, 2026 15:36

Add perf monitoring trace for media files encryption time

680dd7c

Add perf monitoring trace for large files encryption time

47b2830

Add perf monitoring trace for XML files encryption time

fe50e49

avazirna requested review from conroy-ricketts and shubham1g5 January 16, 2026 13:40

coderabbitai bot reviewed Jan 16, 2026

View reviewed changes

shubham1g5 approved these changes Jan 16, 2026

View reviewed changes

conroy-ricketts approved these changes Jan 16, 2026

View reviewed changes

Refactor

bf7e580

avazirna requested a review from Jignesh-dimagi January 18, 2026 22:31

Jignesh-dimagi reviewed Jan 19, 2026

View reviewed changes

Refactor

9aa8df9

avazirna requested a review from Jignesh-dimagi January 19, 2026 05:49

Jignesh-dimagi approved these changes Jan 19, 2026

View reviewed changes

avazirna merged commit 78a496f into commcare_2.61 Jan 19, 2026
1 of 2 checks passed

This was referenced Jan 19, 2026

Make trace nullable when stopping tracing #3499

Merged

Backmerge Commcare 2.61.3 #3504

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Hotfix] Add perf monitoring trace for file encryption time #3496

[Hotfix] Add perf monitoring trace for file encryption time #3496

Uh oh!

avazirna commented Jan 16, 2026

Uh oh!

coderabbitai bot commented Jan 16, 2026

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Jan 16, 2026

Uh oh!

shubham1g5 commented Jan 16, 2026

Uh oh!

Jignesh-dimagi Jan 19, 2026

Uh oh!

avazirna Jan 19, 2026

Uh oh!

avazirna Jan 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

[Hotfix] Add perf monitoring trace for file encryption time #3496

[Hotfix] Add perf monitoring trace for file encryption time #3496

Uh oh!

Conversation

avazirna commented Jan 16, 2026

Product Description

Safety Assurance

Safety story

Labels and Review

Uh oh!

coderabbitai bot commented Jan 16, 2026

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

shubham1g5 commented Jan 16, 2026

Uh oh!

Jignesh-dimagi Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

avazirna Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

avazirna Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants