Skip to content

fix: route timestamps through PostHogDateProvider #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ioannisj wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: route timestamps through PostHogDateProvider #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cdc1871
fix: route remaining timestamps through PostHogDateProvider
ioannisj Mar 16, 2026
9609340
chore: add changeset
ioannisj Mar 16, 2026
be9e93b
Merge branch 'main' into fix/date-provider
ioannisj Mar 16, 2026
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
5 changes: 5 additions & 0 deletions .changeset/vast-sheep-vanish.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"posthog": patch
---

fix: route all timestamps through PostHogDateProvider
4 changes: 4 additions & 0 deletions posthog/src/main/java/com/posthog/PostHog.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ public class PostHog private constructor(
this.queue = queue
this.replayQueue = replayQueue

TimeBasedEpochGenerator.customTimeProvider = config.dateProvider::currentTimeMillis
Copy link
Member

@marandaneto marandaneto Mar 16, 2026

Choose a reason for hiding this comment

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

if you do this, then you dont need that


if (featureFlags is PostHogRemoteConfig) {
config.remoteConfigHolder = featureFlags
}
Expand Down Expand Up @@ -298,6 +300,8 @@ public class PostHog private constructor(
featureFlagsCalled.clear()

endSession()

TimeBasedEpochGenerator.customTimeProvider = null
} catch (e: Throwable) {
config?.logger?.log("Close failed: $e.")
}
Expand Down
2 changes: 1 addition & 1 deletion posthog/src/main/java/com/posthog/PostHogStateless.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public open class PostHogStateless protected constructor(
event,
distinctId,
properties = sanitizedProperties,
timestamp = timestamp ?: Date(),
timestamp = timestamp ?: config?.dateProvider?.currentDate() ?: Date(),
)
var eventChecked: PostHogEvent? = postHogEvent

Expand Down
7 changes: 6 additions & 1 deletion posthog/src/main/java/com/posthog/vendor/uuid/TimeBasedEpochGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ internal object TimeBasedEpochGenerator {
private val numberGenerator: Random = SecureRandom()
private val lock: Lock = ReentrantLock()

private val defaultTimeProvider: () -> Long = { System.currentTimeMillis() }

@Volatile
var customTimeProvider: (() -> Long)? = null
Comment on lines +27 to +30
Copy link
Member

@marandaneto marandaneto Mar 16, 2026

Choose a reason for hiding this comment

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

i'd rather keep just one, and non nullable, that can be replaced with a setter instead of having both custom + default and one nullable


/*
/ **********************************************************************
/ * UUID generation
Expand All @@ -34,7 +39,7 @@ internal object TimeBasedEpochGenerator {
* @return unix epoch time based UUID
*/
fun generate(): UUID {
Copy link
Member

@marandaneto marandaneto Mar 16, 2026

Choose a reason for hiding this comment

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

i think generate() could take the provider or even the timestamp as a param so its easier to test/debug and no need to use a lambda function?

Copy link
Collaborator Author

@ioannisj ioannisj Mar 16, 2026

Choose a reason for hiding this comment

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

yeah, that would be cleaner if all caller paths have access to the config. will check

Copy link
Collaborator Author

@ioannisj ioannisj Mar 16, 2026

Choose a reason for hiding this comment

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

Looks like this is used in a couple of places were we don't have a config object

  • PostHogEvent constructor. We could just pass uuid from buildEvent() so this would be easy to fix
  • PostHogSessionManger. This is a singleton and we'll need to pass time on startSession() call. This would need changes in rn plugin as well, or if we want to avoid that, we do this dance of setting/unsetting the provider/config like we do with TimeBasedEpochGenerator anw.

So I would say it would be easier for this to stay as-is. wdyt?

Copy link
Member

@marandaneto marandaneto Mar 16, 2026

Choose a reason for hiding this comment

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

wdyt about #463 ?
very similar but earlier and we dont to change anything in the posthog class
your call here to keep yours

return generate(System.currentTimeMillis())
return generate((customTimeProvider ?: defaultTimeProvider)())
}

/**
Expand Down
Loading