Skip to content

Commit f4937de

Browse files
abdulraqeeb33AR Abdul Azeez
abdulraqeeb33
and
AR Abdul Azeez
committed
fix: Unify OTEL exporter diagnostics toggle and align headers (#2571)
Co-authored-by: AR Abdul Azeez <abdul@onesignal.com>
1 parent 58a786a commit f4937de

6 files changed

Lines changed: 118 additions & 47 deletions

File tree

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/otel/android/OtelPlatformProvider.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import com.onesignal.core.internal.http.OneSignalService
99
import com.onesignal.debug.internal.logging.Logging
1010
import com.onesignal.otel.IOtelPlatformProvider
1111

12+
// Use this to enable/disable the Otel exporter logging in debug builds.
13+
internal const val OTEL_EXPORTER_LOGGING_ENABLED = false
14+
1215
/**
1316
* Configuration for AndroidOtelPlatformProvider.
1417
*/
@@ -135,6 +138,8 @@ internal class OtelPlatformProvider(
135138
}
136139
}
137140

141+
override val isOtelExporterLoggingEnabled: Boolean = OTEL_EXPORTER_LOGGING_ENABLED
142+
138143
override val appIdForHeaders: String
139144
get() = appId ?: ""
140145

‎OneSignalSDK/onesignal/otel/src/main/java/com/onesignal/otel/IOtelPlatformProvider.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ interface IOtelPlatformProvider {
5353
* Valid values: "NONE", "FATAL", "ERROR", "WARN", "INFO", "DEBUG", "VERBOSE"
5454
*/
5555
val remoteLogLevel: String?
56+
57+
/**
58+
* Debug-only toggle for local exporter diagnostics.
59+
* When true, Otel exporter request/response success/failure logs are emitted to logcat.
60+
*/
61+
val isOtelExporterLoggingEnabled: Boolean
62+
5663
val appIdForHeaders: String
5764

5865
/**

‎OneSignalSDK/onesignal/otel/src/main/java/com/onesignal/otel/OneSignalOpenTelemetry.kt‎

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,6 @@ internal fun LogRecordBuilder.setAllAttributes(attributes: Map<String, String>):
1717
return this
1818
}
1919

20-
/**
21-
* Extension function to set all attributes from an Attributes object.
22-
* Made public so it can be used from other modules (e.g., core module for logging).
23-
*/
24-
fun LogRecordBuilder.setAllAttributes(attributes: io.opentelemetry.api.common.Attributes): LogRecordBuilder {
25-
attributes.forEach { key, value ->
26-
val keyString = key.key
27-
when (value) {
28-
is String -> this.setAttribute(keyString, value)
29-
is Long -> this.setAttribute(keyString, value)
30-
is Double -> this.setAttribute(keyString, value)
31-
is Boolean -> this.setAttribute(keyString, value)
32-
else -> this.setAttribute(keyString, value.toString())
33-
}
34-
}
35-
return this
36-
}
37-
3820
internal abstract class OneSignalOpenTelemetryBase(
3921
private val osTopLevelFields: OtelFieldsTopLevel,
4022
private val osPerEventFields: OtelFieldsPerEvent,
@@ -103,15 +85,19 @@ internal class OneSignalOpenTelemetryRemote(
10385

10486
val extraHttpHeaders: Map<String, String> by lazy {
10587
mapOf(
106-
"X-OneSignal-App-Id" to appId,
107-
"X-OneSignal-SDK-Version" to platformProvider.sdkBaseVersion,
88+
"SDK-Version" to "onesignal/${platformProvider.sdkBase}/${platformProvider.sdkBaseVersion}",
10889
)
10990
}
11091

11192
private val apiBaseUrl: String get() = platformProvider.apiBaseUrl
11293

11394
override val logExporter by lazy {
114-
OtelConfigRemoteOneSignal.HttpRecordBatchExporter.create(extraHttpHeaders, appId, apiBaseUrl)
95+
OtelConfigRemoteOneSignal.HttpRecordBatchExporter.create(
96+
extraHttpHeaders,
97+
appId,
98+
apiBaseUrl,
99+
platformProvider.isOtelExporterLoggingEnabled,
100+
)
115101
}
116102

117103
override fun getSdkInstance(attributes: Map<String, String>): OpenTelemetrySdk =
@@ -123,6 +109,7 @@ internal class OneSignalOpenTelemetryRemote(
123109
extraHttpHeaders,
124110
appId,
125111
apiBaseUrl,
112+
platformProvider.isOtelExporterLoggingEnabled,
126113
)
127114
).build()
128115
}

‎OneSignalSDK/onesignal/otel/src/main/java/com/onesignal/otel/config/OtelConfigRemoteOneSignal.kt‎

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package com.onesignal.otel.config
22

3+
import android.util.Log
34
import io.opentelemetry.exporter.otlp.http.logs.OtlpHttpLogRecordExporter
5+
import io.opentelemetry.sdk.common.CompletableResultCode
46
import io.opentelemetry.sdk.logs.SdkLoggerProvider
7+
import io.opentelemetry.sdk.logs.data.LogRecordData
58
import io.opentelemetry.sdk.logs.export.LogRecordExporter
69
import java.time.Duration
710

@@ -35,23 +38,97 @@ internal class OtelConfigRemoteOneSignal {
3538
extraHttpHeaders: Map<String, String>,
3639
appId: String,
3740
apiBaseUrl: String,
41+
enableExporterLogging: Boolean
3842
): SdkLoggerProvider =
3943
SdkLoggerProvider
4044
.builder()
4145
.setResource(resource)
4246
.addLogRecordProcessor(
4347
OtelConfigShared.LogRecordProcessorConfig.batchLogRecordProcessor(
44-
HttpRecordBatchExporter.create(extraHttpHeaders, appId, apiBaseUrl)
48+
HttpRecordBatchExporter.create(
49+
extraHttpHeaders,
50+
appId,
51+
apiBaseUrl,
52+
enableExporterLogging,
53+
)
4554
)
4655
).setLogLimits(OtelConfigShared.LogLimitsConfig::logLimits)
4756
.build()
4857
}
4958

