Skip to content

Conversation

@haacked
Copy link
Contributor

@haacked haacked commented Dec 3, 2025

Add support for HTTP conditional requests using ETags to reduce bandwidth when polling for feature flag definitions. When flag definitions haven't changed, the server returns 304 Not Modified and the SDK skips processing.

Based on PostHog/posthog-python#381

Add ETag-based caching to reduce bandwidth and server load when polling
for feature flag definitions. The server can now return 304 Not Modified
when flags haven't changed, avoiding unnecessary data transfer.

- Store ETag from feature flag definitions responses
- Send If-None-Match header on subsequent requests
- Handle 304 Not Modified by preserving cached flags
- Add helper to mask tokens in URLs for debug logging
Replace plain @flags_etag instance variable with Concurrent::AtomicReference
to ensure thread-safe access from background polling thread. This aligns with
the existing pattern used for @quota_limited (AtomicBoolean) and other
concurrent state in FeatureFlagsPoller.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds HTTP conditional request support using ETags to optimize bandwidth when polling for feature flag definitions. The implementation stores the ETag from successful responses and sends it with subsequent requests via the If-None-Match header. When the server responds with 304 Not Modified, the SDK skips reprocessing and preserves cached flag data.

Key changes:

  • Added thread-safe ETag storage using Concurrent::AtomicReference
  • Modified the HTTP request/response flow to handle 304 Not Modified responses
  • Added token masking utility for secure logging

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/posthog/feature_flags.rb Added ETag storage (@flags_etag), request/response handling for 304 status codes, If-None-Match header support, and _mask_tokens_in_url helper for secure logging
spec/posthog/flags_spec.rb Updated existing tests to expect etag: nil in responses and added comprehensive test coverage for ETag functionality including storage, 304 handling, ETag updates, and token masking

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Move ETag update logic inside the successful response block so that
error responses (400, 500, etc.) don't clear the cached ETag. This
ensures subsequent requests can still use If-None-Match for conditional
requests after transient server errors.

Address review feedback from PR #84.
@haacked haacked requested a review from Copilot December 3, 2025 16:34
@haacked haacked marked this pull request as ready for review December 3, 2025 16:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Replace hardcoded "GET" with request_object.method for accuracy.
Add include_etag parameter to _request method to control whether ETag
is included in responses. Only the feature flag definitions endpoint
needs ETag support for conditional requests, so other endpoints no
longer include unnecessary etag: nil in their responses.
Add an example script that demonstrates ETag support for local evaluation
polling. The script polls every 5 seconds with debug logging enabled,
showing ETag headers and 304 Not Modified responses.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-project-automation github-project-automation bot moved this from In Review to Approved in Feature Flags Dec 4, 2025
@haacked haacked merged commit ef53a8d into main Dec 4, 2025
11 checks passed
@haacked haacked deleted the haacked/etag-support branch December 4, 2025 19:28
@github-project-automation github-project-automation bot moved this from Approved to Done in Feature Flags Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants