Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/brave-owls-drift.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'posthog': patch
'posthog-android': patch
---

Keep `$recording_status`, the replay trigger statuses and the replay buffer length on every captured event. Add the remaining `$sdk_debug_*` session replay properties and `$sdk_debug_session_start` only to SDK events (names starting with `$`, excluding `$feature_flag_called` and `$snapshot`), at most once every 30 seconds. Remove `$sdk_debug_current_session_duration` and `$sdk_debug_replay_throttle_delay_ms`.
Original file line number Diff line number Diff line change
Expand Up @@ -2606,7 +2606,6 @@ public class PostHogReplayIntegration(

val captureMode = if (isScreenshotCapable) "screenshot" else "wireframe"
props["\$sdk_debug_replay_capture_mode"] = captureMode
props["\$sdk_debug_replay_throttle_delay_ms"] = config.sessionReplayConfig.throttleDelayMs
props["\$sdk_debug_replay_internal_buffer_length"] =
if (buffering) replayQueue?.bufferDepth ?: 0 else replayQueue?.size ?: 0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4385,7 +4385,7 @@ internal class PostHogReplayIntegrationTest {
val sut = getSut(config)

assertEquals("wireframe", sut.debugProperties()["\$sdk_debug_replay_capture_mode"])
assertEquals(1000L, sut.debugProperties()["\$sdk_debug_replay_throttle_delay_ms"])
assertFalse(sut.debugProperties().containsKey("\$sdk_debug_replay_throttle_delay_ms"))
}

@Test
Expand Down Expand Up @@ -4519,7 +4519,6 @@ internal class PostHogReplayIntegrationTest {
assertEquals("disabled", props["\$recording_status"])
assertFalse(props.containsKey("\$sdk_debug_replay_flush_hold_reason"))
assertEquals("wireframe", props["\$sdk_debug_replay_capture_mode"])
assertEquals(1000L, props["\$sdk_debug_replay_throttle_delay_ms"])
assertEquals(0, props["\$sdk_debug_replay_internal_buffer_length"])
} finally {
sut.uninstall()
Expand Down Expand Up @@ -4578,7 +4577,7 @@ internal class PostHogReplayIntegrationTest {
"linked_flag".takeIf { props["\$sdk_debug_replay_linked_flag_trigger_status"] == "trigger_pending" },
)
assertEquals(pending.ifEmpty { null }, props["\$sdk_debug_replay_pending_trigger_conditions"], "pending conditions vs statuses")
assertTrue(props.containsKey("\$sdk_debug_replay_capture_mode") && props.containsKey("\$sdk_debug_replay_throttle_delay_ms"))
assertTrue(props.containsKey("\$sdk_debug_replay_capture_mode"))
}
assertEquals("disabled", sut.debugProperties()["\$recording_status"])
assertFalse(sut.debugProperties().containsKey("\$sdk_debug_replay_flush_hold_reason"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@
},
"$lib": "posthog-android",
"$lib_version": "<sdk-version>",
"$is_identified": false,
"$process_person_profile": true,
"$session_id": "018bcfe5-687b-7abc-8def-0123456789ab",
"amount": 42.5,
"currency": "USD",
Expand All @@ -54,15 +52,13 @@
"quantity": 2
}
],
"$is_identified": false,
"$process_person_profile": true,
"$sdk_debug_pending_queue_size": "<pending-queue-size>",
"$recording_status": "disabled",
"$sdk_debug_replay_event_trigger_status": "trigger_disabled",
"$sdk_debug_replay_linked_flag_trigger_status": "trigger_disabled",
"$sdk_debug_replay_capture_mode": "wireframe",
"$sdk_debug_replay_throttle_delay_ms": 1000,
"$sdk_debug_replay_internal_buffer_length": 0,
"$sdk_debug_session_start": 1700000000123,
"$sdk_debug_current_session_duration": 0,
"$sdk_debug_pending_queue_size": "<pending-queue-size>"
"$sdk_debug_replay_internal_buffer_length": 0
},
"timestamp": "2023-11-14T22:13:20.123Z",
"uuid": "<uuid>"
Expand Down Expand Up @@ -100,19 +96,17 @@
},
"$lib": "posthog-android",
"$lib_version": "<sdk-version>",
"$is_identified": true,
"$process_person_profile": true,
"$session_id": "018bcfe5-687b-7abc-8def-0123456789ab",
"$anon_distinct_id": "anonymous-user",
"$is_identified": true,
"$process_person_profile": true,
"$sdk_debug_pending_queue_size": "<pending-queue-size>",
"$recording_status": "disabled",
"$sdk_debug_replay_event_trigger_status": "trigger_disabled",
"$sdk_debug_replay_linked_flag_trigger_status": "trigger_disabled",
"$sdk_debug_replay_capture_mode": "wireframe",
"$sdk_debug_replay_throttle_delay_ms": 1000,
"$sdk_debug_replay_internal_buffer_length": 0,
"$sdk_debug_session_start": 1700000000123,
"$sdk_debug_current_session_duration": 0,
"$sdk_debug_pending_queue_size": "<pending-queue-size>"
"$sdk_debug_session_start": 1700000000123
},
"timestamp": "2023-11-14T22:13:20.123Z",
"uuid": "<uuid>"
Expand Down Expand Up @@ -141,24 +135,20 @@
"$network_carrier": "Snapshot Telecom",
"$lib": "posthog-android",
"$lib_version": "<sdk-version>",
"$is_identified": true,
"$process_person_profile": true,
"$session_id": "018bcfe5-687b-7abc-8def-0123456789ab",
"$group_type": "company",
"$group_key": "posthog",
"$group_set": {
"industry": "analytics",
"employees": 100
},
"$is_identified": true,
"$process_person_profile": true,
"$sdk_debug_pending_queue_size": "<pending-queue-size>",
"$recording_status": "disabled",
"$sdk_debug_replay_event_trigger_status": "trigger_disabled",
"$sdk_debug_replay_linked_flag_trigger_status": "trigger_disabled",
"$sdk_debug_replay_capture_mode": "wireframe",
"$sdk_debug_replay_throttle_delay_ms": 1000,
"$sdk_debug_replay_internal_buffer_length": 0,
"$sdk_debug_session_start": 1700000000123,
"$sdk_debug_current_session_duration": 0,
"$sdk_debug_pending_queue_size": "<pending-queue-size>"
"$sdk_debug_replay_internal_buffer_length": 0
},
"timestamp": "2023-11-14T22:13:20.123Z",
"uuid": "<uuid>"
Expand Down Expand Up @@ -194,24 +184,20 @@
"$active_feature_flags": [
"snapshot-flag"
],
"$is_identified": true,
"$process_person_profile": true,
"$session_id": "018bcfe5-687b-7abc-8def-0123456789ab",
"$feature_flag": "snapshot-flag",
"$feature_flag_response": "snapshot-variant",
"$feature_flag_request_id": "flag-request-123",
"$feature_flag_id": 123,
"$feature_flag_version": 7,
"$feature_flag_reason": "",
"$is_identified": true,
"$process_person_profile": true,
"$sdk_debug_pending_queue_size": "<pending-queue-size>",
"$recording_status": "disabled",
"$sdk_debug_replay_event_trigger_status": "trigger_disabled",
"$sdk_debug_replay_linked_flag_trigger_status": "trigger_disabled",
"$sdk_debug_replay_capture_mode": "wireframe",
"$sdk_debug_replay_throttle_delay_ms": 1000,
"$sdk_debug_replay_internal_buffer_length": 0,
"$sdk_debug_session_start": 1700000000123,
"$sdk_debug_current_session_duration": 0,
"$sdk_debug_pending_queue_size": "<pending-queue-size>"
"$sdk_debug_replay_internal_buffer_length": 0
},
"timestamp": "2023-11-14T22:13:20.123Z",
"uuid": "<uuid>"
Expand Down Expand Up @@ -247,8 +233,6 @@
"$active_feature_flags": [
"snapshot-flag"
],
"$is_identified": true,
"$process_person_profile": true,
"$session_id": "018bcfe5-687b-7abc-8def-0123456789ab",
"$exception_level": "error",
"$exception_list": [
Expand Down Expand Up @@ -308,15 +292,13 @@
],
"handled": true,
"component": "checkout",
"$is_identified": true,
"$process_person_profile": true,
"$sdk_debug_pending_queue_size": "<pending-queue-size>",
"$recording_status": "disabled",
"$sdk_debug_replay_event_trigger_status": "trigger_disabled",
"$sdk_debug_replay_linked_flag_trigger_status": "trigger_disabled",
"$sdk_debug_replay_capture_mode": "wireframe",
"$sdk_debug_replay_throttle_delay_ms": 1000,
"$sdk_debug_replay_internal_buffer_length": 0,
"$sdk_debug_session_start": 1700000000123,
"$sdk_debug_current_session_duration": 0,
"$sdk_debug_pending_queue_size": "<pending-queue-size>"
"$sdk_debug_replay_internal_buffer_length": 0
},
"timestamp": "2023-11-14T22:13:20.123Z",
"uuid": "<uuid>"
Expand Down
110 changes: 96 additions & 14 deletions posthog/src/main/java/com/posthog/PostHog.kt
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,21 @@ import java.util.concurrent.Executors

