feat: optimize disk buffering export frequency with configurable delay by namanONcode · Pull Request #1446 · open-telemetry/opentelemetry-android

namanONcode · 2025-12-03T19:40:39Z

This pull request optimizes the disk buffering export frequency to significantly reduce battery consumption and backend load, while providing a configurable export interval for enterprise flexibility. The default export frequency is increased from every 10 seconds to every 1 minute, and a new configuration parameter allows customization. Comprehensive test coverage ensures correctness and backward compatibility.

Export Frequency Optimization

Increased default export interval from 10 seconds to 1 minute in both PeriodicWorkImpl (DEFAULT_LOOP_INTERVAL_MILLIS) and DefaultExportScheduler (exportScheduleDelayMillis), reducing export attempts by ~83%. [1] [2] [3]
Updated all scheduling logic to use the new default and propagate the interval consistently throughout the codebase.

Configurability Enhancements

Added exportScheduleDelayMillis parameter to DiskBufferingConfig, with documentation and builder/factory support for custom values. [1] [2] [3] [4]
Integrated configuration into OpenTelemetryRumBuilder to ensure the chosen interval is respected application-wide.

Testing Improvements

Updated and added tests to verify the new default delay and support for custom export intervals in both PeriodicWorkTest and DefaultExportSchedulerTest. [1] [2]
Ensured all tests pass, validating the implementation.

Documentation and Impact Analysis

Added a detailed summary (FIX_SUMMARY.md) outlining the issue, solution, impact on battery and backend, configuration examples, and backward compatibility.

Copilot

Pull request overview

This PR optimizes the disk buffering export frequency to reduce battery consumption and backend load by changing the default export interval from 10 seconds to 1 minute, achieving an ~83% reduction in export attempts. It introduces configurability through a new exportScheduleDelayMillis parameter in DiskBufferingConfig, allowing enterprise users to customize the export frequency based on their specific requirements.

Key Changes

Increased default export interval from 10 seconds to 60 seconds in both PeriodicWorkImpl and DefaultExportScheduler
Added exportScheduleDelayMillis configuration parameter with builder support and documentation
Updated tests to validate the new default 60-second delay and support for custom intervals

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`PeriodicWorkImpl.kt`	Added configurable `loopIntervalMillis` parameter with 60-second default constant
`DefaultExportScheduler.kt`	Made export delay configurable via parameter, defaulting to 60 seconds
`DiskBufferingConfig.kt`	Added `exportScheduleDelayMillis` parameter with documentation and factory method support
`OpenTelemetryRumBuilder.kt`	Integrated configuration to pass custom delay to scheduler
`PeriodicWorkTest.kt`	Updated test constant from 10 to 60 seconds
`DefaultExportSchedulerTest.kt`	Added test for custom delay configuration and updated default delay assertion
`DefaultExportScheduleHandlerTest.kt`	Fixed parameter naming to use named parameter syntax
`FIX_SUMMARY.md`	Added comprehensive documentation of changes, impact analysis, and usage examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-03T19:44:29Z

+        /**
+         * The delay in milliseconds between consecutive export attempts. Defaults to 1 minute (60000 ms).
+         * A higher value reduces battery consumption, while a lower value provides more real-time exporting.
+         */
+        val exportScheduleDelayMillis: Long = DEFAULT_EXPORT_SCHEDULE_DELAY_MILLIS,


The PeriodicWorkImpl loop interval is hardcoded to 60 seconds, but users can configure exportScheduleDelayMillis to values shorter than 60 seconds (e.g., 30 seconds). When exportScheduleDelayMillis is shorter than the loop interval, exports may not occur at the expected frequency.

For example, if a user configures exportScheduleDelayMillis = 30 seconds, the DefaultExportScheduler expects to run every 30 seconds, but PeriodicWorkImpl only checks its queue every 60 seconds. This means exports would still occur approximately every 60 seconds, not every 30 seconds as configured.

Consider either:

Documenting this limitation in the exportScheduleDelayMillis parameter documentation

Making PeriodicWorkImpl loop interval configurable and passing the same value

Validating that exportScheduleDelayMillis is not less than the PeriodicWorkImpl loop interval

Copilot · 2025-12-03T19:44:29Z

+```kotlin
+// Set custom export frequency (e.g., 30 seconds)
+val config = DiskBufferingConfig.create(
+    enabled = true,
+    exportScheduleDelayMillis = TimeUnit.SECONDS.toMillis(30)


The documentation states "Set custom export frequency (e.g., 30 seconds)" but does not mention the limitation that PeriodicWorkImpl has a fixed 60-second loop interval. If a user configures 30 seconds as shown in this example, the actual export frequency will still be approximately 60 seconds, not 30 seconds.

This documentation should either:

Note the minimum effective value (60 seconds due to PeriodicWorkImpl's loop interval)

Include a warning that values less than 60 seconds may not work as expected

Update the example to use values >= 60 seconds

Suggested change

```kotlin