5059
object HttpRecordBatchExporter {
51-
fun create(extraHttpHeaders: Map<String, String>, appId: String, apiBaseUrl: String) =
52-
LogRecordExporterConfig.otlpHttpLogRecordExporter(
53-
extraHttpHeaders,
54-
buildEndpoint(apiBaseUrl, appId)
55-
)
60+
fun create(
61+
extraHttpHeaders: Map<String, String>,
62+
appId: String,
63+
apiBaseUrl: String,
64+
enableExporterLogging: Boolean,
65+
): LogRecordExporter {
66+
val exporter =
67+
LogRecordExporterConfig.otlpHttpLogRecordExporter(
68+
extraHttpHeaders,
69+
buildEndpoint(apiBaseUrl, appId)
70+
)
71+
72+
return if (enableExporterLogging) {
73+
ExporterLoggingConfig.loggingExporter(exporter)
74+
} else {
75+
exporter
76+
}
77+
}
78+
}
79+
80+
object ExporterLoggingConfig {
81+
private const val TAG = "OneSignalOtel"
82+
83+
fun loggingExporter(delegate: LogRecordExporter): LogRecordExporter = LoggingLogRecordExporter(delegate)
84+
85+
private class LoggingLogRecordExporter(
86+
private val delegate: LogRecordExporter
87+
) : LogRecordExporter {
88+
@Suppress("TooGenericExceptionCaught")
89+
private fun resolveHttpFailureMessage(throwable: Throwable?): String {
90+
if (throwable == null) return "unknown"
91+
92+
return try {
93+
if (!throwable.javaClass.name.endsWith("FailedExportException\$HttpExportException")) {
94+
return throwable.message ?: "unknown"
95+
}
96+
97+
val response = throwable.javaClass.getMethod("getResponse").invoke(throwable) ?: return throwable.message ?: "unknown"
98+
val statusCode = response.javaClass.getMethod("statusCode").invoke(response)
99+
val statusMessage = response.javaClass.getMethod("statusMessage").invoke(response)
100+
val responseBodyBytes = response.javaClass.getMethod("responseBody").invoke(response) as? ByteArray
101+
val responseBody = responseBodyBytes?.decodeToString()
102+
103+
"status=$statusCode message=$statusMessage" +
104+
(if (responseBody.isNullOrBlank()) "" else " body=$responseBody")
105+
} catch (_: Throwable) {
106+
throwable.message ?: "unknown"
107+
}
108+
}
109+
110+
override fun export(logs: Collection<LogRecordData>): CompletableResultCode {
111+
Log.d(TAG, "OTEL export request sent to backend. count=${logs.size}")
112+
val result = delegate.export(logs)
113+
result.whenComplete {
114+
if (result.isSuccess) {
115+
Log.d(TAG, "OTEL export response received: success")
116+
} else {
117+
val throwable = result.failureThrowable
118+
val failureMessage = resolveHttpFailureMessage(throwable)
119+
Log.e(
120+
TAG,
121+
"OTEL export response received: failed - $failureMessage",
122+
throwable
123+
)
124+
}
125+
}
126+
return result
127+
}
128+
129+
override fun flush(): CompletableResultCode = delegate.flush()
130+
131+
override fun shutdown(): CompletableResultCode = delegate.shutdown()
132+
}
56133
}
57134
}

