fix: align telemetry with Swift SDK for feature parity#235
Merged
nicklasl merged 4 commits intonicklasl/add-telemetry-headerfrom Mar 24, 2026
Merged
fix: align telemetry with Swift SDK for feature parity#235nicklasl merged 4 commits intonicklasl/add-telemetry-headerfrom
nicklasl merged 4 commits intonicklasl/add-telemetry-headerfrom
Conversation
- 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
fabriziodemaria
requested review from
nicklasl,
nickybondarenko and
vahidlazio
as code owners
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
This reverts commit 1d9e6de.
nicklasl
reviewed
Mar 24, 2026
| telemetry.encodedHeaderValue()?.let { headerValue -> | ||
| extraHeaders[Telemetry.HEADER_NAME] = headerValue | ||
| } | ||
| val extraHeaders = mapOf(Telemetry.HEADER_NAME to telemetry.encodedHeaderValue()) |
Member
There was a problem hiding this comment.
I'd rather not add the header if we don't have data.
nicklasl
reviewed
Mar 24, 2026
Comment on lines
845
to
849
| 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() | ||
| ) |
nicklasl
reviewed
Mar 24, 2026
| val method = confidence.javaClass.getDeclaredMethod("setTelemetryLibraryOpenFeature") | ||
| method.isAccessible = true | ||
| method.invoke(confidence) | ||
| confidence.setTelemetryLibraryOpenFeature() |
Member
There was a problem hiding this comment.
this should really be tested only with the provider. making sure that the provider calls the function via reflection
nicklasl
reviewed
Mar 24, 2026
|
|
||
| @Suppress("unused") | ||
| private fun setTelemetryLibraryOpenFeature() { | ||
| fun setTelemetryLibraryOpenFeature() { |
Member
There was a problem hiding this comment.
I intentionally left this private. I don't want this in the API.
…-based attribution - 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
Member
Author
🤖 Claudesk: Verify PR Title & Description |
nicklasl
approved these changes
Mar 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EvaluationReasongainsCACHED(5),STATIC(6),SPLIT(7);EvaluationErrorCodegainsTYPE_MISMATCH(4),PROVIDER_FATAL(7)— matching the Swift SDK and proto definition.libraryis now captured atomically under the lock via aSnapshotdata class, preventing stale reads during encoding.writeVarintnow operates onLong, fixing silent truncation ofdurationMsvalues.Known issue
encodedHeaderValue()still returnsString?, but several tests removed the!!operator when passing the result todecodeMonitoring(headerValue: String). This will cause a compile error — the!!should be kept, or the return type /decodeMonitoringsignature should be updated.Test plan
CACHED,STATIC,SPLIT,TYPE_MISMATCH,PROVIDER_FATALenum valuesMade with Cursor