From ef828d1cb7c7dccd9b2391e22f8b0503eef2db5c Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Tue, 24 Mar 2026 16:17:44 +0100 Subject: [PATCH 1/4] fix: align telemetry implementation with Swift SDK for feature parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add missing enum values (CACHED, STATIC, SPLIT, TYPE_MISMATCH, PROVIDER_FATAL) - Capture library under lock during snapshot to fix race condition - Always send X-CONFIDENCE-TELEMETRY header (match Swift behavior) - Fix Long→Int truncation in varint encoding for durationMs - Omit zero-valued fields in protobuf encoding (canonical wire format) - Replace reflection-based OpenFeature attribution with direct API call - Update tests for all changes Made-with: Cursor --- .../java/com/spotify/confidence/Confidence.kt | 3 +- .../spotify/confidence/RemoteFlagResolver.kt | 4 +- .../java/com/spotify/confidence/Telemetry.kt | 127 +++++++++--------- .../client/FlagApplierClientImpl.kt | 5 +- .../confidence/ConfidenceRemoteClientTests.kt | 8 +- .../com/spotify/confidence/TelemetryTest.kt | 84 ++++++++---- .../openfeature/ConfidenceFeatureProvider.kt | 8 +- 7 files changed, 135 insertions(+), 104 deletions(-) diff --git a/Confidence/src/main/java/com/spotify/confidence/Confidence.kt b/Confidence/src/main/java/com/spotify/confidence/Confidence.kt index c55d91b0..95af8a2d 100644 --- a/Confidence/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Confidence/src/main/java/com/spotify/confidence/Confidence.kt @@ -226,8 +226,7 @@ class Confidence internal constructor( it.getContext().filterKeys { key -> !removedKeys.contains(key) } + contextMap.value } ?: contextMap.value - @Suppress("unused") - private fun setTelemetryLibraryOpenFeature() { + fun setTelemetryLibraryOpenFeature() { telemetry.library = Telemetry.Library.OPEN_FEATURE } diff --git a/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt b/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt index f9f9e404..719a1829 100644 --- a/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt +++ b/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt @@ -48,9 +48,7 @@ internal class RemoteFlagResolver( .headers(headers) .post(jsonRequest.toRequestBody()) - telemetry.encodedHeaderValue()?.let { headerValue -> - requestBuilder.addHeader(Telemetry.HEADER_NAME, headerValue) - } + requestBuilder.addHeader(Telemetry.HEADER_NAME, telemetry.encodedHeaderValue()) val httpRequest = requestBuilder.build() diff --git a/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt b/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt index 10104e12..700c1560 100644 --- a/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt +++ b/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt @@ -31,67 +31,67 @@ internal class Telemetry( } } - fun encodedHeaderValue(): String? { - val (evalSnapshot, resolveSnapshot) = synchronized(lock) { - val evals = evaluationTraces.toList() - val resolves = resolveLatencyTraces.toList() + fun encodedHeaderValue(): String { + val snapshot = synchronized(lock) { + val s = Snapshot( + evaluations = evaluationTraces.toList(), + resolveTraces = resolveLatencyTraces.toList(), + library = library + ) evaluationTraces.clear() resolveLatencyTraces.clear() - Pair(evals, resolves) + s } - if (evalSnapshot.isEmpty() && resolveSnapshot.isEmpty()) return null - - val bytes = encodeMonitoring(evalSnapshot, resolveSnapshot) + val bytes = encodeMonitoring(snapshot) return Base64.encodeToString(bytes, Base64.NO_WRAP) } - private fun encodeMonitoring( - evals: List, - resolves: List - ): ByteArray { + private data class Snapshot( + val evaluations: List, + val resolveTraces: List, + val library: Library + ) + + private fun encodeMonitoring(snapshot: Snapshot): ByteArray { val out = ByteArrayOutputStream() - // field 1: LibraryTraces (length-delimited) - val libraryTracesBytes = encodeLibraryTraces(evals, resolves) + val libraryTracesBytes = encodeLibraryTraces(snapshot) out.writeTag(1, WIRE_TYPE_LENGTH_DELIMITED) - out.writeVarint(libraryTracesBytes.size) + out.writeVarint(libraryTracesBytes.size.toLong()) out.write(libraryTracesBytes) - // field 2: platform (varint) - KOTLIN = 2 - out.writeTag(2, WIRE_TYPE_VARINT) - out.writeVarint(Platform.KOTLIN.value) + if (Platform.KOTLIN.value != 0) { + out.writeTag(2, WIRE_TYPE_VARINT) + out.writeVarint(Platform.KOTLIN.value.toLong()) + } return out.toByteArray() } - private fun encodeLibraryTraces( - evals: List, - resolves: List - ): ByteArray { + private fun encodeLibraryTraces(snapshot: Snapshot): ByteArray { val out = ByteArrayOutputStream() - // field 1: library (varint) - out.writeTag(1, WIRE_TYPE_VARINT) - out.writeVarint(library.value) + if (snapshot.library.value != 0) { + out.writeTag(1, WIRE_TYPE_VARINT) + out.writeVarint(snapshot.library.value.toLong()) + } - // field 2: library_version (string) - out.writeTag(2, WIRE_TYPE_LENGTH_DELIMITED) val versionBytes = sdkVersion.toByteArray(Charsets.UTF_8) - out.writeVarint(versionBytes.size) + out.writeTag(2, WIRE_TYPE_LENGTH_DELIMITED) + out.writeVarint(versionBytes.size.toLong()) out.write(versionBytes) - // field 3: traces (repeated) - for (resolve in resolves) { + for (resolve in snapshot.resolveTraces) { val traceBytes = encodeResolveLatencyTrace(resolve) out.writeTag(3, WIRE_TYPE_LENGTH_DELIMITED) - out.writeVarint(traceBytes.size) + out.writeVarint(traceBytes.size.toLong()) out.write(traceBytes) } - for (eval in evals) { + for (eval in snapshot.evaluations) { val traceBytes = encodeEvaluationTrace(eval) out.writeTag(3, WIRE_TYPE_LENGTH_DELIMITED) - out.writeVarint(traceBytes.size) + out.writeVarint(traceBytes.size.toLong()) out.write(traceBytes) } @@ -101,14 +101,12 @@ internal class Telemetry( private fun encodeResolveLatencyTrace(trace: ResolveLatencyTrace): ByteArray { val out = ByteArrayOutputStream() - // field 1: id (varint) - RESOLVE_LATENCY = 1 out.writeTag(1, WIRE_TYPE_VARINT) - out.writeVarint(TraceId.RESOLVE_LATENCY.value) + out.writeVarint(TraceId.RESOLVE_LATENCY.value.toLong()) - // field 3: request_trace (length-delimited) - oneof field 3 val requestTraceBytes = encodeRequestTrace(trace) out.writeTag(3, WIRE_TYPE_LENGTH_DELIMITED) - out.writeVarint(requestTraceBytes.size) + out.writeVarint(requestTraceBytes.size.toLong()) out.write(requestTraceBytes) return out.toByteArray() @@ -117,13 +115,15 @@ internal class Telemetry( private fun encodeRequestTrace(trace: ResolveLatencyTrace): ByteArray { val out = ByteArrayOutputStream() - // field 1: latency_ms (varint) - out.writeTag(1, WIRE_TYPE_VARINT) - out.writeVarint(trace.durationMs.toInt()) + if (trace.durationMs != 0L) { + out.writeTag(1, WIRE_TYPE_VARINT) + out.writeVarint(trace.durationMs) + } - // field 2: status (varint) - out.writeTag(2, WIRE_TYPE_VARINT) - out.writeVarint(trace.status.value) + if (trace.status.value != 0) { + out.writeTag(2, WIRE_TYPE_VARINT) + out.writeVarint(trace.status.value.toLong()) + } return out.toByteArray() } @@ -131,15 +131,15 @@ internal class Telemetry( private fun encodeEvaluationTrace(trace: EvaluationTrace): ByteArray { val out = ByteArrayOutputStream() - // field 1: id (varint) - FLAG_EVALUATION = 3 out.writeTag(1, WIRE_TYPE_VARINT) - out.writeVarint(TraceId.FLAG_EVALUATION.value) + out.writeVarint(TraceId.FLAG_EVALUATION.value.toLong()) - // field 5: evaluation_trace (length-delimited) - oneof field 5 val evalTraceBytes = encodeEvalTraceBody(trace) - out.writeTag(5, WIRE_TYPE_LENGTH_DELIMITED) - out.writeVarint(evalTraceBytes.size) - out.write(evalTraceBytes) + if (evalTraceBytes.isNotEmpty()) { + out.writeTag(5, WIRE_TYPE_LENGTH_DELIMITED) + out.writeVarint(evalTraceBytes.size.toLong()) + out.write(evalTraceBytes) + } return out.toByteArray() } @@ -147,13 +147,15 @@ internal class Telemetry( private fun encodeEvalTraceBody(trace: EvaluationTrace): ByteArray { val out = ByteArrayOutputStream() - // field 1: reason (varint) - out.writeTag(1, WIRE_TYPE_VARINT) - out.writeVarint(trace.reason.value) + if (trace.reason.value != 0) { + out.writeTag(1, WIRE_TYPE_VARINT) + out.writeVarint(trace.reason.value.toLong()) + } - // field 2: error_code (varint) - out.writeTag(2, WIRE_TYPE_VARINT) - out.writeVarint(trace.errorCode.value) + if (trace.errorCode.value != 0) { + out.writeTag(2, WIRE_TYPE_VARINT) + out.writeVarint(trace.errorCode.value.toLong()) + } return out.toByteArray() } @@ -204,16 +206,16 @@ internal class Telemetry( } private fun ByteArrayOutputStream.writeTag(fieldNumber: Int, wireType: Int) { - writeVarint((fieldNumber shl 3) or wireType) + writeVarint(((fieldNumber shl 3) or wireType).toLong()) } - private fun ByteArrayOutputStream.writeVarint(value: Int) { + private fun ByteArrayOutputStream.writeVarint(value: Long) { var v = value - while (v and 0x7F.inv() != 0) { - write((v and 0x7F) or 0x80) + while (v and 0x7FL.inv() != 0L) { + write(((v and 0x7F) or 0x80).toInt()) v = v ushr 7 } - write(v) + write(v.toInt()) } } @@ -244,6 +246,9 @@ internal class Telemetry( DEFAULT(2), STALE(3), DISABLED(4), + CACHED(5), + STATIC(6), + SPLIT(7), ERROR(8) } @@ -252,8 +257,10 @@ internal class Telemetry( PROVIDER_NOT_READY(1), FLAG_NOT_FOUND(2), PARSE_ERROR(3), + TYPE_MISMATCH(4), TARGETING_KEY_MISSING(5), INVALID_CONTEXT(6), + PROVIDER_FATAL(7), GENERAL(8) } diff --git a/Confidence/src/main/java/com/spotify/confidence/client/FlagApplierClientImpl.kt b/Confidence/src/main/java/com/spotify/confidence/client/FlagApplierClientImpl.kt index 0af66ae3..470509a3 100644 --- a/Confidence/src/main/java/com/spotify/confidence/client/FlagApplierClientImpl.kt +++ b/Confidence/src/main/java/com/spotify/confidence/client/FlagApplierClientImpl.kt @@ -89,10 +89,7 @@ internal class FlagApplierClientImpl : FlagApplierClient { sdk ) - val extraHeaders = mutableMapOf() - telemetry.encodedHeaderValue()?.let { headerValue -> - extraHeaders[Telemetry.HEADER_NAME] = headerValue - } + val extraHeaders = mapOf(Telemetry.HEADER_NAME to telemetry.encodedHeaderValue()) val result = applyInteractor.invoke(request, extraHeaders).runCatching { if (isSuccessful) { diff --git a/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt b/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt index 0457a847..29f54a83 100644 --- a/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt +++ b/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt @@ -817,7 +817,7 @@ internal class ConfidenceRemoteClientTests { } @Test - fun testApplyRequestOmitsHeaderWhenNoTelemetry() = runTest { + fun testApplyRequestSendsEmptyTelemetryWhenNoTraces() = runTest { val testDispatcher = UnconfinedTestDispatcher(testScheduler) val applyDate = Date.from(Instant.parse("2023-03-01T14:01:46.123Z")) val sendDate = Date.from(Instant.parse("2023-03-01T14:03:46.124Z")) @@ -825,7 +825,6 @@ internal class ConfidenceRemoteClientTests { whenever(mockClock.currentTime()).thenReturn(sendDate) val telemetry = Telemetry(SDK_ID + "_TEST", Telemetry.Library.CONFIDENCE, "1.0.0") - // No events tracked var recordedRequest: RecordedRequest? = null mockWebServer.dispatcher = object : Dispatcher() { @@ -843,9 +842,10 @@ internal class ConfidenceRemoteClientTests { dispatcher = testDispatcher ).apply(listOf(AppliedFlag("flag1", applyDate)), "token1") + val header = recordedRequest?.getHeader(Telemetry.HEADER_NAME) assertTrue( - "No telemetry header expected when no events tracked", - recordedRequest?.getHeader(Telemetry.HEADER_NAME) == null + "Telemetry header should always be present", + header != null && header.isNotEmpty() ) } diff --git a/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt b/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt index ac6f8cd7..29e481f3 100644 --- a/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt +++ b/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt @@ -17,7 +17,7 @@ import okhttp3.mockwebserver.MockWebServer import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull + import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -196,9 +196,12 @@ class TelemetryTest { // --- Snapshot and clear --- @Test - fun testEncodedHeaderValueReturnsNullWhenEmpty() { + fun testEncodedHeaderValueReturnsEmptyMonitoringWhenNoTraces() { val telemetry = Telemetry("test-sdk", Telemetry.Library.CONFIDENCE, "1.0.0") - assertNull(telemetry.encodedHeaderValue()) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) + assertEquals(ProtoPlatform.PLATFORM_KOTLIN, monitoring.platform) + assertEquals(1, monitoring.libraryTracesCount) + assertEquals(0, monitoring.getLibraryTraces(0).tracesCount) } @Test @@ -210,8 +213,11 @@ class TelemetryTest { ) telemetry.trackResolveLatency(150, Telemetry.RequestStatus.SUCCESS) - assertNotNull(telemetry.encodedHeaderValue()) - assertNull(telemetry.encodedHeaderValue()) + val first = decodeMonitoring(telemetry.encodedHeaderValue()) + assertEquals(2, first.getLibraryTraces(0).tracesCount) + + val second = decodeMonitoring(telemetry.encodedHeaderValue()) + assertEquals(0, second.getLibraryTraces(0).tracesCount) } // --- Protobuf round-trip: encode then parse with generated code --- @@ -224,7 +230,7 @@ class TelemetryTest { Telemetry.EvaluationErrorCode.UNSPECIFIED ) - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) assertEquals(ProtoPlatform.PLATFORM_KOTLIN, monitoring.platform) assertEquals(1, monitoring.libraryTracesCount) @@ -248,7 +254,7 @@ class TelemetryTest { val telemetry = Telemetry("test-sdk", Telemetry.Library.CONFIDENCE, "2.0.0") telemetry.trackResolveLatency(142, Telemetry.RequestStatus.SUCCESS) - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) assertEquals(ProtoPlatform.PLATFORM_KOTLIN, monitoring.platform) @@ -273,7 +279,7 @@ class TelemetryTest { Telemetry.EvaluationErrorCode.FLAG_NOT_FOUND ) - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) val lib = monitoring.getLibraryTraces(0) assertEquals(LibraryTraces.Library.LIBRARY_OPEN_FEATURE, lib.library) @@ -298,7 +304,7 @@ class TelemetryTest { Telemetry.EvaluationErrorCode.UNSPECIFIED ) - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) val traces = monitoring.getLibraryTraces(0).tracesList assertEquals(4, traces.size) @@ -324,7 +330,7 @@ class TelemetryTest { val telemetry = Telemetry("test-sdk", Telemetry.Library.CONFIDENCE, "1.0.0") telemetry.trackResolveLatency(250, Telemetry.RequestStatus.ERROR) - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) val req = monitoring.getLibraryTraces(0).getTraces(0).requestTrace assertEquals(250L, req.millisecondDuration) assertEquals(ProtoStatus.STATUS_ERROR, req.status) @@ -370,6 +376,24 @@ class TelemetryTest { ProtoReason.EVALUATION_REASON_DISABLED, ProtoErrorCode.EVALUATION_ERROR_CODE_UNSPECIFIED ), + Case( + Telemetry.EvaluationReason.CACHED, + Telemetry.EvaluationErrorCode.UNSPECIFIED, + ProtoReason.EVALUATION_REASON_CACHED, + ProtoErrorCode.EVALUATION_ERROR_CODE_UNSPECIFIED + ), + Case( + Telemetry.EvaluationReason.STATIC, + Telemetry.EvaluationErrorCode.UNSPECIFIED, + ProtoReason.EVALUATION_REASON_STATIC, + ProtoErrorCode.EVALUATION_ERROR_CODE_UNSPECIFIED + ), + Case( + Telemetry.EvaluationReason.SPLIT, + Telemetry.EvaluationErrorCode.UNSPECIFIED, + ProtoReason.EVALUATION_REASON_SPLIT, + ProtoErrorCode.EVALUATION_ERROR_CODE_UNSPECIFIED + ), Case( Telemetry.EvaluationReason.ERROR, Telemetry.EvaluationErrorCode.GENERAL, @@ -405,6 +429,18 @@ class TelemetryTest { Telemetry.EvaluationErrorCode.TARGETING_KEY_MISSING, ProtoReason.EVALUATION_REASON_ERROR, ProtoErrorCode.EVALUATION_ERROR_CODE_TARGETING_KEY_MISSING + ), + Case( + Telemetry.EvaluationReason.ERROR, + Telemetry.EvaluationErrorCode.TYPE_MISMATCH, + ProtoReason.EVALUATION_REASON_ERROR, + ProtoErrorCode.EVALUATION_ERROR_CODE_TYPE_MISMATCH + ), + Case( + Telemetry.EvaluationReason.ERROR, + Telemetry.EvaluationErrorCode.PROVIDER_FATAL, + ProtoReason.EVALUATION_REASON_ERROR, + ProtoErrorCode.EVALUATION_ERROR_CODE_PROVIDER_FATAL ) ) @@ -412,7 +448,7 @@ class TelemetryTest { val telemetry = Telemetry("test-sdk", Telemetry.Library.CONFIDENCE, "1.0.0") telemetry.trackEvaluation(case.reason, case.errorCode) - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) val eval = monitoring.getLibraryTraces(0).getTraces(0).evaluationTrace assertEquals("reason for ${case.reason}", case.expectedReason, eval.reason) @@ -438,7 +474,7 @@ class TelemetryTest { } @Test - fun testSetTelemetryLibraryOpenFeatureViaReflection() { + fun testSetTelemetryLibraryOpenFeature() { val confidence = Confidence( clientSecret = "", dispatcher = kotlinx.coroutines.Dispatchers.Unconfined, @@ -449,22 +485,17 @@ class TelemetryTest { debugLogger = null ) - // Default should be CONFIDENCE assertEquals(Telemetry.Library.CONFIDENCE, confidence.telemetry.library) - // Call the private method via reflection (same as ConfidenceFeatureProvider.create) - val method = confidence.javaClass.getDeclaredMethod("setTelemetryLibraryOpenFeature") - method.isAccessible = true - method.invoke(confidence) + confidence.setTelemetryLibraryOpenFeature() assertEquals(Telemetry.Library.OPEN_FEATURE, confidence.telemetry.library) - // Verify encoded header uses OPEN_FEATURE confidence.telemetry.trackEvaluation( Telemetry.EvaluationReason.DEFAULT, Telemetry.EvaluationErrorCode.UNSPECIFIED ) - val monitoring = decodeMonitoring(confidence.telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(confidence.telemetry.encodedHeaderValue()) assertEquals( LibraryTraces.Library.LIBRARY_OPEN_FEATURE, monitoring.getLibraryTraces(0).library @@ -532,13 +563,14 @@ class TelemetryTest { latch.await() executor.shutdown() - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) val traces = monitoring.getLibraryTraces(0).tracesList // Each list is capped at 100, so max 200 total (100 eval + 100 latency) assertTrue("Traces should be capped at 200", traces.size <= 200) assertTrue("Should have some traces", traces.size > 0) - assertNull(telemetry.encodedHeaderValue()) + val afterFlush = decodeMonitoring(telemetry.encodedHeaderValue()) + assertEquals(0, afterFlush.getLibraryTraces(0).tracesCount) } @Test @@ -554,7 +586,7 @@ class TelemetryTest { telemetry.trackResolveLatency(50, Telemetry.RequestStatus.SUCCESS) } - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()!!) + val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) val traces = monitoring.getLibraryTraces(0).tracesList val evalTraces = traces.filter { it.id == LibraryTraces.TraceId.TRACE_ID_FLAG_EVALUATION @@ -640,6 +672,7 @@ class TelemetryTest { assertNotNull("Expected ${Telemetry.HEADER_NAME} header", headerValue) val monitoring = decodeMonitoring(headerValue!!) + assertEquals(ProtoPlatform.PLATFORM_KOTLIN, monitoring.platform) val lib = monitoring.getLibraryTraces(0) @@ -658,7 +691,7 @@ class TelemetryTest { } @Test - fun testResolveRequestOmitsHeaderWhenNoTelemetry() = runTest { + fun testResolveRequestSendsEmptyTelemetryWhenNoTraces() = runTest { val mockWebServer = MockWebServer() mockWebServer.start() try { @@ -676,7 +709,10 @@ class TelemetryTest { telemetry = telemetry ).resolve(listOf(), mapOf("targeting_key" to ConfidenceValue.String("user1"))) - assertNull(mockWebServer.takeRequest().getHeader(Telemetry.HEADER_NAME)) + val header = mockWebServer.takeRequest().getHeader(Telemetry.HEADER_NAME) + assertNotNull("Header should always be present", header) + val monitoring = decodeMonitoring(header!!) + assertEquals(0, monitoring.getLibraryTraces(0).tracesCount) } finally { mockWebServer.shutdown() } diff --git a/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt index d7c48bac..2a5cf3d3 100644 --- a/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt @@ -137,13 +137,7 @@ class ConfidenceFeatureProvider private constructor( hooks: List> = listOf(), metadata: ProviderMetadata = ConfidenceMetadata() ): ConfidenceFeatureProvider { - try { - val method = confidence.javaClass.getDeclaredMethod("setTelemetryLibraryOpenFeature") - method.isAccessible = true - method.invoke(confidence) - } catch (_: Exception) { - // Best effort - telemetry will default to CONFIDENCE library - } + confidence.setTelemetryLibraryOpenFeature() return ConfidenceFeatureProvider( hooks = hooks, metadata = metadata, From 1d9e6de16954cd4d3465341beeb0b330085b97ad Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Tue, 24 Mar 2026 16:39:44 +0100 Subject: [PATCH 2/4] chore: add telemetry test button to demo app Adds a "Test Telemetry" button that evaluates both real flags (match) and nonexistent flags (error), then triggers a fetch to flush the telemetry header for verification in Grafana. Made-with: Cursor --- .../example/confidencedemoapp/MainActivity.kt | 3 ++ .../com/example/confidencedemoapp/MainVm.kt | 30 ++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt index e9af25ca..791431ed 100644 --- a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt +++ b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt @@ -49,6 +49,9 @@ class MainActivity : ComponentActivity() { Text("Re-apply evaluationContext") } + Button(onClick = { vm.testTelemetry() }) { + Text("Test Telemetry (match + error)") + } Button(onClick = { vm.clear() }) { Text(text = "Clear out for re-fetching") } diff --git a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainVm.kt b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainVm.kt index e4123207..5467ce8e 100644 --- a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainVm.kt +++ b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainVm.kt @@ -31,6 +31,8 @@ class MainVm(app: Application) : AndroidViewModel(app) { val color: LiveData = _color val surfaceText: LiveData = _surfaceText + private lateinit var confidence: Confidence + init { val start = System.currentTimeMillis() val clientSecret = ClientSecretProvider.clientSecret(app.applicationContext) @@ -43,7 +45,7 @@ class MainVm(app: Application) : AndroidViewModel(app) { mutableMap["list"] = ConfidenceValue.stringList(listOf("")) mutableMap["my_struct"] = ConfidenceValue.Struct(mapOf("x" to ConfidenceValue.Double(2.0))) - val confidence = ConfidenceFactory.create( + confidence = ConfidenceFactory.create( app.applicationContext, clientSecret, initialContext = mapOf("targeting_key" to ConfidenceValue.String("a98a4291-53b0-49d9-bae8-73d3f5da2070")), @@ -111,6 +113,32 @@ class MainVm(app: Application) : AndroidViewModel(app) { } } + fun testTelemetry() { + val client = OpenFeatureAPI.getClient() + Log.d(TAG, "--- Telemetry test: generating match + error evaluations ---") + + val matchResult = client.getStringDetails("hawkflag.color", "default") + Log.d(TAG, "Match eval: value=${matchResult.value}, reason=${matchResult.reason}") + + val errorResult = client.getStringDetails("nonexistent-flag.value", "fallback") + Log.d(TAG, "Error eval: value=${errorResult.value}, reason=${errorResult.reason}, " + + "errorCode=${errorResult.errorCode}") + + val matchResult2 = client.getIntegerDetails("hawkflag.size", 0) + Log.d(TAG, "Match eval 2: value=${matchResult2.value}, reason=${matchResult2.reason}") + + val errorResult2 = client.getBooleanDetails("also-does-not-exist.enabled", false) + Log.d(TAG, "Error eval 2: value=${errorResult2.value}, reason=${errorResult2.reason}, " + + "errorCode=${errorResult2.errorCode}") + + Log.d(TAG, "Triggering fetch to flush telemetry header...") + viewModelScope.launch { + confidence.fetchAndActivate() + Log.d(TAG, "Fetch complete — telemetry header sent with 2 match + 2 error evaluations") + _message.postValue("Telemetry flushed: 2 match + 2 error evals") + } + } + fun clear() { Log.d(TAG, "clearing context") OpenFeatureAPI.setEvaluationContext(ImmutableContext()) From 72e827c0dac9b728401124357c4bead89639360f Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Tue, 24 Mar 2026 16:41:10 +0100 Subject: [PATCH 3/4] Revert "chore: add telemetry test button to demo app" This reverts commit 1d9e6de16954cd4d3465341beeb0b330085b97ad. --- .../example/confidencedemoapp/MainActivity.kt | 3 -- .../com/example/confidencedemoapp/MainVm.kt | 30 +------------------ 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt index 791431ed..e9af25ca 100644 --- a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt +++ b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt @@ -49,9 +49,6 @@ class MainActivity : ComponentActivity() { Text("Re-apply evaluationContext") } - Button(onClick = { vm.testTelemetry() }) { - Text("Test Telemetry (match + error)") - } Button(onClick = { vm.clear() }) { Text(text = "Clear out for re-fetching") } diff --git a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainVm.kt b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainVm.kt index 5467ce8e..e4123207 100644 --- a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainVm.kt +++ b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainVm.kt @@ -31,8 +31,6 @@ class MainVm(app: Application) : AndroidViewModel(app) { val color: LiveData = _color val surfaceText: LiveData = _surfaceText - private lateinit var confidence: Confidence - init { val start = System.currentTimeMillis() val clientSecret = ClientSecretProvider.clientSecret(app.applicationContext) @@ -45,7 +43,7 @@ class MainVm(app: Application) : AndroidViewModel(app) { mutableMap["list"] = ConfidenceValue.stringList(listOf("")) mutableMap["my_struct"] = ConfidenceValue.Struct(mapOf("x" to ConfidenceValue.Double(2.0))) - confidence = ConfidenceFactory.create( + val confidence = ConfidenceFactory.create( app.applicationContext, clientSecret, initialContext = mapOf("targeting_key" to ConfidenceValue.String("a98a4291-53b0-49d9-bae8-73d3f5da2070")), @@ -113,32 +111,6 @@ class MainVm(app: Application) : AndroidViewModel(app) { } } - fun testTelemetry() { - val client = OpenFeatureAPI.getClient() - Log.d(TAG, "--- Telemetry test: generating match + error evaluations ---") - - val matchResult = client.getStringDetails("hawkflag.color", "default") - Log.d(TAG, "Match eval: value=${matchResult.value}, reason=${matchResult.reason}") - - val errorResult = client.getStringDetails("nonexistent-flag.value", "fallback") - Log.d(TAG, "Error eval: value=${errorResult.value}, reason=${errorResult.reason}, " + - "errorCode=${errorResult.errorCode}") - - val matchResult2 = client.getIntegerDetails("hawkflag.size", 0) - Log.d(TAG, "Match eval 2: value=${matchResult2.value}, reason=${matchResult2.reason}") - - val errorResult2 = client.getBooleanDetails("also-does-not-exist.enabled", false) - Log.d(TAG, "Error eval 2: value=${errorResult2.value}, reason=${errorResult2.reason}, " + - "errorCode=${errorResult2.errorCode}") - - Log.d(TAG, "Triggering fetch to flush telemetry header...") - viewModelScope.launch { - confidence.fetchAndActivate() - Log.d(TAG, "Fetch complete — telemetry header sent with 2 match + 2 error evaluations") - _message.postValue("Telemetry flushed: 2 match + 2 error evals") - } - } - fun clear() { Log.d(TAG, "clearing context") OpenFeatureAPI.setEvaluationContext(ImmutableContext()) From 72be549ba7dfe030ff37a76ddafcce8d7c2de96d Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Tue, 24 Mar 2026 17:27:48 +0100 Subject: [PATCH 4/4] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20keep=20header=20conditional=20and=20reflection-base?= =?UTF-8?q?d=20attribution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Revert encodedHeaderValue() to return String? (don't send header when no traces) - Restore conditional header logic in RemoteFlagResolver and FlagApplierClientImpl - Keep setTelemetryLibraryOpenFeature() private, called via reflection from Provider - Restore corresponding tests to assert null/reflection behavior Retained from parity work: - Missing enum values (CACHED, STATIC, SPLIT, TYPE_MISMATCH, PROVIDER_FATAL) - Library captured under lock during snapshot (race condition fix) - Long-capable writeVarint (durationMs truncation fix) - Zero-valued field omission (canonical protobuf encoding) Made-with: Cursor --- .../java/com/spotify/confidence/Confidence.kt | 3 +- .../spotify/confidence/RemoteFlagResolver.kt | 4 ++- .../java/com/spotify/confidence/Telemetry.kt | 4 ++- .../client/FlagApplierClientImpl.kt | 5 ++- .../confidence/ConfidenceRemoteClientTests.kt | 7 ++-- .../com/spotify/confidence/TelemetryTest.kt | 34 +++++++------------ .../openfeature/ConfidenceFeatureProvider.kt | 8 ++++- 7 files changed, 35 insertions(+), 30 deletions(-) diff --git a/Confidence/src/main/java/com/spotify/confidence/Confidence.kt b/Confidence/src/main/java/com/spotify/confidence/Confidence.kt index 95af8a2d..c55d91b0 100644 --- a/Confidence/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Confidence/src/main/java/com/spotify/confidence/Confidence.kt @@ -226,7 +226,8 @@ class Confidence internal constructor( it.getContext().filterKeys { key -> !removedKeys.contains(key) } + contextMap.value } ?: contextMap.value - fun setTelemetryLibraryOpenFeature() { + @Suppress("unused") + private fun setTelemetryLibraryOpenFeature() { telemetry.library = Telemetry.Library.OPEN_FEATURE } diff --git a/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt b/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt index 719a1829..f9f9e404 100644 --- a/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt +++ b/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt @@ -48,7 +48,9 @@ internal class RemoteFlagResolver( .headers(headers) .post(jsonRequest.toRequestBody()) - requestBuilder.addHeader(Telemetry.HEADER_NAME, telemetry.encodedHeaderValue()) + telemetry.encodedHeaderValue()?.let { headerValue -> + requestBuilder.addHeader(Telemetry.HEADER_NAME, headerValue) + } val httpRequest = requestBuilder.build() diff --git a/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt b/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt index 700c1560..440e60be 100644 --- a/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt +++ b/Confidence/src/main/java/com/spotify/confidence/Telemetry.kt @@ -31,7 +31,7 @@ internal class Telemetry( } } - fun encodedHeaderValue(): String { + fun encodedHeaderValue(): String? { val snapshot = synchronized(lock) { val s = Snapshot( evaluations = evaluationTraces.toList(), @@ -43,6 +43,8 @@ internal class Telemetry( s } + if (snapshot.evaluations.isEmpty() && snapshot.resolveTraces.isEmpty()) return null + val bytes = encodeMonitoring(snapshot) return Base64.encodeToString(bytes, Base64.NO_WRAP) } diff --git a/Confidence/src/main/java/com/spotify/confidence/client/FlagApplierClientImpl.kt b/Confidence/src/main/java/com/spotify/confidence/client/FlagApplierClientImpl.kt index 470509a3..0af66ae3 100644 --- a/Confidence/src/main/java/com/spotify/confidence/client/FlagApplierClientImpl.kt +++ b/Confidence/src/main/java/com/spotify/confidence/client/FlagApplierClientImpl.kt @@ -89,7 +89,10 @@ internal class FlagApplierClientImpl : FlagApplierClient { sdk ) - val extraHeaders = mapOf(Telemetry.HEADER_NAME to telemetry.encodedHeaderValue()) + val extraHeaders = mutableMapOf() + telemetry.encodedHeaderValue()?.let { headerValue -> + extraHeaders[Telemetry.HEADER_NAME] = headerValue + } val result = applyInteractor.invoke(request, extraHeaders).runCatching { if (isSuccessful) { diff --git a/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt b/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt index 29f54a83..3c449cb8 100644 --- a/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt +++ b/Confidence/src/test/java/com/spotify/confidence/ConfidenceRemoteClientTests.kt @@ -817,7 +817,7 @@ internal class ConfidenceRemoteClientTests { } @Test - fun testApplyRequestSendsEmptyTelemetryWhenNoTraces() = runTest { + fun testApplyRequestOmitsHeaderWhenNoTelemetry() = runTest { val testDispatcher = UnconfinedTestDispatcher(testScheduler) val applyDate = Date.from(Instant.parse("2023-03-01T14:01:46.123Z")) val sendDate = Date.from(Instant.parse("2023-03-01T14:03:46.124Z")) @@ -842,10 +842,9 @@ internal class ConfidenceRemoteClientTests { dispatcher = testDispatcher ).apply(listOf(AppliedFlag("flag1", applyDate)), "token1") - val header = recordedRequest?.getHeader(Telemetry.HEADER_NAME) assertTrue( - "Telemetry header should always be present", - header != null && header.isNotEmpty() + "No telemetry header expected when no events tracked", + recordedRequest?.getHeader(Telemetry.HEADER_NAME) == null ) } diff --git a/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt b/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt index 29e481f3..b45bed55 100644 --- a/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt +++ b/Confidence/src/test/java/com/spotify/confidence/TelemetryTest.kt @@ -17,7 +17,7 @@ import okhttp3.mockwebserver.MockWebServer import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull - +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -196,12 +196,9 @@ class TelemetryTest { // --- Snapshot and clear --- @Test - fun testEncodedHeaderValueReturnsEmptyMonitoringWhenNoTraces() { + fun testEncodedHeaderValueReturnsNullWhenEmpty() { val telemetry = Telemetry("test-sdk", Telemetry.Library.CONFIDENCE, "1.0.0") - val monitoring = decodeMonitoring(telemetry.encodedHeaderValue()) - assertEquals(ProtoPlatform.PLATFORM_KOTLIN, monitoring.platform) - assertEquals(1, monitoring.libraryTracesCount) - assertEquals(0, monitoring.getLibraryTraces(0).tracesCount) + assertNull(telemetry.encodedHeaderValue()) } @Test @@ -213,11 +210,8 @@ class TelemetryTest { ) telemetry.trackResolveLatency(150, Telemetry.RequestStatus.SUCCESS) - val first = decodeMonitoring(telemetry.encodedHeaderValue()) - assertEquals(2, first.getLibraryTraces(0).tracesCount) - - val second = decodeMonitoring(telemetry.encodedHeaderValue()) - assertEquals(0, second.getLibraryTraces(0).tracesCount) + assertNotNull(telemetry.encodedHeaderValue()) + assertNull(telemetry.encodedHeaderValue()) } // --- Protobuf round-trip: encode then parse with generated code --- @@ -474,7 +468,7 @@ class TelemetryTest { } @Test - fun testSetTelemetryLibraryOpenFeature() { + fun testSetTelemetryLibraryOpenFeatureViaReflection() { val confidence = Confidence( clientSecret = "", dispatcher = kotlinx.coroutines.Dispatchers.Unconfined, @@ -487,7 +481,9 @@ class TelemetryTest { assertEquals(Telemetry.Library.CONFIDENCE, confidence.telemetry.library) - confidence.setTelemetryLibraryOpenFeature() + val method = confidence.javaClass.getDeclaredMethod("setTelemetryLibraryOpenFeature") + method.isAccessible = true + method.invoke(confidence) assertEquals(Telemetry.Library.OPEN_FEATURE, confidence.telemetry.library) @@ -495,7 +491,7 @@ class TelemetryTest { Telemetry.EvaluationReason.DEFAULT, Telemetry.EvaluationErrorCode.UNSPECIFIED ) - val monitoring = decodeMonitoring(confidence.telemetry.encodedHeaderValue()) + val monitoring = decodeMonitoring(confidence.telemetry.encodedHeaderValue()!!) assertEquals( LibraryTraces.Library.LIBRARY_OPEN_FEATURE, monitoring.getLibraryTraces(0).library @@ -569,8 +565,7 @@ class TelemetryTest { assertTrue("Traces should be capped at 200", traces.size <= 200) assertTrue("Should have some traces", traces.size > 0) - val afterFlush = decodeMonitoring(telemetry.encodedHeaderValue()) - assertEquals(0, afterFlush.getLibraryTraces(0).tracesCount) + assertNull(telemetry.encodedHeaderValue()) } @Test @@ -691,7 +686,7 @@ class TelemetryTest { } @Test - fun testResolveRequestSendsEmptyTelemetryWhenNoTraces() = runTest { + fun testResolveRequestOmitsHeaderWhenNoTelemetry() = runTest { val mockWebServer = MockWebServer() mockWebServer.start() try { @@ -709,10 +704,7 @@ class TelemetryTest { telemetry = telemetry ).resolve(listOf(), mapOf("targeting_key" to ConfidenceValue.String("user1"))) - val header = mockWebServer.takeRequest().getHeader(Telemetry.HEADER_NAME) - assertNotNull("Header should always be present", header) - val monitoring = decodeMonitoring(header!!) - assertEquals(0, monitoring.getLibraryTraces(0).tracesCount) + assertNull(mockWebServer.takeRequest().getHeader(Telemetry.HEADER_NAME)) } finally { mockWebServer.shutdown() } diff --git a/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt index 2a5cf3d3..d7c48bac 100644 --- a/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt @@ -137,7 +137,13 @@ class ConfidenceFeatureProvider private constructor( hooks: List> = listOf(), metadata: ProviderMetadata = ConfidenceMetadata() ): ConfidenceFeatureProvider { - confidence.setTelemetryLibraryOpenFeature() + try { + val method = confidence.javaClass.getDeclaredMethod("setTelemetryLibraryOpenFeature") + method.isAccessible = true + method.invoke(confidence) + } catch (_: Exception) { + // Best effort - telemetry will default to CONFIDENCE library + } return ConfidenceFeatureProvider( hooks = hooks, metadata = metadata,