Skip to content

Commit 6cbc102

Browse files
tanderson-ldtanderson-ld
committed
addressing lots of review comments
1 parent 3d7315a commit 6cbc102

File tree

9 files changed

+112
-143
lines changed

9 files changed

+112
-143
lines changed

‎sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/client/InstrumentationManager.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -413,10 +413,10 @@ class InstrumentationManager(
413413
// pipeline to provide instrumentation specific caching and export.
414414
val routingLogRecordProcessor =
415415
RoutingLogRecordProcessor(fallthroughProcessor = baseProcessor)
416-
for (i in options.instrumentations) {
417-
i.getLogRecordProcessor(credential = sdkKey)?.let {
418-
i.getLoggerScopeName().let { scopeName ->
419-
routingLogRecordProcessor.registerProcessor(scopeName, it)
416+
options.instrumentations.forEach { instrumentation ->
417+
instrumentation.getLogRecordProcessor(credential = sdkKey)?.let { processor ->
418+
instrumentation.getLoggerScopeName().let { scopeName ->
419+
routingLogRecordProcessor.addProcessor(scopeName, processor)
420420
}
421421
}
422422
}

‎sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/client/NoopLogRecordPrcoessor.kt‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,6 @@ import io.opentelemetry.sdk.logs.ReadWriteLogRecord
77
/**
88
* A [LogRecordProcessor] that, surprise, does nothing.
99
*/
10-
internal class NoopLogRecordProcessor private constructor() : LogRecordProcessor {
10+
internal object NoopLogRecordProcessor : LogRecordProcessor {
1111
override fun onEmit(context: Context, logRecord: ReadWriteLogRecord) {}
12-
13-
companion object {
14-
private val INSTANCE = NoopLogRecordProcessor()
15-
16-
val instance: LogRecordProcessor
17-
get() = INSTANCE
18-
}
19-
}
12+
}

‎sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/client/RoutingLogRecordProcessor.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ import java.util.concurrent.ConcurrentHashMap
1111
* scope name, the [fallthroughProcessor] is called to handle the log.
1212
*/
1313
class RoutingLogRecordProcessor(
14-
private val fallthroughProcessor: LogRecordProcessor = NoopLogRecordProcessor.instance
14+
private val fallthroughProcessor: LogRecordProcessor = NoopLogRecordProcessor
1515
) : LogRecordProcessor {
1616
private val processors = ConcurrentHashMap<String, LogRecordProcessor>()
1717

18-
fun registerProcessor(scopeName: String, processor: LogRecordProcessor) {
18+
fun addProcessor(scopeName: String, processor: LogRecordProcessor) {
1919
processors[scopeName] = processor
2020
}
2121

‎sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/CaptureSource.kt‎

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@ class CaptureSource(
4444
private val sessionManager: SessionManager,
4545
private val privacyProfile: PrivacyProfile,
4646
// TODO: O11Y-628 - add captureQuality options
47-
) :
48-
Application.ActivityLifecycleCallbacks {
47+
) : Application.ActivityLifecycleCallbacks {
4948

5049
private var _activity: Activity? = null
5150

@@ -69,13 +68,10 @@ class CaptureSource(
6968
/**
7069
* Requests a [Capture] be taken now.
7170
*/
72-
fun captureNow() {
73-
// TODO: O11Y-621 - don't use global scope
74-
CoroutineScope(Dispatchers.Default).launch {
75-
val capture = doCapture()
76-
if (capture != null) {
77-
_captureFlow.emit(capture)
78-
}
71+
suspend fun captureNow() {
72+
val capture = doCapture()
73+
if (capture != null) {
74+
_captureFlow.emit(capture)
7975
}
8076
}
8177

@@ -92,7 +88,7 @@ class CaptureSource(
9288
}
9389

9490
override fun onActivityPaused(activity: Activity) {
95-
_activity = null;
91+
_activity = null
9692
}
9793

9894
override fun onActivityStopped(activity: Activity) {
@@ -111,7 +107,7 @@ class CaptureSource(
111107
* Internal capture routine.
112108
*/
113109
private suspend fun doCapture(): Capture? = withContext(Dispatchers.Main) {
114-
val activity = _activity ?: return@withContext null;
110+
val activity = _activity ?: return@withContext null
115111

116112
try {
117113
val window = activity.window
@@ -146,28 +142,38 @@ class CaptureSource(
146142
val session = sessionManager.getSessionId()
147143

148144
if (result == PixelCopy.SUCCESS) {
149-
val postMask: Bitmap
150-
if (privacyProfile == PrivacyProfile.STRICT) {
151-
postMask = maskSensitiveAreas(bitmap, activity)
152-
} else {
153-
postMask = bitmap
145+
// Offload heavy bitmap work to a background dispatcher
146+
CoroutineScope(Dispatchers.Default).launch {
147+
try {
148+
val postMask = bitmap;
149+
// TODO: O11Y-620 - masking
150+
// val postMask: Bitmap =
151+
// if (privacyProfile == PrivacyProfile.STRICT) {
152+
// maskSensitiveAreas(bitmap, activity)
153+
// } else {
154+
// bitmap
155+
// }
156+
157+
// TODO: O11Y-625 - optimize memory allocations here, re-use byte arrays and such
158+
val outputStream = ByteArrayOutputStream()
159+
// TODO: O11Y-628 - calculate quality using captureQuality options
160+
postMask.compress(Bitmap.CompressFormat.WEBP, 30, outputStream)
161+
val byteArray = outputStream.toByteArray()
162+
val compressedImage =
163+
Base64.encodeToString(byteArray, Base64.NO_WRAP)
164+
165+
val capture = Capture(
166+
imageBase64 = compressedImage,
167+
origWidth = decorViewWidth,
168+
origHeight = decorViewHeight,
169+
timestamp = timestamp,
170+
session = session,
171+
)
172+
continuation.resume(capture)
173+
} catch (e: Exception) {
174+
continuation.resumeWithException(e)
175+
}
154176
}
155-
156-
// TODO: O11Y-625 - optimize memory allocations here, re-use byte arrays and such
157-
val outputStream = ByteArrayOutputStream()
158-
// TODO: O11Y-628 - calculate quality using captureQuality options
159-
postMask.compress(Bitmap.CompressFormat.WEBP, 30, outputStream)
160-
val byteArray = outputStream.toByteArray()
161-
val compressedImage = Base64.encodeToString(byteArray, Base64.NO_WRAP)
162-
163-
val capture = Capture(
164-
imageBase64 = compressedImage,
165-
origWidth = decorViewWidth,
166-
origHeight = decorViewHeight,
167-
timestamp = timestamp,
168-
session = session,
169-
)
170-
continuation.resume(capture)
171177
} else {
172178
// TODO: O11Y-624 - implement handling/shutdown for errors and unsupported API levels
173179
continuation.resumeWithException(Exception("PixelCopy failed with result: $result"))
@@ -182,7 +188,7 @@ class CaptureSource(
182188
}
183189
} catch (e: Exception) {
184190
// TODO: O11Y-624 - implement handling/shutdown for errors and unsupported API levels
185-
throw RuntimeException(e);
191+
throw RuntimeException(e)
186192
}
187193
}
188194

@@ -351,10 +357,6 @@ class CaptureSource(
351357

352358
// Check for content description containing "sensitive"
353359
val contentDescriptions = node.config.getOrNull(SemanticsProperties.ContentDescription)
354-
if (contentDescriptions?.any { it.contains("sensitive", ignoreCase = true) } == true) {
355-
return true
356-
}
357-
358-
return false
360+
return contentDescriptions?.any { it.contains("sensitive", ignoreCase = true) } == true
359361
}
360362
}

‎sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/RRwebGraphQLReplayLogExporter.kt‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.launchdarkly.observability.replay
22

3-
import android.util.Log
43
import com.launchdarkly.observability.network.GraphQLClient
54
import io.opentelemetry.api.common.AttributeKey
65
import io.opentelemetry.sdk.common.CompletableResultCode
@@ -44,9 +43,14 @@ class RRwebGraphQLReplayLogExporter(
4443
// TODO: O11Y-624 - need to implement sid, payloadId reset when multiple sessions occur in one application process lifecycle.
4544
private var sidCounter = 0
4645
private var payloadIdCounter = 0
47-
private var lastSentHeight = 0
48-
private var lastSentWidth = 0
49-
private var lastSessionId: String? = null
46+
47+
private data class LastSentState(
48+
val sessionId: String?,
49+
val height: Int,
50+
val width: Int,
51+
)
52+
53+
private var lastSentState = LastSentState(sessionId = null, height = 0, width = 0)
5054

5155
override fun export(logs: MutableCollection<LogRecordData>): CompletableResultCode {
5256
val resultCode = CompletableResultCode()
@@ -58,7 +62,7 @@ class RRwebGraphQLReplayLogExporter(
5862
if (capture != null) {
5963
// TODO: O11Y-624 - investigate if there is a size limit on the push that is imposed server side.
6064
val success =
61-
if (!capture.session.equals(lastSessionId) || lastSentWidth != capture.origWidth || lastSentHeight != capture.origHeight) {
65+
if (capture.session != lastSentState.sessionId || capture.origHeight != lastSentState.height || capture.origWidth != lastSentState.width) {
6266
// we need to send a full capture if the session id changes or there is a resize/orientation change
6367
sendCaptureFull(capture)
6468
} else {
@@ -86,21 +90,21 @@ class RRwebGraphQLReplayLogExporter(
8690

8791
override fun flush(): CompletableResultCode {
8892
// TODO: O11Y-621 - Handle flush
89-
TODO("Not yet implemented")
93+
return CompletableResultCode.ofSuccess()
9094
}
9195

9296
override fun shutdown(): CompletableResultCode {
9397
// TODO: O11Y-621 - Handle shutdown
94-
TODO("Not yet implemented")
98+
return CompletableResultCode.ofSuccess()
9599
}
96100

97101
fun nextSid(): Int {
98-
sidCounter++;
102+
sidCounter++
99103
return sidCounter
100104
}
101105

102106
fun nextPayloadId(): Int {
103-
payloadIdCounter++;
107+
payloadIdCounter++
104108
return payloadIdCounter
105109
}
106110

@@ -141,7 +145,7 @@ class RRwebGraphQLReplayLogExporter(
141145
val incrementalEvent = Event(
142146
type = EventType.INCREMENTAL_SNAPSHOT,
143147
timestamp = timestamp,
144-
_sid = nextSid(),
148+
sid = nextSid(),
145149
data = EventDataUnion.CustomEventDataWrapper(
146150
Json.parseToJsonElement("""{"source":9,"id":6,"type":0,"commands":[{"property":"clearRect","args":[0,0,${capture.origWidth},${capture.origHeight}]},{"property":"drawImage","args":[{"rr_type":"ImageBitmap","args":[{"rr_type":"Blob","data":[{"rr_type":"ArrayBuffer","base64":"${capture.imageBase64}"}],"type":"image/jpeg"}]},0,0,${capture.origWidth},${capture.origHeight}]}]}""")
147151
)
@@ -154,7 +158,7 @@ class RRwebGraphQLReplayLogExporter(
154158
Event(
155159
type = EventType.INCREMENTAL_SNAPSHOT,
156160
timestamp = timestamp,
157-
_sid = nextSid(),
161+
sid = nextSid(),
158162
data = EventDataUnion.CustomEventDataWrapper(
159163
Json.parseToJsonElement("""{"source":2,"type":2,"x":1, "y":1}""")
160164
)
@@ -164,10 +168,8 @@ class RRwebGraphQLReplayLogExporter(
164168
replayApiService.pushPayload(capture.session, "${nextPayloadId()}", eventsBatch)
165169

166170
// record last sent state only after successful completion
167-
lastSessionId = capture.session
168-
lastSentWidth = capture.origWidth
169-
lastSentHeight = capture.origHeight
170-
171+
lastSentState = LastSentState(sessionId = capture.session, height = capture.origHeight, width = capture.origWidth)
172+
171173
true
172174
} catch (e: Exception) {
173175
// TODO: O11Y-627 - pass in logger to implementation and use here
@@ -199,7 +201,7 @@ class RRwebGraphQLReplayLogExporter(
199201
val metaEvent = Event(
200202
type = EventType.META,
201203
timestamp = timestamp,
202-
_sid = nextSid(),
204+
sid = nextSid(),
203205
data = EventDataUnion.StandardEventData(
204206
EventData(
205207
width = capture.origWidth,
@@ -212,7 +214,7 @@ class RRwebGraphQLReplayLogExporter(
212214
val snapShotEvent = Event(
213215
type = EventType.FULL_SNAPSHOT,
214216
timestamp = timestamp,
215-
_sid = nextSid(),
217+
sid = nextSid(),
216218
data = EventDataUnion.StandardEventData(
217219
EventData(
218220
node = EventNode(
@@ -267,7 +269,7 @@ class RRwebGraphQLReplayLogExporter(
267269
val viewportEvent = Event(
268270
type = EventType.CUSTOM,
269271
timestamp = timestamp,
270-
_sid = nextSid(),
272+
sid = nextSid(),
271273
data = EventDataUnion.CustomEventDataWrapper(
272274
Json.parseToJsonElement("""{"tag":"Viewport","payload":{"width":${capture.origWidth},"height":${capture.origHeight},"availWidth":${capture.origWidth},"availHeight":${capture.origHeight},"colorDepth":30,"pixelDepth":30,"orientation":0}}""")
273275
)
@@ -278,9 +280,7 @@ class RRwebGraphQLReplayLogExporter(
278280
replayApiService.pushPayload(capture.session, "${nextPayloadId()}", eventBatch)
279281

280282
// record last sent state only after successful completion
281-
lastSessionId = capture.session
282-
lastSentWidth = capture.origWidth
283-
lastSentHeight = capture.origHeight
283+
lastSentState = LastSentState(sessionId = capture.session, height = capture.origHeight, width = capture.origWidth)
284284

285285
true
286286
} catch (e: Exception) {

‎sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/ReplayInstrumentation.kt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,13 @@ private const val BATCH_MAX_EXPORT_SIZE = 10
6565
*/
6666
class ReplayInstrumentation(
6767
private val options: ReplayOptions = ReplayOptions(),
68-
) : AndroidInstrumentation, LDExtendedInstrumentation {
68+
) : LDExtendedInstrumentation {
6969

7070
private lateinit var _otelLogger: Logger
7171
private lateinit var _captureSource: CaptureSource
7272

7373
private var _captureJob: Job? = null
74-
private val _isPaused = AtomicBoolean(false)
74+
private var _isPaused: Boolean = false
7575
private val _captureMutex = Mutex()
7676

7777
override val name: String = INSTRUMENTATION_SCOPE_NAME
@@ -105,12 +105,12 @@ class ReplayInstrumentation(
105105
suspend fun runCapture() {
106106
_captureMutex.withLock {
107107
// If already running (not paused), do nothing
108-
if (!_isPaused.get()) {
108+
if (!_isPaused) {
109109
return
110110
}
111111

112112
// Clear paused flag and start/resume periodic capture
113-
_isPaused.set(false)
113+
_isPaused = false
114114
internalStartCapture()
115115
}
116116
}
@@ -119,12 +119,12 @@ class ReplayInstrumentation(
119119
suspend fun pauseCapture() {
120120
_captureMutex.withLock {
121121
// if already paused, do nothing
122-
if (_isPaused.get()) {
122+
if (_isPaused) {
123123
return
124124
}
125125

126126
// pause the periodic capture by terminating the job
127-
_isPaused.set(true)
127+
_isPaused = true
128128
_captureJob?.cancel()
129129
_captureJob = null
130130
}

0 commit comments

Comments
 (0)