Skip to content

Commit acb4aaf

Browse files
author
AR Abdul Azeez
committed
some more cleanup and fixes
1 parent e6f0e6c commit acb4aaf

4 files changed

Lines changed: 41 additions & 41 deletions

File tree

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

Lines changed: 0 additions & 18 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,

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ import java.time.Duration
1010

1111
internal class OtelConfigRemoteOneSignal {
1212
companion object {
13-
const val OTEL_PATH = "sdk/otel"
13+
const val OTEL_PATH = "sdk"
1414

1515
fun buildEndpoint(apiBaseUrl: String, appId: String): String =
16-
"$apiBaseUrl$OTEL_PATH/v1/logs?app_id=$appId"
16+
"$apiBaseUrl$OTEL_PATH/log?app_id=$appId"
1717
}
1818

1919
object LogRecordExporterConfig {
@@ -85,6 +85,28 @@ internal class OtelConfigRemoteOneSignal {
8585
private class LoggingLogRecordExporter(
8686
private val delegate: LogRecordExporter
8787
) : 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+
88110
override fun export(logs: Collection<LogRecordData>): CompletableResultCode {
89111
Log.d(TAG, "OTEL export request sent to backend. count=${logs.size}")
90112
val result = delegate.export(logs)
@@ -93,9 +115,10 @@ internal class OtelConfigRemoteOneSignal {
93115
Log.d(TAG, "OTEL export response received: success")
94116
} else {
95117
val throwable = result.failureThrowable
118+
val failureMessage = resolveHttpFailureMessage(throwable)
96119
Log.e(
97120
TAG,
98-
"OTEL export response received: failed${throwable?.let { " - ${it.message}" } ?: ""}",
121+
"OTEL export response received: failed - $failureMessage",
99122
throwable
100123
)
101124
}

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)