From 731327ac3e9a0fd0e4b794ec9b7ca29c38114c89 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 12 May 2022 14:47:25 -0700 Subject: [PATCH 1/2] add DeciderClient & return Decider w/ empty context if no span --- reddit_decider/__init__.py | 50 +++++++++++++++++++++++++++++++++++++- tests/decider_tests.py | 13 ++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/reddit_decider/__init__.py b/reddit_decider/__init__.py index f461e28..894fe43 100644 --- a/reddit_decider/__init__.py +++ b/reddit_decider/__init__.py @@ -649,12 +649,22 @@ def make_object_for_context(self, name: str, span: Span) -> Decider: validate_decider(decider) + if span is None: + return Decider( + decider_context=DeciderContext(user_id=""), + config_watcher=self._filewatcher, + server_span=span, + context_name=name, + event_logger=self._event_logger, + ) + try: request = span.context ec = request.edge_context except Exception as exc: logger.info(f"Unable to access `request.edge_context` in `make_object_for_context()`. details: {exc}") + parsed_extracted_fields = {} try: if self._request_field_extractor: extracted_fields = self._request_field_extractor(request) @@ -752,6 +762,44 @@ def make_object_for_context(self, name: str, span: Span) -> Decider: event_logger=self._event_logger, ) + +class DeciderClient(config.Parser): + """Configure a decider client. + + This is meant to be used with + :py:meth:`baseplate.Baseplate.configure_context`. + + See :py:func:`decider_client_from_config` for available configuration settings. + + :param event_logger: The EventLogger instance to be used to log bucketing events. + + :param prefix: the prefix used to filter config keys (defaults to "experiments."). + + :param request_field_extractor: (optional) function used to populate fields such as + "app_name" & "build_number" in DeciderContext() via `extracted_fields` arg + """ + + def __init__( + self, + event_logger: EventLogger, + prefix: str = "experiments.", + request_field_extractor: Optional[Callable[[RequestContext], Dict[str, str]]] = None + ): + self._prefix = prefix + self._event_logger = event_logger + self._request_field_extractor = request_field_extractor + + def parse(self, _key_path: str, raw_config: config.RawConfig) -> DeciderContextFactory: + # `_key_path` is ignored for prefix because most services will not change `app_config` + # to use "decider" key, so using `prefix` from `__init__` + return decider_client_from_config( + app_config=raw_config, + event_logger=self._event_logger, + prefix=self._prefix, + request_field_extractor=self._request_field_extractor + ) + + def decider_client_from_config( app_config: config.RawConfig, event_logger: EventLogger, @@ -774,7 +822,7 @@ def decider_client_from_config( ``backoff`` (optional) The base amount of time for exponential backoff when trying to find the experiments config file. Defaults to no backoff between tries. - ``request_field_extractor`` (optional) used to populate fields such as + ``request_field_extractor`` (optional) function used to populate fields such as "app_name" & "build_number" in DeciderContext() via `extracted_fields` arg :param raw_config: The application configuration which should have diff --git a/tests/decider_tests.py b/tests/decider_tests.py index 676f36f..2b19573 100644 --- a/tests/decider_tests.py +++ b/tests/decider_tests.py @@ -169,6 +169,19 @@ def test_make_object_for_context_and_decider_context(self, _filewatcher): self.assertEqual(decider_event_dict["canonical_url"], CANONICAL_URL) self.assertEqual(decider_event_dict["request"]["canonical_url"], CANONICAL_URL) + def test_make_object_for_context_and_decider_context_without_span(self, _filewatcher): + decider_ctx_factory = decider_client_from_config( + {"experiments.path": "/tmp/test", "experiments.timeout": "60 seconds"}, + self.event_logger, + prefix="experiments.", + request_field_extractor=decider_field_extractor, + ) + decider = decider_ctx_factory.make_object_for_context(name="test", span=None) + self.assertIsInstance(decider, Decider) + + decider_ctx_dict = decider._decider_context.to_dict() + self.assertEqual(decider_ctx_dict["user_id"], "") + def test_make_object_for_context_and_decider_context_with_broken_decider_field_extractor(self, _filewatcher): def broken_decider_field_extractor(_request: RequestContext): return { From 555fddba32e7754c498f770dccb4964bc5249a41 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 12 May 2022 15:05:50 -0700 Subject: [PATCH 2/2] add debug logger --- reddit_decider/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/reddit_decider/__init__.py b/reddit_decider/__init__.py index 894fe43..becb089 100644 --- a/reddit_decider/__init__.py +++ b/reddit_decider/__init__.py @@ -650,6 +650,7 @@ def make_object_for_context(self, name: str, span: Span) -> Decider: validate_decider(decider) if span is None: + logger.debug("`span` is `None` in reddit_decider `make_object_for_context()`.") return Decider( decider_context=DeciderContext(user_id=""), config_watcher=self._filewatcher,