private const val PUSH_NOTIFICATION_OPENED_EVENT = "\$push_notification_opened"
private const val MAX_DEBUG_ERROR_LENGTH = 500
private const val REPLAY_DEBUG_PROPERTIES_INTERVAL_MILLIS = 30_000L

// Kept on every event because the app reads them per event; the rest of the replay debug bundle is throttled.
private val REQUIRED_REPLAY_DEBUG_PROPERTY_KEYS =
setOf(
"\$recording_status",
"\$sdk_debug_replay_event_trigger_status",
"\$sdk_debug_replay_linked_flag_trigger_status",
"\$sdk_debug_replay_internal_buffer_length",
)

private fun isReplayDebugEvent(event: String): Boolean =
event.startsWith("\$") &&
event != PostHogEventName.FEATURE_FLAG_CALLED.event &&
event != PostHogEventName.SNAPSHOT.event

// A duplicate report of one tap arrives within the same launch: milliseconds after a warm tap, seconds
// after a cold start while the host's JS/Dart handlers register. Finite so that a re-send carrying no
Expand Down Expand Up @@ -99,6 +114,11 @@ public class PostHog private constructor(

private val featureFlagsCalledLock = Any()
private val cachedPersonPropertiesLock = Any()
private val replayDebugPropertiesLock = Any()

// Guarded by replayDebugPropertiesLock.
private var lastReplayDebugPropertiesAt: Long? = null
private var outstandingReplayDebugClaimAt: Long? = null

private var replayQueue: PostHogQueueInterface<PostHogEvent>? = null

Expand Down Expand Up @@ -570,6 +590,10 @@ public class PostHog private constructor(

featureFlagsCalled.clear()
lastScreenName = null
synchronized(replayDebugPropertiesLock) {
lastReplayDebugPropertiesAt = null
outstandingReplayDebugClaimAt = null
}

PostHogSessionManager.setOnSessionIdChangedListener(null)

Expand Down Expand Up @@ -669,6 +693,7 @@ public class PostHog private constructor(
appendSharedProps: Boolean = true,
appendGroups: Boolean = true,
timestamp: Date? = null,
claimReplayDebugBundle: () -> Boolean = { false },
): MutableMap<String, Any> {
val props = mutableMapOf<String, Any>()

Expand Down Expand Up @@ -762,7 +787,7 @@ public class PostHog private constructor(
// extend(properties, sdkDebugProperties). After session resolution so the debug snapshot
// never precedes a rotation triggered by getActiveSessionId() above.
if (appendSharedProps) {
props.putAll(sdkDebugProperties(sessionIdString, timestamp))
props.putAll(sdkDebugProperties(sessionIdString, timestamp, claimReplayDebugBundle))
}

// only Session replay needs distinct_id also in the props
Expand All @@ -779,6 +804,7 @@ public class PostHog private constructor(
private fun sdkDebugProperties(
sessionId: String?,
timestamp: Date?,
claimReplayDebugBundle: () -> Boolean,
): Map<String, Any> {
val props = mutableMapOf<String, Any>()
val managerStart = PostHogSessionManager.getSessionStartedAt()
Expand All @@ -787,17 +813,22 @@ public class PostHog private constructor(
if (timestamp != null && managerStart > 0 && timestamp.time < managerStart) {
return props
}
val claimed = claimReplayDebugBundle()
// Guarded separately from the session and queue keys below: those come from the session
// manager and the queue, not from replay, so a handler that throws must not suppress them.
try {
props.putAll(sessionReplayHandler?.debugProperties() ?: mapOf("\$recording_status" to "disabled"))
val replayProps = sessionReplayHandler?.debugProperties() ?: mapOf("\$recording_status" to "disabled")
if (claimed) {
props.putAll(replayProps)
} else {
REQUIRED_REPLAY_DEBUG_PROPERTY_KEYS.forEach { key -> replayProps[key]?.let { props[key] = it } }
}
} catch (e: Throwable) {
props["\$sdk_debug_error_capturing_properties"] = e.toString().take(MAX_DEBUG_ERROR_LENGTH)
}
try {
sessionStart(sessionId, managerStart)?.let { start ->
props["\$sdk_debug_session_start"] = start
props["\$sdk_debug_current_session_duration"] = PostHogSessionManager.currentTimeMillis() - start
if (claimed) {
sessionStart(sessionId, managerStart)?.let { props["\$sdk_debug_session_start"] = it }
}
queue?.size?.let { props["\$sdk_debug_pending_queue_size"] = it }
} catch (e: Throwable) {
Expand All @@ -806,6 +837,43 @@ public class PostHog private constructor(
return props
}

private fun currentTimeMillis(): Long = config?.dateProvider?.currentTimeMillis() ?: System.currentTimeMillis()

private fun isWithinReplayDebugInterval(
since: Long?,
now: Long,
): Boolean = since != null && now - since < REPLAY_DEBUG_PROPERTIES_INTERVAL_MILLIS

/**
* Single outstanding claim, so a capture inside `beforeSend` can't also take it; a claim older
* than the interval counts as leaked. The window starts at commit, on wall clock so a
* future-dated capture can't hold it shut.
*/
private fun claimReplayDebugPropertiesWindow(): Boolean {
synchronized(replayDebugPropertiesLock) {
val now = currentTimeMillis()
if (isWithinReplayDebugInterval(lastReplayDebugPropertiesAt, now) ||
isWithinReplayDebugInterval(outstandingReplayDebugClaimAt, now)
) {
return false
}
outstandingReplayDebugClaimAt = now
return true
}
}

private fun commitReplayDebugPropertiesWindow() {
synchronized(replayDebugPropertiesLock) {
lastReplayDebugPropertiesAt = currentTimeMillis()
}
}

private fun releaseReplayDebugPropertiesClaim() {
synchronized(replayDebugPropertiesLock) {
outstandingReplayDebugClaimAt = null
}
Comment on lines +871 to +874

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Older captures clear newer claims

releaseReplayDebugPropertiesClaim() clears the current claim without checking who owns it. If capture A stays in beforeSend for over 30 seconds, capture B can replace its expired claim. If A then returns null, its finally clears B's claim. Capture C can now claim while B is still running, letting B and C queue the full debug bundle within 30 seconds.

Give each claim an owner token and release it only when that token still owns the claim. Add a test for this overlap.

Knowledge Base Used: Event capture and delivery

Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/src/main/java/com/posthog/PostHog.kt
Line: 871-874

Comment:
**Older captures clear newer claims**

`releaseReplayDebugPropertiesClaim()` clears the current claim without checking who owns it. If capture A stays in `beforeSend` for over 30 seconds, capture B can replace its expired claim. If A then returns `null`, its `finally` clears B's claim. Capture C can now claim while B is still running, letting B and C queue the full debug bundle within 30 seconds.

Give each claim an owner token and release it only when that token still owns the claim. Add a test for this overlap.

**Knowledge Base Used:** [Event capture and delivery](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog-android/-/docs/event-capture-and-delivery.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

}

// The manager's clock for its own session. A caller-supplied id (the RN/Flutter bridges) carries
// its start in the UUIDv7 timestamp, the derivation posthog-js applies to a bootstrapped id.
private fun sessionStart(
Expand Down Expand Up @@ -848,6 +916,9 @@ public class PostHog private constructor(
groups: Map<String, String>?,
timestamp: Date?,
) {
// Tracked on this call rather than derived from the event, so it survives beforeSend renaming
// or replacing the event.
var claimedReplayDebugBundle = false
try {
if (!isEnabled()) {
return
Expand Down Expand Up @@ -917,6 +988,9 @@ public class PostHog private constructor(
// only append groups if not a group identify event and not a snapshot
appendGroups = !groupIdentify,
timestamp = timestamp,
claimReplayDebugBundle = {
isReplayDebugEvent(event) && claimReplayDebugPropertiesWindow().also { claimedReplayDebugBundle = it }
},
)

val postHogEvent = buildEvent(event, newDistinctId, mergedProperties, timestamp)
Expand All @@ -942,22 +1016,30 @@ public class PostHog private constructor(
if (isSnapshotEvent) {
replayQueue?.add(postHogEvent)
} else {
super.captureStateless(
postHogEvent.event,
newDistinctId,
postHogEvent.properties ?: emptyMap(),
userProperties,
userPropertiesSetOnce,
groups,
timestamp,
)
val queued =
captureStatelessInternal(
postHogEvent.event,
newDistinctId,
postHogEvent.properties ?: emptyMap(),
userProperties,
userPropertiesSetOnce,
groups,
timestamp,
)
if (queued && claimedReplayDebugBundle) {
commitReplayDebugPropertiesWindow()
}
// Notify surveys integration about the event
surveysHandler?.onEvent(event, mergedProperties)
// Notify session replay handler about the event for event triggers
sessionReplayHandler?.onEvent(event, mergedProperties)
}
} catch (e: Throwable) {
config?.logger?.log("Capture failed: $e.")
} finally {
if (claimedReplayDebugBundle) {
releaseReplayDebugPropertiesClaim()
}
}
}

Expand Down
Loading
Loading