Allow each signal API to be thoroughly disabled by bidetofevil · Pull Request #1785 · open-telemetry/opentelemetry-android

bidetofevil · 2026-06-03T05:37:40Z

Add the ability to disable each signal API via the config DSL that provides a no-op provider instead of just not configuring any processors. This eliminates the overhead of instrumentation that are still configured but whose telemetry doesn't ever get recorded and sent.

codecov · 2026-06-03T05:50:22Z

Codecov Report

❌ Patch coverage is 97.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.36%. Comparing base (37ae421) to head (ca726ef).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...o/opentelemetry/android/OpenTelemetryRumBuilder.kt	94.87%	0 Missing and 2 partials ⚠️
...metry/android/agent/OpenTelemetryRumInitializer.kt	95.23%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1785      +/-   ##
==========================================
+ Coverage   62.99%   63.36%   +0.37%     
==========================================
  Files         157      159       +2     
  Lines        3448     3502      +54     
  Branches      352      370      +18     
==========================================
+ Hits         2172     2219      +47     
  Misses       1179     1179              
- Partials       97      104       +7

☔ View full report in Codecov by Harness.
📢 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.

LikeTheSalad · 2026-06-03T08:25:34Z

+    /**
+     * Whether the tracing API is enabled. Toggle with using [setTracingEnabled]
+     */
+    @Volatile


Is there a need to make these volatile?

I suppose its creation and consumption are all done on the main thread, and this object is thrown away after the SDK is initialized, so I can remove this if you feel it makes things slightly more convoluted.

Yeah. I don't see a reason why making it thread-safe, at least not now. Plus, if it were needed, then I think we should apply the same safety guards to the rest of the config options. But for now I think it's better to keep it simple.

breedx-splk · 2026-06-09T15:25:20Z

Related to #1776

breedx-splk · 2026-06-09T22:00:22Z

+    fun disableTracing(disable: Boolean) {
+        rumConfig.setTracingEnabled(!disable)
+    }


I think these would benefit from not allowing the inverse logic, especially when they are enabled by default.

Suggested change

fun disableTracing(disable: Boolean) {

rumConfig.setTracingEnabled(!disable)

}

fun disableTracing() {

rumConfig.setTracingEnabled(false)

}

breedx-splk · 2026-06-09T22:00:40Z

+    fun disableLogging(disable: Boolean) {
+        rumConfig.setLoggingEnabled(!disable)
+    }


Suggested change

fun disableLogging(disable: Boolean) {

rumConfig.setLoggingEnabled(!disable)

}

fun disableLogging() {

rumConfig.setLoggingEnabled(false)

}

breedx-splk · 2026-06-09T22:00:55Z

+    fun disableMetrics(disable: Boolean) {
+        rumConfig.setMetricsEnabled(!disable)
+    }


Suggested change

fun disableMetrics(disable: Boolean) {

rumConfig.setMetricsEnabled(!disable)

}

fun disableMetrics() {

rumConfig.setMetricsEnabled(false)

}

breedx-splk · 2026-06-09T22:15:28Z

+    }
+
+    /**
+     * Disable logging in the SDK by providing no-op implementations that don't incur overhead even if instrumentation emits logs


Maybe we also mention that this silences events too?

breedx-splk · 2026-06-09T22:33:41Z

+            if (rumConfig.tracingEnabled) {
+                addSpanExporterCustomizer {
+                    createSpanExporter(cfg.exportConfig.spansEndpoint())
+                }
+            }
+            if (rumConfig.loggingEnabled) {
+                addLogRecordExporterCustomizer {
+                    createLogExporter(cfg.exportConfig.logsEndpoint())
+                }
+            }
+            if (rumConfig.metricsEnabled) {
+                addMetricExporterCustomizer {
+                    createMetricExporter(cfg.exportConfig.metricsEndpoint())
+                }
+            }


One interesting edge case of this is that if (for some reason) telemetry data was left on disk, and, after an upgrade, one of the signals was no longer turned on, we wouldn't have an exporter to handle that data on disk. Presumably we would read it and export it to a noop, effectively losing it.

I think I'm fine with that, but just wanted to make a note of it in case it hadn't been considered.

breedx-splk · 2026-06-09T22:40:03Z

+        )
+
+        server.awaitLogRequest { findLog(it) }
+        assertEquals(0, server.traceRequestCount())


Prefer fluent assertj assertions.

Suggested change

assertEquals(0, server.traceRequestCount())

assertThat(server.traceRequestCount()).isZero()

breedx-splk · 2026-06-09T22:50:23Z

-            }
-        assertTraceRequestReceived(traceRequest)
+        server.awaitTraceRequest { findSpan(it) }
+        assertEquals(0, server.logRequestCount())


Suggested change

assertEquals(0, server.logRequestCount())

assertThat(server.logRequestCount()).isZero()

breedx-splk · 2026-06-09T22:51:49Z

+    fun setTracingEnabled(enabled: Boolean): OtelRumConfig {
+        tracingEnabled = enabled
+        return this
+    }
+
+    /**
+     * Enables or disables the logging API for the SDK. When disabled, the SDK will provide no-op implementations to instrumentation
+     * that use this API.
+     */
+    fun setLoggingEnabled(enabled: Boolean): OtelRumConfig {
+        loggingEnabled = enabled
+        return this
+    }
+
+    /**
+     * Enables or disables the metrics API for the SDK. When disabled, the SDK will provide no-op implementations to instrumentation
+     * that use this API.
+     */
+    fun setMetricsEnabled(enabled: Boolean): OtelRumConfig {
+        metricsEnabled = enabled
+        return this
+    }


I think my preference would be to have these as disable methods with no arg, since the default is enabled.

breedx-splk · 2026-06-09T23:01:01Z

-    private val openTelemetrySdk: OpenTelemetrySdk,
+    override val openTelemetry: OpenTelemetry,


breedx-splk

Left some comments, but it's looking mostly good to me! Thanks for picking this up.

Allow each signal API to be thoroughly disabled

cc14042

bidetofevil requested a review from a team as a code owner June 3, 2026 05:37

bidetofevil mentioned this pull request Jun 3, 2026

Removing unwanted exporters #1776

Open

LikeTheSalad reviewed Jun 3, 2026

View reviewed changes

Clarify lack of threadsafety of OtelRumConfig

ca726ef

breedx-splk added this to the 1.5.0 milestone Jun 9, 2026

breedx-splk reviewed Jun 9, 2026

View reviewed changes

	assertEquals(0, server.traceRequestCount())
	assertThat(server.traceRequestCount()).isZero()

	assertEquals(0, server.logRequestCount())
	assertThat(server.logRequestCount()).isZero()

		private val openTelemetrySdk: OpenTelemetrySdk,
		override val openTelemetry: OpenTelemetry,

Conversation

bidetofevil commented Jun 3, 2026

Uh oh!

codecov Bot commented Jun 3, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

breedx-splk commented Jun 9, 2026

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!

breedx-splk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jun 3, 2026 •

edited

Loading