// Set custom export frequency (e.g., 30 seconds)

val config = DiskBufferingConfig.create(

enabled = true,

exportScheduleDelayMillis = TimeUnit.SECONDS.toMillis(30)

> **Note:** The minimum effective export frequency is **60 seconds** due to the fixed loop interval in `PeriodicWorkImpl`. Values less than 60 seconds may not work as expected.

```kotlin

// Set custom export frequency (e.g., 60 seconds)

val config = DiskBufferingConfig.create(

enabled = true,

exportScheduleDelayMillis = TimeUnit.SECONDS.toMillis(60)

Copilot · 2025-12-03T19:44:29Z

+  - Reduce to 30 seconds for critical real-time monitoring
+  - Increase to 5 minutes for battery-constrained environments
+  - Set to any custom value matching their requirements


The documentation states "Reduce to 30 seconds for critical real-time monitoring" but this is misleading because the PeriodicWorkImpl loop interval is fixed at 60 seconds. Configuring a 30-second export delay will not result in exports every 30 seconds; they will still occur approximately every 60 seconds.

Update this to reflect realistic configuration options that respect the 60-second minimum imposed by the PeriodicWorkImpl loop interval.

Suggested change

- Reduce to 30 seconds for critical real-time monitoring

- Increase to 5 minutes for battery-constrained environments

- Set to any custom value matching their requirements

- Reduce to 1 minute (minimum) for critical real-time monitoring (due to the 60-second loop interval)

- Increase to 5 minutes for battery-constrained environments

- Set to any custom value matching their requirements (minimum 1 minute enforced by implementation)

…e performance and reduce battery drain

namanONcode · 2025-12-03T20:40:47Z

@breedx-splk here also i remove ai generated documentation please review and merge , new contributor i m learning

codecov · 2025-12-03T20:52:44Z

Codecov Report

❌ Patch coverage is 87.00000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.31%. Comparing base (8f82e21) to head (cb82493).
⚠️ Report is 170 commits behind head on main.

Files with missing lines	Patch %	Lines
...o/opentelemetry/android/OpenTelemetryRumBuilder.kt	40.00%	5 Missing and 1 partial ⚠️
...roid/features/diskbuffering/DiskBufferingConfig.kt	76.47%	4 Missing ⚠️
...kbuffering/scheduler/ExportScheduleAutoDetector.kt	94.64%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1446      +/-   ##
==========================================
+ Coverage   63.70%   64.31%   +0.60%     
==========================================
  Files         159      160       +1     
  Lines        3149     3242      +93     
  Branches      325      342      +17     
==========================================
+ Hits         2006     2085      +79     
- Misses       1045     1058      +13     
- Partials       98       99       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

breedx-splk · 2025-12-03T20:58:32Z

+         *
+         * Example impact: A 10-second export interval means ~2880 export attempts per 8-hour day.
+         * A 60-second interval reduces this to ~480 attempts. A 5-minute interval reduces it to ~96 attempts.
+         */


I think this comment is too long.

breedx-splk

While I like the idea of allowing users to more easily configure their export frequency, there seems to be an unsubstantiated confidence in this PR that 1 minute is more optimized/efficient than 10 seconds. This is not necessarily true, and the tradeoffs need to be carefully considered before jumping in to a change in defaults like this.

In an application that generates high amounts of telemetry, the volume of data required to be sent in a given export cycle has now been increased 600%. That doesn't necessarily seem more efficient to me. Furthermore, this increases the likelihood of data being stuck on device, rather than being exported...and requires RUM product users to wait longer before seeing an updated session.

My suggestion to move this forward is to separate out the configurability from the change in default behavior. In other words, propose a change that makes it easier to configure the period, but don't change the default period. If/when that lands, feel free to follow up with a proposed change to the defaults -- but be sure to substantiate it and support it with data.

… data freshness

namanONcode · 2025-12-03T21:26:12Z

@breedx-splk i am adding a auto detection with default 10 sec typically based on device characteristics like:
Device memory
Battery level
Network connectivity
CPU load

…device conditions

…ity in tests

…ortScheduleAutoDetector and related tests

…nsive battery and memory checks improved coverage

…eduleAutoDetectorTest

…r improved readability

namanONcode · 2025-12-04T03:28:01Z

@breedx-splk i have done implementing auto detector with all coverage test and also its default is 10 sec and there are multiple test cases with different real condition in testing time can be adjusted according 1 min two min etc .

atulgpt · 2025-12-07T18:50:42Z

 const val DEFAULT_MAX_FILE_AGE_FOR_WRITE_MS = 30L
 const val DEFAULT_MIN_FILE_AGE_FOR_READ_MS = 33L
 const val DEFAULT_MAX_FILE_AGE_FOR_READ_MS = 18L
+const val DEFAULT_EXPORT_SCHEDULE_DELAY_MILLIS: Long = 10000L // 10 seconds


Suggested change

const val DEFAULT_EXPORT_SCHEDULE_DELAY_MILLIS: Long = 10000L // 10 seconds

const val DEFAULT_EXPORT_SCHEDULE_DELAY_MS: Long = 10000L

Following same convention

atulgpt · 2025-12-07T18:51:57Z


 class DefaultExportScheduler(
    periodicWorkProvider: () -> PeriodicWork,
+    private val exportScheduleDelayMillis: Long = TimeUnit.SECONDS.toMillis(10),


@breedx-splk do we use @JVMOverloads constructor to have nicer api and good backward compatibility support in case of default param? If yes we should add the same here

It's only a "nice-to-have" if we expect java users to be calling it.

atulgpt · 2025-12-07T18:54:48Z

+    private const val DEFAULT_EXPORT_INTERVAL_MILLIS = 10000L // 10 seconds
+    private const val BATTERY_SAVER_INTERVAL_MILLIS = 30000L // 30 seconds
+    private const val LOW_MEMORY_INTERVAL_MILLIS = 20000L // 20 seconds


Suggested change

private const val DEFAULT_EXPORT_INTERVAL_MILLIS = 10000L // 10 seconds

private const val BATTERY_SAVER_INTERVAL_MILLIS = 30000L // 30 seconds

private const val LOW_MEMORY_INTERVAL_MILLIS = 20000L // 20 seconds

private const val DEFAULT_EXPORT_INTERVAL_MILLIS = 10000L

private const val BATTERY_SAVER_INTERVAL_MILLIS = 30000L

private const val LOW_MEMORY_INTERVAL_MILLIS = 20000L

this should not be neccessart // 10 seconds as 10000L itsefl implies the same thing

...or if you think 10000L is hard to read, use a constant like 10.seconds.inWholeMilliseconds

the constant suffixes _MILLIS vs _MS are also inconsistent with the above file.

atulgpt · 2025-12-07T19:01:45Z

+    internal fun checkBatteryStatus(context: Context): Long {
+        return try {
+            val batteryIntent = context.registerReceiver(null, IntentFilter(Intent.ACTION_BATTERY_CHANGED))
+            if (batteryIntent != null) {


this if can be used as expression like below

return if {} else {} // rest your code

atulgpt · 2025-12-07T19:02:42Z

+                val status = batteryIntent.getIntExtra(BatteryManager.EXTRA_STATUS, -1)
+
+                val isCharging = plugged != 0
+                val isBatteryLow = status == BatteryManager.BATTERY_STATUS_UNKNOWN || level < 0


Suggested change

val isBatteryLow = status == BatteryManager.BATTERY_STATUS_UNKNOWN || level < 0

val isBatteryStatusUnknown = status == BatteryManager.BATTERY_STATUS_UNKNOWN || level < 0

atulgpt · 2025-12-07T19:11:05Z

+ * Integration tests for ExportScheduleAutoDetector with DiskBufferingConfig.
+ * Tests the complete feature workflow and all integration points.
+ */
+class ExportScheduleAutoDetectorIntegrationTest {


Suggested change

class ExportScheduleAutoDetectorIntegrationTest {

class ExportScheduleAutoDetectorTest {

atulgpt · 2025-12-07T19:11:50Z

+    // ============================================================================
+    // INTEGRATION TEST SUITE 1: Configuration Integration
+    // ============================================================================


You can use nested class if you want to group tests

atulgpt · 2025-12-07T19:12:48Z

+    @Test
+    fun `integration - DiskBufferingConfig with auto-detection disabled uses fixed interval`() {
+        val config =
+            io.opentelemetry.android.features.diskbuffering.DiskBufferingConfig(


add import for DiskBufferingConfig

atulgpt · 2025-12-07T19:13:28Z

+                exportScheduleDelayMillis = TimeUnit.SECONDS.toMillis(10),
+            )
+
+        assertEquals(false, config.autoDetectExportSchedule)


can we use assertJ

atulgpt · 2025-12-07T19:14:04Z

+    fun `integration - DiskBufferingConfig with auto-detection enabled uses detection`() {
+        val config =
+            io.opentelemetry.android.features.diskbuffering.DiskBufferingConfig.create(
+                enabled = true,
+                autoDetectExportSchedule = true,
+                exportScheduleDelayMillis = TimeUnit.SECONDS.toMillis(10),
+            )
+
+        assertEquals(true, config.autoDetectExportSchedule)
+        assertEquals(TimeUnit.SECONDS.toMillis(10), config.exportScheduleDelayMillis)
+    }


we can use paramaterised test

atulgpt · 2025-12-07T19:15:16Z

Hi @namanONcode can you review the autogenerated code if anything can be done to improve those?

namanONcode · 2025-12-07T19:18:01Z

Yeah I will definitely check these suggestions> Hi @namanONcode can you review the autogenerated code if anything can be done to improve those?

…clarity

…t for cleaner code

namanONcode · 2025-12-09T18:20:43Z

@atulgpt @breedx-splk @fractalwrench i have done implementing all valid suggestion that is tested and coveraged

Naming consistency: DEFAULT_EXPORT_SCHEDULE_DELAY_MILLIS → DEFAULT_EXPORT_SCHEDULE_DELAY_MS
Remove // 10 seconds comments near numeric constants
Use return if
Integration test class name should not include “IntegrationTest”
Use nested test classes for grouping
Import DiskBufferingConfig instead of using fully-qualified name

Now give a final review for this implementation anything else

atulgpt · 2025-12-11T15:18:27Z

Hi @namanONcode either you missed few places, or didn't pushed your entire changes. Few comments are still open

namanONcode · 2025-12-11T15:43:58Z

@atulgpt give me some time i will fix this is there any changes more i will make that also> Hi @namanONcode either you missed few places, or didn't pushed your entire changes. Few comments are still open

…d auto-detection via DSL.

…rt schedule.

…xport schedule auto-detection tests with additional coverage.

namanONcode · 2025-12-16T17:29:28Z

@atulgpt @breedx-splk @fractalwrench please review i have fixed the problem

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-16T17:30:41Z

+
+    @Test
+    fun `default loop interval constant is correct`() {
+        assertThat(PeriodicWork.DEFAULT_LOOP_INTERVAL_MS).isEqualTo(10000L)


Test expects DEFAULT_LOOP_INTERVAL_MS to be 10000L (10 seconds), but based on the PR description and context, the default should be updated to 60000L (60 seconds) to match the new default export interval.

Suggested change

assertThat(PeriodicWork.DEFAULT_LOOP_INTERVAL_MS).isEqualTo(10000L)

assertThat(PeriodicWork.DEFAULT_LOOP_INTERVAL_MS).isEqualTo(60000L)

Copilot · 2025-12-16T17:30:41Z

+         * queue is checked for pending tasks. This interval controls the granularity of task
+         * scheduling and should be set based on the specific use case and performance requirements.
+         */
+        const val DEFAULT_LOOP_INTERVAL_MS: Long = 10000L


The DEFAULT_LOOP_INTERVAL_MS is set to 10000L (10 seconds), but according to the PR description, the default export interval should be increased to 60000L (60 seconds) to match the optimization goals.

Suggested change

const val DEFAULT_LOOP_INTERVAL_MS: Long = 10000L

const val DEFAULT_LOOP_INTERVAL_MS: Long = 60000L

Copilot · 2025-12-16T17:30:42Z

+                    try {
+                        Log.w(OTEL_RUM_LOG_TAG, "minFileAgeForReadMillis must be greater than maxFileAgeForWriteMillis")
+                        Log.w(
+                            OTEL_RUM_LOG_TAG,
+                            "overriding minFileAgeForReadMillis from $minFileAgeForReadMillis to $minRead",
+                        )
+                    } catch (e: RuntimeException) {
+                        // Keep going, this is just a warning, and we might be running in a unit test
+                    }


Wrapping logging statements in a try-catch for RuntimeException is unusual and may hide legitimate issues. If this is specifically for unit test environments where logging might fail, consider a more explicit check or documentation explaining why this pattern is necessary.

github-actions · 2026-03-19T17:18:15Z

This has been automatically marked as stale because it has been marked as needing author feedback and has not had any activity for 21 days. It will be closed automatically if there is no response from the author within 14 additional days from this comment.

github-actions · 2026-04-13T01:08:33Z

This has been automatically marked as stale because it has been marked as needing author feedback and has not had any activity for 21 days. It will be closed automatically if there is no response from the author within 14 additional days from this comment.

namanONcode · 2026-04-13T06:15:49Z

@breedx-splk is there any one to review this pr ?

feat: optimize disk buffering export frequency with configurable delay

7bfcc18

Copilot AI review requested due to automatic review settings December 3, 2025 19:40

namanONcode requested a review from a team as a code owner December 3, 2025 19:40

Copilot started reviewing on behalf of namanONcode December 3, 2025 19:41 View session

Copilot finished reviewing on behalf of namanONcode December 3, 2025 19:43

Copilot AI reviewed Dec 3, 2025

View reviewed changes

breedx-splk added the now-with-even-more-AI-than-ever label Dec 3, 2025

namanONcode added 2 commits December 4, 2025 01:57

feat: add configurable export frequency for disk buffering to optimiz…

b853d1a

…e performance and reduce battery drain

removed ai generated documentation

7b11fe0

breedx-splk reviewed Dec 3, 2025

View reviewed changes

breedx-splk requested changes Dec 3, 2025

View reviewed changes

feat: reduce default export schedule delay to 10 seconds for improved…

b58a737

… data freshness

namanONcode added 6 commits December 4, 2025 03:28

feat: implement auto-detection for optimal export frequency based on …

00e810b

…device conditions

refactor: replace for loop with repeat function for improved readabil…

c7fa8b4

…ity in tests

refactor: improve code readability by standardizing formatting in Exp…

ce13cf3

…ortScheduleAutoDetector and related tests

test: enhance unit tests for ExportScheduleAutoDetector with comprehe…

c32a8ca

…nsive battery and memory checks improved coverage

refactor: remove unused imports and improve code clarity in ExportSch…

c991106

…eduleAutoDetectorTest

refactor: standardize formatting in ExportScheduleAutoDetectorTest fo…

693b7f0

…r improved readability

namanONcode requested a review from breedx-splk December 4, 2025 03:11

atulgpt reviewed Dec 7, 2025

View reviewed changes

namanONcode added 2 commits December 9, 2025 23:04

refactor: rename export schedule delay constants for consistency and …

efd0b03

…clarity

refactor: remove trailing whitespace in ExportScheduleAutoDetectorTes…

46499d3

…t for cleaner code

namanONcode added 3 commits December 16, 2025 17:38

feat: enable configuration of disk buffering export schedule delay an…

6197112

…d auto-detection via DSL.

feat: Add auto-detect and delay configuration for disk buffering expo…

5a7756c

…rt schedule.

feat: externalize memory info retrieval for testability and enhance e…

cb82493

…xport schedule auto-detection tests with additional coverage.

namanONcode requested review from atulgpt and Copilot December 16, 2025 17:29

Copilot AI reviewed Dec 16, 2025

View reviewed changes

$@fractalwrench$ fractalwrench added the needs author feedback Waiting for additional feedback from the author label Feb 26, 2026

github-actions Bot added stale and removed stale labels Mar 19, 2026

github-actions Bot added the stale label Apr 13, 2026

github-actions Bot removed the stale label Apr 13, 2026

	const val DEFAULT_EXPORT_SCHEDULE_DELAY_MILLIS: Long = 10000L // 10 seconds
	const val DEFAULT_EXPORT_SCHEDULE_DELAY_MS: Long = 10000L

	val isBatteryLow = status == BatteryManager.BATTERY_STATUS_UNKNOWN \|\| level < 0
	val isBatteryStatusUnknown = status == BatteryManager.BATTERY_STATUS_UNKNOWN \|\| level < 0

	class ExportScheduleAutoDetectorIntegrationTest {
	class ExportScheduleAutoDetectorTest {

	assertThat(PeriodicWork.DEFAULT_LOOP_INTERVAL_MS).isEqualTo(10000L)
	assertThat(PeriodicWork.DEFAULT_LOOP_INTERVAL_MS).isEqualTo(60000L)

	const val DEFAULT_LOOP_INTERVAL_MS: Long = 10000L
	const val DEFAULT_LOOP_INTERVAL_MS: Long = 60000L

Conversation

namanONcode commented Dec 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

codecov Bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

breedx-splk left a comment

Choose a reason for hiding this comment

Uh oh!

namanONcode commented Dec 3, 2025

Uh oh!

namanONcode commented Dec 4, 2025

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atulgpt commented Dec 7, 2025

Uh oh!

namanONcode commented Dec 7, 2025

Uh oh!

namanONcode commented Dec 9, 2025

Uh oh!

atulgpt commented Dec 11, 2025

Uh oh!

namanONcode commented Dec 11, 2025

Uh oh!

namanONcode commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

namanONcode commented Dec 3, 2025 •

edited

Loading

codecov Bot commented Dec 3, 2025 •

edited

Loading