Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
3 changes: 3 additions & 0 deletions openlibrary/asgi_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware

import infogami
from openlibrary.fastapi.middleware.experiments import ABTestingMiddleware
from openlibrary.utils.request_context import set_context_from_fastapi
from openlibrary.utils.sentry import Sentry, init_sentry

Expand Down Expand Up @@ -179,6 +180,8 @@ def create_app() -> FastAPI | None:
# Needed for the staging nginx proxy
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*")

app.add_middleware(ABTestingMiddleware)

@app.middleware("http")
async def add_fastapi_header(request: Request, call_next):
"""Middleware to add a header indicating the response came from FastAPI."""
Expand Down
78 changes: 78 additions & 0 deletions openlibrary/core/experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import hashlib
from typing import Final, Literal, TypedDict


class Audience:
"""Constants for experiment audience targeting."""

ALL: Final = "all"
LOGGED_IN: Final = "logged_in"
LOGGED_OUT: Final = "logged_out"


class ExperimentConfig(TypedDict, total=False):
"""Type schema for autocomplete and static analysis."""

variants: dict[str, int]
audience: Literal["all", "logged_in", "logged_out"]


ACTIVE_EXPERIMENTS: dict[str, ExperimentConfig] = {
"Sample_Experiment_1": {
"variants": {"control": 30, "a": 35, "b": 35},
"audience": Audience.ALL,
},
"Sample_Experiment_2": {
"variants": {"control": 50, "treatment": 50},
"audience": Audience.LOGGED_OUT,
},
"Sample_Experiment_3": {
"variants": {"control": 50, "treatment": 50},
"audience": Audience.LOGGED_IN,
},
}


def get_variant(experiment_name: str, user_identifier: str | None, is_logged_in: bool = False) -> str:
"""Assigns a user to a variant based on configured weights and audience rules."""
if not user_identifier or experiment_name not in ACTIVE_EXPERIMENTS:
return "control"

config = ACTIVE_EXPERIMENTS[experiment_name]
audience = config.get("audience", Audience.ALL)

if audience == Audience.LOGGED_IN and not is_logged_in:
return "control"
if audience == Audience.LOGGED_OUT and is_logged_in:
return "control"

hash_key = f"{experiment_name}-{user_identifier}".encode()
bucket = int(hashlib.md5(hash_key).hexdigest()[:8], 16) % 100

cumulative_weight = 0
for variant, weight in config.get("variants", {}).items():
Comment thread
Sadashii marked this conversation as resolved.
cumulative_weight += weight
if bucket < cumulative_weight:
return variant

return "control"


def get_user_experiments(
user_identifier: str | None,
overrides: dict[str, str] | None = None,
is_logged_in: bool = False,
) -> dict[str, str]:
"""Evaluates all active experiments for a user, handling optional overrides."""
overrides = overrides or {}
results = {}

for exp_name, config in ACTIVE_EXPERIMENTS.items():
override_val = overrides.get(f"experiment_{exp_name}")

if override_val in config.get("variants", {}):
results[exp_name] = override_val
else:
results[exp_name] = get_variant(exp_name, user_identifier, is_logged_in)

return results
39 changes: 39 additions & 0 deletions openlibrary/fastapi/middleware/experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
from fastapi import Request
from starlette.types import ASGIApp, Receive, Scope, Send

from infogami import config
from openlibrary.accounts.model import verify_session_cookie
from openlibrary.core.experiments import get_user_experiments


class ABTestingMiddleware:
def __init__(self, app: ASGIApp) -> None:
self.app = app

async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
if scope["type"] != "http":
await self.app(scope, receive, send)
return

request = Request(scope, receive=receive)

session_value = request.cookies.get(config.get("login_cookie_name", "session"))
is_authenticated = False
user_id = None

if session_value and session_value.startswith("/people/") and verify_session_cookie(session_value):
is_authenticated = True
user_id = session_value.split(",")[0]

if not user_id:
forwarded_for = request.headers.get("X-Forwarded-For")
if forwarded_for:
user_id = forwarded_for.split(",")[0].strip()
else:
user_id = request.client.host if request.client else "127.0.0.1"

query_overrides = {k: v for k, v in request.query_params.items() if k.startswith("experiment_")}

request.state.experiments = get_user_experiments(user_id, overrides=query_overrides, is_logged_in=is_authenticated)
Comment thread
Sadashii marked this conversation as resolved.
Outdated

await self.app(scope, receive, send)
9 changes: 7 additions & 2 deletions openlibrary/plugins/openlibrary/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,14 @@ def setup_contextvars(handler):
delegate.app.add_processor(processors.PreferenceProcessor())
# Refer to https://github.com/internetarchive/openlibrary/pull/10005 to force patron's to login
# delegate.app.add_processor(processors.RequireLogoutProcessor())
# IMPORTANT: setup_contextvars must run AFTER other processors but BEFORE the handler
# It's added here (not as a loadhook) so it runs in the same request context

# IMPORTANT: setup_contextvars must run AFTER other processors but BEFORE the handler.
# The only exception is ExperimentsProcessor, which needs to run after setup_contextvars
# so that request ContextVars (like `site` and `req_context`) are fully initialized,
# allowing `get_current_user()` to function correctly.
# It's added here (not as a loadhook) so it runs in the same request context.
Comment thread
Sadashii marked this conversation as resolved.
Outdated
delegate.app.add_processor(setup_contextvars)
delegate.app.add_processor(processors.ExperimentsProcessor())

Comment thread
Sadashii marked this conversation as resolved.
Outdated
try:
from infogami.plugins.api import code as api
Expand Down
10 changes: 10 additions & 0 deletions openlibrary/plugins/openlibrary/js/experiments.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export function getExperiment(experimentName) {
if (typeof window === 'undefined') {
return 'control';
}
return window.OL_EXPERIMENTS?.[experimentName] || 'control';
}

if (typeof window !== 'undefined') {
window.getExperiment = getExperiment;
}
1 change: 1 addition & 0 deletions openlibrary/plugins/openlibrary/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { exposeGlobally } from './jsdef';
import initAnalytics from './ol.analytics';
import init from './ol.js';
import initServiceWorker from './service-worker-init.js';
import './experiments.js';
import '../../../../static/css/js-all.css';
// polyfill Promise support for IE11
import Promise from 'promise-polyfill';
Expand Down
36 changes: 36 additions & 0 deletions openlibrary/plugins/openlibrary/processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import web

from infogami import config
from infogami.utils.context import context
from openlibrary.accounts import get_current_user
from openlibrary.core import helpers as h
from openlibrary.core.experiments import get_user_experiments
from openlibrary.core.processors import (
ReadableUrlProcessor, # noqa: F401 side effects may be needed
)
Expand Down Expand Up @@ -156,6 +158,40 @@ def __call__(self, handler):
return handler()


class ExperimentsProcessor:
"""Processor to evaluate A/B testing feature flags and inject them
into the global web.ctx so they are available to macros and templates.
"""

def __call__(self, handler):
session_value = web.cookies().get(config.get("login_cookie_name", "session"))
is_authenticated = False
user_id = None

if session_value and session_value.startswith("/people/"):
from openlibrary.accounts.model import verify_session_cookie

if verify_session_cookie(session_value):
is_authenticated = True
user_id = session_value.split(",")[0]
Comment thread
Sadashii marked this conversation as resolved.
Outdated

if not user_id:
forwarded_for = web.ctx.env.get("HTTP_X_FORWARDED_FOR")
user_id = forwarded_for.split(",")[0].strip() if forwarded_for else web.ctx.ip

try:
overrides = {k: v for k, v in web.input(_method="GET").items() if k.startswith("experiment_")}
except AttributeError, KeyError, ValueError:
Comment thread
Sadashii marked this conversation as resolved.
overrides = {}

experiments = get_user_experiments(user_id, overrides=overrides, is_logged_in=is_authenticated)

web.ctx["experiments"] = experiments
context["experiments"] = experiments

return handler()


if __name__ == "__main__":
import doctest

Expand Down
5 changes: 5 additions & 0 deletions openlibrary/templates/site/head.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@
}
</style>
</noscript>

<script>
window.OL_EXPERIMENTS = $:dumps(ctx.get("experiments", {}));
</script>
Comment thread
Sadashii marked this conversation as resolved.

<script>
$if not is_bot() and not ctx.get('disable_analytics'):
// Dimension 1 (visit-scoped): "Days Since Registration" — ID must match Matomo admin config (issue #12144)
Expand Down
150 changes: 150 additions & 0 deletions openlibrary/tests/core/test_experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
from openlibrary.core.experiments import ACTIVE_EXPERIMENTS, Audience, get_user_experiments, get_variant


def test_get_variant_fallback():
# Empty or None user identifier should fall back to control
assert get_variant("Sample_Experiment_1", "") == "control"
assert get_variant("Sample_Experiment_1", None) == "control"

# Non-existent experiment should fall back to control
assert get_variant("Non_Existent", "user123") == "control"


def test_get_variant_distribution():
# Set up a test experiment
ACTIVE_EXPERIMENTS["Test_Distribution"] = {
"variants": {
"control": 30,
"a": 35,
"b": 35,
},
"audience": Audience.ALL,
}
try:
Comment thread
Sadashii marked this conversation as resolved.
Outdated
variants_seen = set()
for i in range(100):
variant = get_variant("Test_Distribution", f"user_{i}")
assert variant in ACTIVE_EXPERIMENTS["Test_Distribution"]["variants"]
variants_seen.add(variant)

# Ensure all variants are allocated at least once (probabilistically guaranteed for N=100)
assert len(variants_seen) > 1
assert "control" in variants_seen
assert "a" in variants_seen
assert "b" in variants_seen
finally:
ACTIVE_EXPERIMENTS.pop("Test_Distribution", None)


def test_get_user_experiments():
ACTIVE_EXPERIMENTS["Test_User_Exp"] = {
"variants": {
"control": 50,
"treatment": 50,
},
"audience": Audience.ALL,
}
try:
experiments = get_user_experiments("user_456")
assert "Test_User_Exp" in experiments
assert experiments["Test_User_Exp"] in ACTIVE_EXPERIMENTS["Test_User_Exp"]["variants"]
finally:
ACTIVE_EXPERIMENTS.pop("Test_User_Exp", None)


def test_get_user_experiments_overrides():
ACTIVE_EXPERIMENTS["Test_Overrides"] = {
"variants": {
"control": 50,
"treatment": 50,
},
"audience": Audience.ALL,
}
try:
# Valid override should change the variant
overrides = {"experiment_Test_Overrides": "treatment"}
experiments = get_user_experiments("user_456", overrides=overrides)
assert experiments["Test_Overrides"] == "treatment"

# Invalid override group/variant should be ignored
overrides = {"experiment_Test_Overrides": "invalid_group"}
experiments = get_user_experiments("user_456", overrides=overrides)
assert experiments["Test_Overrides"] != "invalid_group"
assert experiments["Test_Overrides"] in ACTIVE_EXPERIMENTS["Test_Overrides"]["variants"]

# Invalid override key/name format should be ignored
overrides = {"Test_Overrides": "treatment"}
experiments = get_user_experiments("user_456", overrides=overrides)
assert experiments["Test_Overrides"] in ACTIVE_EXPERIMENTS["Test_Overrides"]["variants"]
finally:
ACTIVE_EXPERIMENTS.pop("Test_Overrides", None)


def test_audience_targeting_logged_in():
ACTIVE_EXPERIMENTS["Test_Logged_In"] = {
"variants": {
"control": 20,
"treatment": 80,
},
"audience": Audience.LOGGED_IN,
}
try:
# If not logged in, should always fall back to control
for i in range(100):
assert get_variant("Test_Logged_In", f"user_{i}", is_logged_in=False) == "control"

# If logged in, should distribute into variants
variants_seen = set()
for i in range(100):
variant = get_variant("Test_Logged_In", f"user_{i}", is_logged_in=True)
assert variant in ["control", "treatment"]
variants_seen.add(variant)

assert len(variants_seen) > 1
finally:
ACTIVE_EXPERIMENTS.pop("Test_Logged_In", None)


def test_audience_targeting_logged_out():
ACTIVE_EXPERIMENTS["Test_Logged_Out"] = {
"variants": {
"control": 20,
"treatment": 80,
},
"audience": Audience.LOGGED_OUT,
}
try:
# If logged in, should always fall back to control
for i in range(100):
assert get_variant("Test_Logged_Out", f"user_{i}", is_logged_in=True) == "control"

# If not logged in, should distribute into variants
variants_seen = set()
for i in range(100):
variant = get_variant("Test_Logged_Out", f"user_{i}", is_logged_in=False)
assert variant in ["control", "treatment"]
variants_seen.add(variant)

assert len(variants_seen) > 1
finally:
ACTIVE_EXPERIMENTS.pop("Test_Logged_Out", None)


def test_get_user_experiments_audience():
ACTIVE_EXPERIMENTS["Test_User_Exp_Audience"] = {
"variants": {
"control": 20,
"treatment": 80,
},
"audience": Audience.LOGGED_IN,
}
try:
# If not logged in, should get control
experiments = get_user_experiments("user_456", is_logged_in=False)
assert experiments["Test_User_Exp_Audience"] == "control"

# If logged in, should evaluate variant
experiments_logged_in = get_user_experiments("user_456", is_logged_in=True)
assert experiments_logged_in["Test_User_Exp_Audience"] in ["control", "treatment"]
finally:
ACTIVE_EXPERIMENTS.pop("Test_User_Exp_Audience", None)