Repository navigation
fix: flush feature flag telemetry at invocation end #872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5c81661
22bc92c
f6f08ba
5b47539
41477ba
69ed67a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,61 @@ | ||||||||||||||||||||||||||||||||||||||||
| """Compatibility boundary for invocation-end feature flag telemetry.""" | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||||||||
| import math | ||||||||||||||||||||||||||||||||||||||||
| from time import monotonic | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| from datadog_lambda.config import config | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||
| from ddtrace.internal.openfeature import _telemetry | ||||||||||||||||||||||||||||||||||||||||
| except ImportError: | ||||||||||||||||||||||||||||||||||||||||
| _telemetry = None | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||||||||
| SAFETY_MARGIN_MS = 100.0 | ||||||||||||||||||||||||||||||||||||||||
| MAX_CONSECUTIVE_TIMEOUTS = 3 | ||||||||||||||||||||||||||||||||||||||||
| TIMEOUT_BACKOFF_S = 60.0 | ||||||||||||||||||||||||||||||||||||||||
| # Warm invocations share this state; a new process starts with no cooldown. | ||||||||||||||||||||||||||||||||||||||||
| _consecutive_timeouts = 0 | ||||||||||||||||||||||||||||||||||||||||
| _skip_until = 0.0 | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| def flush_feature_flag_telemetry(context): | ||||||||||||||||||||||||||||||||||||||||
| global _consecutive_timeouts, _skip_until | ||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||
| if _telemetry is None: | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| if getattr(_telemetry, "FLUSH_API_VERSION", None) != 1: | ||||||||||||||||||||||||||||||||||||||||
| logger.debug("Skipping feature flag telemetry flush: unsupported tracer") | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| cap_ms = config.feature_flag_flush_timeout_ms | ||||||||||||||||||||||||||||||||||||||||
| if not math.isfinite(cap_ms) or cap_ms <= 0: | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| # No remaining-time information means no safe invocation budget. | ||||||||||||||||||||||||||||||||||||||||
|
vjfridge marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||
| get_remaining_ms = getattr(context, "get_remaining_time_in_millis", None) | ||||||||||||||||||||||||||||||||||||||||
| if get_remaining_ms is None: | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| budget_ms = min(cap_ms, get_remaining_ms() - SAFETY_MARGIN_MS) | ||||||||||||||||||||||||||||||||||||||||
| if budget_ms <= 0: | ||||||||||||||||||||||||||||||||||||||||
| logger.debug( | ||||||||||||||||||||||||||||||||||||||||
| "Skipping feature flag telemetry flush: no invocation time remains" | ||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| if monotonic() < _skip_until: | ||||||||||||||||||||||||||||||||||||||||
| logger.debug("Skipping feature flag telemetry flush: backing off") | ||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||
| outcome = _telemetry.flush(budget_ms / 1000.0) | ||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Comment from Claude working with Aaron Silverman: [P1] Back off after repeated timeouts that use the full cap. Without a backoff, a slow or unreachable intake adds up to the whole cap, 500 ms by default, to every invocation that has unsent flag events.
Datadog's serverless libraries already back off in this situation. datadogpy, which this wrapper uses to send metrics without the extension, stops sending for 300 s after 3 timeouts. datadog-lambda-go opens a circuit breaker for 60 s after more than 4 failures in 30 s. The suggested fix takes the threshold from datadogpy and the window from datadog-lambda-go: after 3 full-cap timeouts with no completed or failed flush between them, it skips the wait for 60 s. A timeout from a short remaining-time budget neither counts nor resets the streak. During the window, invocations skip the invocation-end flush, and the background writers keep sending on their own schedule. After the window, the next invocation flushes again. Keep the 500 ms default and the 100 ms margin. The industry standard for serverless SDKs is to flush before the invocation returns, with caps of 1 s or longer, so 500 ms is already conservative. A reserve of about 100 ms of remaining time matches published guidance. Precedent — Datadog libraries that already back off
A timeout caused by a short remaining-time budget does not count, because it says nothing about the intake. Any outcome other than Suggested fix and tests — each test fails when its branch is removed--- a/datadog_lambda/feature_flags.py
+++ b/datadog_lambda/feature_flags.py
@@ -2,6 +2,7 @@
import logging
import math
+import time
from datadog_lambda.config import config
@@ -13,9 +14,15 @@ except ImportError:
logger = logging.getLogger(__name__)
SAFETY_MARGIN_MS = 100.0
+# After repeated full-cap timeouts, stop adding the wait to every invocation for a while.
+MAX_CONSECUTIVE_TIMEOUTS = 3
+TIMEOUT_BACKOFF_S = 60.0
+_consecutive_timeouts = 0
+_skip_until = 0.0
def flush_feature_flag_telemetry(context):
+ global _consecutive_timeouts, _skip_until
try:
if _telemetry is None or getattr(_telemetry, "FLUSH_API_VERSION", None) != 1:
return
@@ -30,7 +37,17 @@ def flush_feature_flag_telemetry(context):
"Skipping feature flag telemetry flush: no invocation time remains"
)
return
+ if time.time() < _skip_until:
+ logger.debug("Skipping feature flag telemetry flush: backing off")
+ return
outcome = _telemetry.flush(budget_ms / 1000.0)
+ if outcome != "timed_out":
+ _consecutive_timeouts = 0
+ elif budget_ms >= cap_ms:
+ _consecutive_timeouts += 1
+ if _consecutive_timeouts >= MAX_CONSECUTIVE_TIMEOUTS:
+ _consecutive_timeouts = 0
+ _skip_until = time.time() + TIMEOUT_BACKOFF_S
if outcome != "completed":
logger.debug("Feature flag telemetry flush incomplete: %s", outcome)
except Exception:
--- a/tests/test_feature_flags.py
+++ b/tests/test_feature_flags.py
@@ -9,6 +9,37 @@ from datadog_lambda.config import Config
class FeatureFlagFlushTest(unittest.TestCase):
+ def setUp(self):
+ for name, value in [("_consecutive_timeouts", 0), ("_skip_until", 0.0)]:
+ patcher = patch.object(feature_flags, name, value)
+ patcher.start()
+ self.addCleanup(patcher.stop)
+
+ def _flush_with_outcomes(self, outcomes, remaining_ms=30000):
+ context = Mock(get_remaining_time_in_millis=lambda: remaining_ms)
+ with patch.object(feature_flags, "_telemetry") as telemetry:
+ telemetry.FLUSH_API_VERSION = 1
+ telemetry.flush.side_effect = outcomes
+ for _ in outcomes:
+ feature_flags.flush_feature_flag_telemetry(context)
+ return telemetry.flush.call_count
+
+ def test_repeated_full_budget_timeouts_back_off(self):
+ with patch.object(feature_flags.time, "time", return_value=1000.0) as now:
+ self.assertEqual(self._flush_with_outcomes(["timed_out"] * 4), 3)
+ now.return_value = 1000.0 + feature_flags.TIMEOUT_BACKOFF_S
+ self.assertEqual(self._flush_with_outcomes(["completed"]), 1)
+
+ def test_success_resets_timeout_count(self):
+ outcomes = ["timed_out", "timed_out", "completed", "timed_out", "timed_out"]
+ self.assertEqual(self._flush_with_outcomes(outcomes), 5)
+ self.assertEqual(feature_flags._skip_until, 0.0)
+
+ def test_short_budget_timeouts_do_not_back_off(self):
+ self.assertEqual(
+ self._flush_with_outcomes(["timed_out"] * 4, remaining_ms=200), 4
+ )
+
def test_budget(self):
for remaining, cap, expected in [
(5000, 500, 0.5),The change adds module state, so |
||||||||||||||||||||||||||||||||||||||||
| if outcome != "timed_out": | ||||||||||||||||||||||||||||||||||||||||
| _consecutive_timeouts = 0 | ||||||||||||||||||||||||||||||||||||||||
| elif budget_ms >= cap_ms: | ||||||||||||||||||||||||||||||||||||||||
| # A short invocation budget says nothing about intake health. | ||||||||||||||||||||||||||||||||||||||||
| _consecutive_timeouts += 1 | ||||||||||||||||||||||||||||||||||||||||
| if _consecutive_timeouts >= MAX_CONSECUTIVE_TIMEOUTS: | ||||||||||||||||||||||||||||||||||||||||
| _consecutive_timeouts = 0 | ||||||||||||||||||||||||||||||||||||||||
| _skip_until = monotonic() + TIMEOUT_BACKOFF_S | ||||||||||||||||||||||||||||||||||||||||
| if outcome != "completed": | ||||||||||||||||||||||||||||||||||||||||
| logger.debug("Feature flag telemetry flush incomplete: %s", outcome) | ||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked your other PR, do we even need to return something from the |
||||||||||||||||||||||||||||||||||||||||
| except Exception: | ||||||||||||||||||||||||||||||||||||||||
| # Never replace the response or the original handler exception. | ||||||||||||||||||||||||||||||||||||||||
| logger.debug("Feature flag telemetry flush failed", exc_info=True) | ||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be very honest, this method just seems really over engineered as it is right now. Why the check of Instead, this PR could simply be these two additions to # added to datadog_lambda/config.py
feature_flags_enabled = _get_env("DD_FEATURE_FLAGS_ENABLED", "true", as_bool)# at top of file
_feature_flags = None
if config.feature_flags_enabled:
try:
from ddtrace.internal.openfeature import _telemetry as _feature_flags
except ImportError:
pass# at end of self._after
if _feature_flags is not None:
_feature_flags.flush()
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, if we just call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Comment from Claude working with Aaron Silverman: Calling
On routing: with the default agentless configuration source, the tracer uses a local route only if A possible middle path: keep your wiring, and bound the wait in the wrapper, for example by running the two Results — 5d3d3e3 with dd-trace-py main 9b07368c88, Python 3.12, three runsBoth writers pointed at a local HTTP server through a remote-config route selector, which uses the local route without discovery. The handler enqueued one evaluation and one exposure. For the in-flight case, a background
Probe test — runs inside this repo's
|
||||||||||||||||||||||||||||||||||||||||
| Bottlecap | Tracer route | Queued for relay |
|---|---|---|
| patched | local /evp_proxy/v2; both periodic() calls took 1.9, 0.9 and 0.8 ms in three runs |
https://event-platform-intake.<site>/api/v2/flagevaluation (312 bytes) and /api/v2/exposures (216 bytes), each with DD-EVP-ORIGIN: dd-trace-py, DD-EVP-ORIGIN-VERSION and X-Datadog-EVP-Subdomain: event-platform-intake |
| unpatched | none; both periodic() calls returned in 0.1 ms |
nothing |
The test config has an empty site, so the printed host was event-platform-intake. with no domain.
What the tracer checks — real dd-trace-py discovery against each /info shape
FeatureFlagEVPRouteSelector with the default agentless configuration source, pointed at a local /info:
/info shape |
DD_API_KEY set |
DD_API_KEY unset |
|---|---|---|
| bottlecap today | direct | no route |
| plus the two flag routes | direct | no route |
plus /evp_proxy/v2/, no allowed headers |
direct | no route |
plus /evp_proxy/v2/ and the two allowed headers |
local /evp_proxy/v2, direct fallback |
local /evp_proxy/v2 |
| same, header names in lower case | local /evp_proxy/v2, direct fallback |
local /evp_proxy/v2 |
same, plus /evp_proxy/v4/ |
local /evp_proxy/v4 |
local /evp_proxy/v4 |
With the remote-config source, the tracer uses the local v2 route without discovery. On the local route, the tracer sends Content-Type: application/json, DD-EVP-ORIGIN, DD-EVP-ORIGIN-VERSION and X-Datadog-EVP-Subdomain: event-platform-intake, and no DD-API-KEY. The checks live in FeatureFlagEVPRouteSelector._discover_local_route() in ddtrace/internal/openfeature/_evp_transport.py.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| """Short-invocation smoke handler; use an exposure-enabled string test flag.""" | ||
|
|
||
| import json | ||
| import os | ||
| import time | ||
| import uuid | ||
|
|
||
| from datadog_lambda.wrapper import datadog_lambda_wrapper | ||
| from ddtrace.openfeature import DataDogProvider | ||
| from openfeature import api | ||
| from openfeature.evaluation_context import EvaluationContext | ||
|
|
||
| provider = DataDogProvider() | ||
| api.set_provider_and_wait(provider) | ||
| client = api.get_client("lambda-flush-smoke") | ||
| sandbox_id = uuid.uuid4().hex | ||
| invocations = 0 | ||
|
|
||
|
|
||
| @datadog_lambda_wrapper | ||
| def handle(event, context): | ||
| global invocations | ||
| invocations += 1 | ||
| started = time.monotonic() | ||
| subject = event["subject"] | ||
| details = client.get_string_details( | ||
| os.environ["FFL_SMOKE_FLAG_KEY"], | ||
| "smoke-default", | ||
| EvaluationContext(targeting_key=subject), | ||
| ) | ||
| result = { | ||
| "request_id": context.aws_request_id, | ||
| "sandbox_id": sandbox_id, | ||
| "invocation": invocations, | ||
| "subject": subject, | ||
| "value": details.value, | ||
| "variant": details.variant, | ||
| "error_code": str(details.error_code) if details.error_code else None, | ||
| "evaluation_ms": (time.monotonic() - started) * 1000, | ||
| } | ||
| # Only synthetic test subjects belong in this smoke-test log. | ||
| print(json.dumps(result)) | ||
| if details.error_code: | ||
| raise RuntimeError( | ||
| "Smoke flag did not resolve; verify test flag and configuration" | ||
| ) | ||
| if event.get("raise"): | ||
| raise RuntimeError("intentional smoke-test handler failure") | ||
| return result |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,236 @@ | ||
| import os | ||
| import subprocess | ||
| import sys | ||
| import textwrap | ||
| import unittest | ||
| from unittest.mock import Mock, patch | ||
|
|
||
| from datadog_lambda import feature_flags | ||
| from datadog_lambda.config import Config | ||
|
|
||
|
|
||
| class FeatureFlagFlushTest(unittest.TestCase): | ||
| def setUp(self): | ||
| for name, value in [("_consecutive_timeouts", 0), ("_skip_until", 0.0)]: | ||
| patcher = patch.object(feature_flags, name, value) | ||
| patcher.start() | ||
| self.addCleanup(patcher.stop) | ||
|
|
||
| def test_full_budget_timeouts_back_off_and_retry_after_expiry(self): | ||
| with patch.object( | ||
| feature_flags, "monotonic", return_value=1000.0 | ||
| ) as now, patch.object(feature_flags, "_telemetry") as telemetry, patch.object( | ||
| Config, "feature_flag_flush_timeout_ms", 250 | ||
| ): | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| telemetry.flush.return_value = "timed_out" | ||
| context = Mock(get_remaining_time_in_millis=lambda: 30000) | ||
| for _ in range(3): | ||
| feature_flags.flush_feature_flag_telemetry(context) | ||
| self.assertEqual(telemetry.flush.call_count, 3) | ||
| telemetry.flush.assert_called_with(0.25) | ||
| for timestamp in (1000.0, 1030.0, 1059.999): | ||
| now.return_value = timestamp | ||
| feature_flags.flush_feature_flag_telemetry(context) | ||
| self.assertEqual(telemetry.flush.call_count, 3) | ||
| now.return_value = 1060.0 | ||
| telemetry.flush.return_value = "completed" | ||
| feature_flags.flush_feature_flag_telemetry(context) | ||
| self.assertEqual(telemetry.flush.call_count, 4) | ||
| # Recovery starts a new streak, not an immediate second cooldown. | ||
| telemetry.flush.return_value = "timed_out" | ||
| for _ in range(2): | ||
| feature_flags.flush_feature_flag_telemetry(context) | ||
| telemetry.flush.return_value = "completed" | ||
| feature_flags.flush_feature_flag_telemetry(context) | ||
| self.assertEqual(telemetry.flush.call_count, 7) | ||
|
|
||
| def test_completed_and_failed_flushes_reset_timeout_streak(self): | ||
| for outcome in ("completed", "failed"): | ||
| with self.subTest(outcome=outcome), patch.object( | ||
| feature_flags, "_telemetry" | ||
| ) as telemetry, patch.object( | ||
| feature_flags, "monotonic", return_value=1000.0 | ||
| ): | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| outcomes = [ | ||
| "timed_out", | ||
| "timed_out", | ||
| outcome, | ||
| "timed_out", | ||
| "timed_out", | ||
| "completed", | ||
| ] | ||
| telemetry.flush.side_effect = outcomes | ||
| for _ in outcomes: | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: 30000) | ||
| ) | ||
| self.assertEqual(telemetry.flush.call_count, len(outcomes)) | ||
|
|
||
| def test_short_budget_timeouts_neither_count_nor_reset_streak(self): | ||
| with patch.object(feature_flags, "_telemetry") as telemetry, patch.object( | ||
| feature_flags, "monotonic", return_value=1000.0 | ||
| ): | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| telemetry.flush.return_value = "timed_out" | ||
| for remaining in (30000, 200, 200, 200, 30000, 30000): | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: remaining) | ||
| ) | ||
| self.assertEqual(telemetry.flush.call_count, 6) | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: 30000) | ||
| ) | ||
| self.assertEqual(telemetry.flush.call_count, 6) | ||
|
|
||
| def test_budget(self): | ||
| for remaining, cap, expected in [ | ||
| (5000, 500, 0.5), | ||
| (200, 500, 0.1), | ||
| (101, 500, 0.001), | ||
| ]: | ||
| with self.subTest(remaining=remaining), patch.object( | ||
| feature_flags, "_telemetry" | ||
| ) as telemetry: | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| telemetry.flush.return_value = "completed" | ||
| with patch.object( | ||
| type(feature_flags.config), "feature_flag_flush_timeout_ms", cap | ||
| ): | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: remaining) | ||
| ) | ||
| telemetry.flush.assert_called_once_with(expected) | ||
|
|
||
| def test_no_budget(self): | ||
| for remaining, cap in [ | ||
| (100, 500), | ||
| (0, 500), | ||
| (1000, 0), | ||
| (1000, -1), | ||
| (1000, float("nan")), | ||
| ]: | ||
| with self.subTest(remaining=remaining, cap=cap), patch.object( | ||
| feature_flags, "_telemetry" | ||
| ) as telemetry: | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| with patch.object( | ||
| type(feature_flags.config), "feature_flag_flush_timeout_ms", cap | ||
| ): | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: remaining) | ||
| ) | ||
| telemetry.flush.assert_not_called() | ||
|
|
||
| def test_old_tracer(self): | ||
| for telemetry in [None, Mock(FLUSH_API_VERSION=2)]: | ||
| with self.subTest(telemetry=telemetry), patch.object( | ||
| feature_flags, "_telemetry", telemetry | ||
| ): | ||
| context = Mock() | ||
| context.get_remaining_time_in_millis.return_value = 1000 | ||
| feature_flags.flush_feature_flag_telemetry(context) | ||
| context.get_remaining_time_in_millis.assert_not_called() | ||
| if telemetry is not None: | ||
| telemetry.flush.assert_not_called() | ||
|
|
||
| def test_timeout_environment_parsing_preserves_flush(self): | ||
| for value, expected in [ | ||
| (None, 0.5), | ||
| ("250.5", 0.2505), | ||
| ("", 0.5), | ||
| ("garbage", 0.5), | ||
| ("500ms", 0.5), | ||
| ]: | ||
| with self.subTest(value=value), patch.dict(os.environ), patch.object( | ||
| feature_flags, "config", Config() | ||
| ), patch.object(feature_flags, "_telemetry") as telemetry: | ||
| if value is None: | ||
| os.environ.pop("DD_FEATURE_FLAGS_FLUSH_TIMEOUT_MS", None) | ||
| else: | ||
| os.environ["DD_FEATURE_FLAGS_FLUSH_TIMEOUT_MS"] = value | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| telemetry.flush.return_value = "completed" | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: 1000) | ||
| ) | ||
| telemetry.flush.assert_called_once_with(expected) | ||
|
|
||
| def test_missing_context_and_telemetry_errors_are_safe(self): | ||
| with patch.object(feature_flags, "_telemetry") as telemetry: | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| telemetry.flush.side_effect = RuntimeError("unavailable") | ||
| feature_flags.flush_feature_flag_telemetry(None) | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: 1000) | ||
| ) | ||
| telemetry.flush.assert_called_once() | ||
|
|
||
| def test_missing_remaining_time_skips_without_failure_log(self): | ||
| for context in (None, object()): | ||
| with self.subTest(context=context), patch.object( | ||
| feature_flags, "_telemetry" | ||
| ) as telemetry, patch.object(feature_flags, "logger") as log: | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| feature_flags.flush_feature_flag_telemetry(context) | ||
| telemetry.flush.assert_not_called() | ||
| log.debug.assert_not_called() | ||
|
|
||
| def test_unsupported_tracer_skip_is_logged(self): | ||
| with patch.object( | ||
| feature_flags, "_telemetry", Mock(FLUSH_API_VERSION=2) | ||
| ) as telemetry, patch.object(feature_flags, "logger") as log: | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: 1000) | ||
| ) | ||
| telemetry.flush.assert_not_called() | ||
| log.debug.assert_called_once_with( | ||
| "Skipping feature flag telemetry flush: unsupported tracer" | ||
| ) | ||
|
|
||
| def test_disabled_feature_flags_do_not_import_flush_modules(self): | ||
| code = textwrap.dedent( | ||
| """ | ||
| import sys | ||
|
|
||
| class BlockFlushImports: | ||
| def find_spec(self, fullname, *args): | ||
| if fullname in ( | ||
| "datadog_lambda.feature_flags", | ||
| "ddtrace.internal.openfeature._telemetry", | ||
| ): | ||
| raise AssertionError("unexpected import: " + fullname) | ||
|
|
||
| sys.meta_path.insert(0, BlockFlushImports()) | ||
| from datadog_lambda import wrapper | ||
| assert not wrapper.config.feature_flags_enabled | ||
| assert "datadog_lambda.feature_flags" not in sys.modules | ||
| assert "ddtrace.internal.openfeature._telemetry" not in sys.modules | ||
| """ | ||
| ) | ||
| result = subprocess.run( | ||
| [sys.executable, "-c", code], | ||
| env={ | ||
| **os.environ, | ||
| "DD_FEATURE_FLAGS_ENABLED": "false", | ||
| "DD_INSTRUMENTATION_TELEMETRY_ENABLED": "false", | ||
| }, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=30, | ||
| ) | ||
| self.assertEqual(result.returncode, 0, result.stdout + result.stderr) | ||
|
|
||
| def test_incomplete_diagnostic(self): | ||
| with patch.object(feature_flags, "_telemetry") as telemetry, patch.object( | ||
| feature_flags, "logger" | ||
| ) as log: | ||
| telemetry.FLUSH_API_VERSION = 1 | ||
| telemetry.flush.return_value = "timed_out" | ||
| feature_flags.flush_feature_flag_telemetry( | ||
| Mock(get_remaining_time_in_millis=lambda: 1000) | ||
| ) | ||
| log.debug.assert_called_with( | ||
| "Feature flag telemetry flush incomplete: %s", "timed_out" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we get this PR more ready to merge, I'd like to run a before/after test on the cold start duration. We've found in the past that adding imports, when the code ultimately isn't being used here, can add significant amounts of time to the beginning of the first lambda invocation. From what I can tell, I don't think that will be the case here, but it will be good to know for certain. I'll run that testing once we get to a good point.