Skip to content

perf(feature_flags): validate schema once per fetched document - #8431

Merged
leandrodamascena merged 3 commits into
developfrom
perf/validate-schema-once-per-document
Sep 14, 2026
Merged

leandrodamascena merged 3 commits into
developfrom
perf/validate-schema-once-per-document

Conversation

@dreamorosi

Copy link
Copy Markdown
Contributor

Issue number: closes #8426

Summary

Changes

FeatureFlags.get_configuration() now skips schema validation when the store hands back the same document object it validated last time. A new document is always validated, so the "always validate fresh configuration" guarantee is unchanged; the only work removed is repeat validation of an unchanged, cached document.

The check is object identity (config is self._last_validated_config). The Parameters cache stores the transformed dict and returns that same object on every hit until max_age expires (parameters/base.py, fetch_from_cache), so identity is a faithful signal for "same fetch". FeatureFlags keeps a strong reference to the last validated document, so its id() cannot be recycled by an unrelated object. Third-party StoreProvider implementations that return a fresh dict per call get today's behaviour: validation on every call.

AppConfigStore with an envelope would have defeated the check on its own, since the JMESPath query produces a new object each call. The store now memoises the extracted result against the raw document's identity, so envelope users get the same benefit.

Failed validation is not cached: _last_validated_config is only assigned after validate() returns, so an invalid document raises on every call as before.

Tests cover: one validation across five evaluate calls plus get_enabled_features; re-validation when the store returns a new document and no re-validation when it returns the same one again; invalid documents raising on every call and not poisoning the cache; and the envelope path returning the same extracted object and validating once.

User experience

No API or behavioural change for evaluate, get_enabled_features, or get_configuration. Return values are identical.

For a handler evaluating N flags per invocation against a cached document, schema validation runs once per cache refresh instead of N times per invocation. The saving scales with document size (features x rules x conditions).


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

FeatureFlags.get_configuration built a SchemaValidator and walked the
whole document on every call. evaluate and get_enabled_features both call
it, so a handler evaluating five flags validated the full document five
times per invocation, even when the store served it from cache.

Skip validation when the store returns the same dict object that was
last validated. The Parameters cache hands back the same object until
expiry, so identity is a reliable signal for a cache hit, and a fresh
document is always validated. A strong reference to the last validated
document is kept so its id() cannot be recycled.

AppConfigStore with an envelope produced a new object per call from the
JMESPath query, which would defeat the check. Memoise the extraction on
the raw document's identity so envelope users benefit as well.

Closes #8426
@boring-cyborg boring-cyborg Bot added the tests label Sep 3, 2026
@powertools-for-aws-oss-automation powertools-for-aws-oss-automation Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 3, 2026
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.65%. Comparing base (de4af6b) to head (9b5aa34).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #8431   +/-   ##
========================================
  Coverage    96.65%   96.65%           
========================================
  Files          296      296           
  Lines        14885    14896   +11     
  Branches      1263     1265    +2     
========================================
+ Hits         14387    14398   +11     
  Misses         363      363           
  Partials       135      135           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dreamorosi
dreamorosi marked this pull request as ready for review September 3, 2026 16:46
@dreamorosi
dreamorosi requested a review from a team as a code owner September 3, 2026 16:46
@dreamorosi
dreamorosi requested review from hjgraca and leandrodamascena and removed request for hjgraca September 3, 2026 16:46
@leandrodamascena

Copy link
Copy Markdown
Contributor

I had the feeling this assumes the returned object is immutable, so I checked that case.

The standard AppConfigStore creates a new dictionary after a refresh, so this works fine. It only becomes a problem if someone changes the returned dictionary manually, or if a custom store updates the same dictionary in place.

I am fine considering this incorrect usage. The returned configuration must be treated as read-only, and custom stores must return a new dictionary when the configuration changes. I will add a commit to make this clear in the documentation.

@boring-cyborg boring-cyborg Bot added the documentation Improvements or additions to documentation label Sep 14, 2026
@sonarqubecloud

Copy link
Copy Markdown

@leandrodamascena leandrodamascena left a comment

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.

Thanks for working on this @dreamorosi! APPROVED

@leandrodamascena
leandrodamascena merged commit 23017e0 into develop Sep 14, 2026
17 checks passed
@leandrodamascena
leandrodamascena deleted the perf/validate-schema-once-per-document branch September 14, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tech debt: Feature Flags validates the full schema on every evaluate(), including cache hits

2 participants