‎OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/OneSignalOpenTelemetryTest.kt‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package com.onesignal.otel
22

33
import io.kotest.core.spec.style.FunSpec
4+
import io.kotest.matchers.maps.shouldContainKey
5+
import io.kotest.matchers.maps.shouldNotContainKey
6+
import io.kotest.matchers.shouldBe
47
import io.kotest.matchers.shouldNotBe
58
import io.kotest.matchers.types.shouldBeInstanceOf
69
import io.mockk.clearMocks
710
import io.mockk.coEvery
811
import io.mockk.every
912
import io.mockk.mockk
10-
import io.opentelemetry.api.common.Attributes
1113
import io.opentelemetry.api.logs.LogRecordBuilder
1214
import kotlinx.coroutines.runBlocking
1315

@@ -77,6 +79,15 @@ class OneSignalOpenTelemetryTest : FunSpec({
7779
}
7880
}
7981

82+
test("remote telemetry should only send SDK-Version header and not legacy OneSignal SDK header") {
83+
val remoteTelemetry = OtelFactory.createRemoteTelemetry(mockPlatformProvider) as OneSignalOpenTelemetryRemote
84+
val headers = remoteTelemetry.extraHttpHeaders
85+
86+
headers.shouldContainKey("SDK-Version")
87+
headers["SDK-Version"] shouldBe "onesignal/android/5.0.0"
88+
headers.shouldNotContainKey("X-OneSignal-SDK-Version")
89+
}
90+
8091
// ===== Crash Local Telemetry Tests =====
8192

8293
test("createCrashLocalTelemetry should return IOtelOpenTelemetryCrash") {
@@ -126,23 +137,6 @@ class OneSignalOpenTelemetryTest : FunSpec({
126137
io.mockk.verify { mockBuilder.setAttribute("key2", "value2") }
127138
}
128139

129-
test("setAllAttributes with Attributes should handle different types") {
130-
val mockBuilder = mockk<LogRecordBuilder>(relaxed = true)
131-
val attributes = Attributes.builder()
132-
.put("string.key", "string-value")
133-
.put("long.key", 123L)
134-
.put("double.key", 45.67)
135-
.put("boolean.key", true)
136-
.build()
137-
138-
mockBuilder.setAllAttributes(attributes)
139-
140-
io.mockk.verify { mockBuilder.setAttribute("string.key", "string-value") }
141-
io.mockk.verify { mockBuilder.setAttribute("long.key", 123L) }
142-
io.mockk.verify { mockBuilder.setAttribute("double.key", 45.67) }
143-
io.mockk.verify { mockBuilder.setAttribute("boolean.key", true) }
144-
}
145-
146140
// ===== SDK Caching Tests =====
147141

148142
test("remote telemetry should cache SDK instance") {

‎OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/config/OtelConfigTest.kt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,15 @@ class OtelConfigTest : FunSpec({
5959

6060
test("buildEndpoint should construct correct URL from base and appId") {
6161
val endpoint = OtelConfigRemoteOneSignal.buildEndpoint("https://api.onesignal.com", "my-app")
62-
endpoint shouldBe "https://api.onesignal.com/sdk/otel/v1/logs?app_id=my-app"
62+
endpoint shouldBe "https://api.onesignal.com/sdk/log?app_id=my-app"
6363
}
6464

6565
test("HttpRecordBatchExporter should create exporter with correct endpoint") {
6666
val headers = mapOf("X-Test-Header" to "test-value")
6767
val appId = "test-app-id"
6868
val apiBaseUrl = "https://api.onesignal.com"
6969

70-
val exporter = OtelConfigRemoteOneSignal.HttpRecordBatchExporter.create(headers, appId, apiBaseUrl)
70+
val exporter = OtelConfigRemoteOneSignal.HttpRecordBatchExporter.create(headers, appId, apiBaseUrl, false)
7171

7272
exporter shouldNotBe null
7373
}
@@ -93,6 +93,7 @@ class OtelConfigTest : FunSpec({
9393
headers,
9494
"test-app-id",
9595
"https://api.onesignal.com",
96+
false,
9697
)
9798

9899
provider shouldNotBe null

0 commit comments

Comments
 (0)