Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
|
This is a real pain point for us at WeWard, with a significant performance impact. We patched it locally and saw immediate benefits. |
yienyien
marked this pull request as ready for review
October 12, 2026 14:51
This branch has not been deployed
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
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.
Summary
LocalEvaluationClient.evaluate_v2logs the whole flag config snapshot at DEBUG on every call:The f-string is built before the logger checks its level, so every evaluation renders every flag config to text, even when only one flag is requested and even when DEBUG is disabled. DEBUG is disabled by default:
LocalEvaluationConfigsets the default logger toWARNINGunlessdebug=True. The cost therefore grows with the number of flags in the project, not with the number of flags evaluated, and it is paid on the hot path of every server-side evaluation for a log line that is then thrown away.Observed impact
Measured on this branch's base (
main, 1.13.0) with 100 synthetic flags (two segments each, one condition, one bucket) in storage, evaluating a single flag with the default logger (WARNING), best of 5 × 5,000 calls on Python 3.14:evaluate_v2(user, {'flag-7'})mainAbout 99% of the evaluation time went into building a log message that was then discarded. We found this in production, where the repr was the largest cost of local evaluation in our request handlers. We currently work around it by swapping the storage's snapshot for a dict subclass with a constant-time
__repr__. We would like to drop that workaround.Change
The two
logger.debugcalls inevaluate_v2now use the standardlogginglazy formatting ("... %s", arg) instead of f-strings, so the arguments are only rendered when a handler actually emits the record. With DEBUG enabled, the output is identical:%sand an f-string placeholder both callstr().The other f-string debug calls in the SDK (
flag_config_updater,cohort_download_api,remote/client) run once per poll or per network fetch, not per evaluation, so this PR leaves them as they are to keep the diff minimal. Happy to convert them too if you prefer one style throughout.Tests (
tests/local/evaluate_logging_test.py, no network, client never started):WARNING,evaluate_v2evaluates correctly without rendering any flag config (EvaluationFlag.__repr__patched to raise). This test fails onmain.DEBUG, both records are still emitted with the same content (user JSON, every flag config, result).The 4
setUpClasserrors intests/local/client_test.pyandtests/local/client_eu_test.py(KeyError: 'API_KEY'/'EU_API_KEY') occur identically on unmodifiedmainin my environment. They need the CI secrets.Checklist
🤖 Generated with Claude Code
Note
Low Risk
Logging-only change on the evaluation path; DEBUG output is unchanged when enabled, and behavior is covered by new unit tests.
Overview
evaluate_v2no longer builds DEBUG log strings on every call. The twologger.debuglines inLocalEvaluationClient.evaluate_v2switch from f-strings to lazy%sformatting, souser, the full flag config snapshot, and results are only stringified when DEBUG is actually enabled.New tests in
evaluate_logging_test.pyassert that with the logger at WARNING, evaluation still works without callingEvaluationFlag.__repr__, and that with DEBUG on the same two log records still appear with the expected content.Reviewed by Cursor Bugbot for commit 8138f48. Bugbot is set up for automated code reviews on this repo. Configure here.