feat(node): Add timestamp tracking for debugging local feature flag evaluation#3163
Merged
gustavohstrassburger merged 6 commits intomainfrom Feb 27, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Contributor
|
Size Change: +1.05 kB (+0.02%) Total Size: 6.69 MB
ℹ️ View Unchanged
|
dmarticus
reviewed
Feb 27, 2026
Contributor
There was a problem hiding this comment.
Okay, this is a valid problem and I'm glad you're tackling it. I am not sure it's exactly the right way to go about it though; here's what I'm thinking:
- The
evaluationTimestampCachedesign is awkward. The context is created fresh per-request, so the "cache" is set during evaluation and read moments later in the same call. It's not really caching anything
across requests - it's just a roundabout way to passDate.now()from the evaluation point to the event emission. - Over-engineering? For local evaluation, the flag is computed synchronously. The evaluation timestamp is essentially "now." Why not just capture
Date.now()directly when building the event properties, rather than storing it in a context object first? - The
flagDefinitionsLoadedAtpart makes sense - knowing when your cached definitions were fetched is useful for cache TTL management and debugging staleness issues. - The per-flag timestamp seems less useful - For remote evaluation,
evaluatedAtcomes from the server and tells you when the server made the decision (which could differ from request time due to network latency). For local evaluation, evaluation happens immediately when you call the method - the timestamp is always "right now."
A simpler approach might be:
- Keep
flagDefinitionsLoadedAt - Drop the
evaluationTimestampCachecomplexity and just use Date.now() inline when setting$feature_flag_evaluated_atfor locally evaluated flags (unless there's a specific use case where you evaluate a flag, then later send the event and need to preserve the original evaluation time – but that doesn't seem like the common pattern here)
Remove evaluationTimestampCache complexity and use inline Date.now() for local flag evaluation timestamps. The cache was unnecessary since evaluation happens synchronously and timestamps are used immediately. - Remove evaluationTimestampCache from FeatureFlagEvaluationContext - Remove getFlagEvaluatedAt method from FeatureFlagsPoller - Use Date.now() inline when setting $feature_flag_evaluated_at - Update tests to verify timestamp functionality via event capture - Keep flagDefinitionsLoadedAt for cache TTL management 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Contributor
Author
|
Hey @dmarticus, Appreciate the feedback, it actually made me realize that I got something wrong in terms of the flow of local eval. I'll simplify this PR with your suggestions. |
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Rename and update changeset to emphasize the new timestamp tracking feature for local evaluation rather than implementation details. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
gustavohstrassburger
changed the title
Change from minor to patch version bump and reframe as debugging improvement rather than major feature addition. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
gustavohstrassburger
changed the title
dmarticus
approved these changes
Feb 27, 2026
Contributor
dmarticus
left a comment
dmarticus
left a comment
There was a problem hiding this comment.
Looks good — this is a nice simplification from the earlier version. The inline Date.now() approach is much cleaner than the cache abstraction.
Two minor nits:
-
$feature_flag_evaluated_atgets set twice for local evaluations — It's first set toevaluatedAt(which isundefinedfor local evals) in the initial properties object, then overridden withDate.now()in the new block. This works but is slightly confusing to read. You could set it once:$feature_flag_evaluated_at: flagWasLocallyEvaluated ? Date.now() : evaluatedAt,
...and then the new
ifblock only needs to handle$feature_flag_definitions_loaded_at. Minor nit though, not blocking. -
flagDefinitionsLoadedAtsemantics when loading from cache —updateFlagStateis called from both the network fetch path and the cache load path. This means if definitions are loaded from cache,flagDefinitionsLoadedAtreflects when the cache was read, not when the definitions were originally fetched from the server. If the goal is debugging staleness, you might want the original fetch time instead. Worth a comment clarifying the intended semantics either way.
Neither of these is blocking — approving as-is.
Address Dylan's feedback: - Consolidate $feature_flag_evaluated_at setting using ternary operator - Fix flagDefinitionsLoadedAt to only reflect original server fetch time - Preserve cache load efficiency by not setting timestamp on cache hits 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
gustavohstrassburger
requested a deployment
to
NPM Release
GitHub Actions
Waiting
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.
Problem
Local feature flag evaluation in posthog-node was missing the timestamp tracking that's available in remote evaluation, making it difficult to debug timing-related issues when troubleshooting flag behavior.
When debugging cache TTL problems or investigating timing between flag definition loading and evaluation, developers had no visibility into:
Changes
Added simple timestamp tracking to improve debugging and observability for local feature flag evaluation:
flagDefinitionsLoadedAt)$feature_flag_calledevents for locally evaluated flagsThe implementation uses a straightforward approach with inline
Date.now()calls based on code review feedback, avoiding complex data structures while providing the necessary debugging information.This ensures locally evaluated flags include the same timestamp data as remotely evaluated flags in analytics events, making it easier to debug timing-related issues and maintain consistency between evaluation methods.
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file