Skip to content

Commit 6c6705a

Browse files
authored
fix(spotlight): align behavior with SDK spec (#5169)
Align the Spotlight implementation with the official spec at https://develop.sentry.dev/sdk/expected-features/spotlight/ Changes: 1. Fix precedence rules: - When spotlight=True and SENTRY_SPOTLIGHT env var is a URL, use the env var URL (per spec requirement) - Log warning when config URL overrides env var URL - Log warning when spotlight=False explicitly disables despite env var being set 2. Route all envelopes to Spotlight: - Sessions, logs, and metrics now also get sent to Spotlight via the _capture_envelope callback - Move Spotlight initialization before batchers are created 3. Add exponential backoff retry logic: - SpotlightClient now implements proper exponential backoff when the Spotlight server is unreachable - Skips sending during backoff period to avoid blocking - Logs errors only once per backoff cycle 4. Update tests: - Fix test expectation for spotlight=True + env URL case - Add tests for warning scenarios - Add test for session envelope routing to Spotlight
1 parent 7449603 commit 6c6705a

File tree

4 files changed

+194
-43
lines changed

4 files changed

+194
-43
lines changed

sentry_sdk/client.py

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,8 @@ def _init_impl(self):
360360

361361
def _capture_envelope(envelope):
362362
# type: (Envelope) -> None
363+
if self.spotlight is not None:
364+
self.spotlight.capture_envelope(envelope)
363365
if self.transport is not None:
364366
self.transport.capture_envelope(envelope)
365367

@@ -387,6 +389,18 @@ def _record_lost_event(
387389
if self.options["enable_backpressure_handling"]:
388390
self.monitor = Monitor(self.transport)
389391

392+
# Setup Spotlight before creating batchers so _capture_envelope can use it.
393+
# setup_spotlight handles all config/env var resolution per the SDK spec.
394+
from sentry_sdk.spotlight import setup_spotlight
395+
396+
self.spotlight = setup_spotlight(self.options)
397+
if self.spotlight is not None and not self.options["dsn"]:
398+
sample_all = lambda *_args, **_kwargs: 1.0
399+
self.options["send_default_pii"] = True
400+
self.options["error_sampler"] = sample_all
401+
self.options["traces_sampler"] = sample_all
402+
self.options["profiles_sampler"] = sample_all
403+
390404
self.session_flusher = SessionFlusher(capture_func=_capture_envelope)
391405

392406
self.log_batcher = None
@@ -437,29 +451,6 @@ def _record_lost_event(
437451
options=self.options,
438452
)
439453

440-
spotlight_config = self.options.get("spotlight")
441-
if spotlight_config is None and "SENTRY_SPOTLIGHT" in os.environ:
442-
spotlight_env_value = os.environ["SENTRY_SPOTLIGHT"]
443-
spotlight_config = env_to_bool(spotlight_env_value, strict=True)
444-
self.options["spotlight"] = (
445-
spotlight_config
446-
if spotlight_config is not None
447-
else spotlight_env_value
448-
)
449-
450-
if self.options.get("spotlight"):
451-
# This is intentionally here to prevent setting up spotlight
452-
# stuff we don't need unless spotlight is explicitly enabled
453-
from sentry_sdk.spotlight import setup_spotlight
454-
455-
self.spotlight = setup_spotlight(self.options)
456-
if not self.options["dsn"]:
457-
sample_all = lambda *_args, **_kwargs: 1.0
458-
self.options["send_default_pii"] = True
459-
self.options["error_sampler"] = sample_all
460-
self.options["traces_sampler"] = sample_all
461-
self.options["profiles_sampler"] = sample_all
462-
463454
sdk_name = get_sdk_name(list(self.integrations.keys()))
464455
SDK_INFO["name"] = sdk_name
465456
logger.debug("Setting SDK name to '%s'", sdk_name)

sentry_sdk/spotlight.py

Lines changed: 111 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import io
22
import logging
33
import os
4+
import time
45
import urllib.parse
56
import urllib.request
67
import urllib.error
@@ -34,14 +35,37 @@
3435

3536

3637
class SpotlightClient:
38+
"""
39+
A client for sending envelopes to Sentry Spotlight.
40+
41+
Implements exponential backoff retry logic per the SDK spec:
42+
- Logs error at least once when server is unreachable
43+
- Does not log for every failed envelope
44+
- Uses exponential backoff to avoid hammering an unavailable server
45+
- Never blocks normal Sentry operation
46+
"""
47+
48+
# Exponential backoff settings
49+
INITIAL_RETRY_DELAY = 1.0 # Start with 1 second
50+
MAX_RETRY_DELAY = 60.0 # Max 60 seconds
51+
3752
def __init__(self, url):
3853
# type: (str) -> None
3954
self.url = url
4055
self.http = urllib3.PoolManager()
41-
self.fails = 0
56+
self._retry_delay = self.INITIAL_RETRY_DELAY
57+
self._last_error_time = 0.0 # type: float
4258

4359
def capture_envelope(self, envelope):
4460
# type: (Envelope) -> None
61+
62+
# Check if we're in backoff period - skip sending to avoid blocking
63+
if self._last_error_time > 0:
64+
time_since_error = time.time() - self._last_error_time
65+
if time_since_error < self._retry_delay:
66+
# Still in backoff period, skip this envelope
67+
return
68+
4569
body = io.BytesIO()
4670
envelope.serialize_into(body)
4771
try:
@@ -54,18 +78,23 @@ def capture_envelope(self, envelope):
5478
},
5579
)
5680
req.close()
57-
self.fails = 0
81+
# Success - reset backoff state
82+
self._retry_delay = self.INITIAL_RETRY_DELAY
83+
self._last_error_time = 0.0
5884
except Exception as e:
59-
if self.fails < 2:
60-
sentry_logger.warning(str(e))
61-
self.fails += 1
62-
elif self.fails == 2:
63-
self.fails += 1
64-
sentry_logger.warning(
65-
"Looks like Spotlight is not running, will keep trying to send events but will not log errors."
66-
)
67-
# omitting self.fails += 1 in the `else:` case intentionally
68-
# to avoid overflowing the variable if Spotlight never becomes reachable
85+
self._last_error_time = time.time()
86+
87+
# Increase backoff delay exponentially first, so logged value matches actual wait
88+
self._retry_delay = min(self._retry_delay * 2, self.MAX_RETRY_DELAY)
89+
90+
# Log error once per backoff cycle (we skip sends during backoff, so only one failure per cycle)
91+
sentry_logger.warning(
92+
"Failed to send envelope to Spotlight at %s: %s. "
93+
"Will retry after %.1f seconds.",
94+
self.url,
95+
e,
96+
self._retry_delay,
97+
)
6998

7099

71100
try:
@@ -207,20 +236,83 @@ def process_exception(self, _request, exception):
207236
settings = None
208237

209238

239+
def _resolve_spotlight_url(spotlight_config, sentry_logger):
240+
# type: (Any, Any) -> Optional[str]
241+
"""
242+
Resolve the Spotlight URL based on config and environment variable.
243+
244+
Implements precedence rules per the SDK spec:
245+
https://develop.sentry.dev/sdk/expected-features/spotlight/
246+
247+
Returns the resolved URL string, or None if Spotlight should be disabled.
248+
"""
249+
spotlight_env_value = os.environ.get("SENTRY_SPOTLIGHT")
250+
251+
# Parse env var to determine if it's a boolean or URL
252+
spotlight_from_env = None # type: Optional[bool]
253+
spotlight_env_url = None # type: Optional[str]
254+
if spotlight_env_value:
255+
parsed = env_to_bool(spotlight_env_value, strict=True)
256+
if parsed is None:
257+
# It's a URL string
258+
spotlight_from_env = True
259+
spotlight_env_url = spotlight_env_value
260+
else:
261+
spotlight_from_env = parsed
262+
263+
# Apply precedence rules per spec:
264+
# https://develop.sentry.dev/sdk/expected-features/spotlight/#precedence-rules
265+
if spotlight_config is False:
266+
# Config explicitly disables spotlight - warn if env var was set
267+
if spotlight_from_env:
268+
sentry_logger.warning(
269+
"Spotlight is disabled via spotlight=False config option, "
270+
"ignoring SENTRY_SPOTLIGHT environment variable."
271+
)
272+
return None
273+
elif spotlight_config is True:
274+
# Config enables spotlight with boolean true
275+
# If env var has URL, use env var URL per spec
276+
if spotlight_env_url:
277+
return spotlight_env_url
278+
else:
279+
return DEFAULT_SPOTLIGHT_URL
280+
elif isinstance(spotlight_config, str):
281+
# Config has URL string - use config URL, warn if env var differs
282+
if spotlight_env_value and spotlight_env_value != spotlight_config:
283+
sentry_logger.warning(
284+
"Spotlight URL from config (%s) takes precedence over "
285+
"SENTRY_SPOTLIGHT environment variable (%s).",
286+
spotlight_config,
287+
spotlight_env_value,
288+
)
289+
return spotlight_config
290+
elif spotlight_config is None:
291+
# No config - use env var
292+
if spotlight_env_url:
293+
return spotlight_env_url
294+
elif spotlight_from_env:
295+
return DEFAULT_SPOTLIGHT_URL
296+
# else: stays None (disabled)
297+
298+
return None
299+
300+
210301
def setup_spotlight(options):
211302
# type: (Dict[str, Any]) -> Optional[SpotlightClient]
303+
url = _resolve_spotlight_url(options.get("spotlight"), sentry_logger)
304+
305+
if url is None:
306+
return None
307+
308+
# Only set up logging handler when spotlight is actually enabled
212309
_handler = logging.StreamHandler(sys.stderr)
213310
_handler.setFormatter(logging.Formatter(" [spotlight] %(levelname)s: %(message)s"))
214311
logger.addHandler(_handler)
215312
logger.setLevel(logging.INFO)
216313

217-
url = options.get("spotlight")
218-
219-
if url is True:
220-
url = DEFAULT_SPOTLIGHT_URL
221-
222-
if not isinstance(url, str):
223-
return None
314+
# Update options with resolved URL for consistency
315+
options["spotlight"] = url
224316

225317
with capture_internal_exceptions():
226318
if (

tests/test_client.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1181,7 +1181,8 @@ def test_debug_option(
11811181
(None, "t", DEFAULT_SPOTLIGHT_URL),
11821182
(None, "1", DEFAULT_SPOTLIGHT_URL),
11831183
(True, None, DEFAULT_SPOTLIGHT_URL),
1184-
(True, "http://localhost:8080/slurp", DEFAULT_SPOTLIGHT_URL),
1184+
# Per spec: spotlight=True + env URL -> use env URL
1185+
(True, "http://localhost:8080/slurp", "http://localhost:8080/slurp"),
11851186
("http://localhost:8080/slurp", "f", "http://localhost:8080/slurp"),
11861187
(None, "http://localhost:8080/slurp", "http://localhost:8080/slurp"),
11871188
],

tests/test_spotlight.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22

33
import sentry_sdk
4+
from sentry_sdk.spotlight import DEFAULT_SPOTLIGHT_URL
45

56

67
@pytest.fixture
@@ -54,3 +55,69 @@ def test_spotlight_envelope(sentry_init, capture_spotlight_envelopes):
5455
payload = envelope.items[0].payload.json
5556

5657
assert payload["exception"]["values"][0]["value"] == "aha!"
58+
59+
60+
def test_spotlight_true_with_env_url_uses_env_url(sentry_init, monkeypatch):
61+
"""Per spec: spotlight=True + env URL -> use env URL"""
62+
monkeypatch.setenv("SENTRY_SPOTLIGHT", "http://custom:9999/stream")
63+
sentry_init(spotlight=True)
64+
65+
spotlight = sentry_sdk.get_client().spotlight
66+
assert spotlight is not None
67+
assert spotlight.url == "http://custom:9999/stream"
68+
69+
70+
def test_spotlight_false_ignores_env_var(sentry_init, monkeypatch, caplog):
71+
"""Per spec: spotlight=False ignores env var and logs warning"""
72+
import logging
73+
74+
with caplog.at_level(logging.WARNING, logger="sentry_sdk.errors"):
75+
monkeypatch.setenv("SENTRY_SPOTLIGHT", "true")
76+
sentry_init(spotlight=False, debug=True)
77+
78+
assert sentry_sdk.get_client().spotlight is None
79+
assert "ignoring SENTRY_SPOTLIGHT environment variable" in caplog.text
80+
81+
82+
def test_spotlight_config_url_overrides_env_url_with_warning(
83+
sentry_init, monkeypatch, caplog
84+
):
85+
"""Per spec: config URL takes precedence over env URL with warning"""
86+
import logging
87+
88+
with caplog.at_level(logging.WARNING, logger="sentry_sdk.errors"):
89+
monkeypatch.setenv("SENTRY_SPOTLIGHT", "http://env:9999/stream")
90+
sentry_init(spotlight="http://config:8888/stream", debug=True)
91+
92+
spotlight = sentry_sdk.get_client().spotlight
93+
assert spotlight is not None
94+
assert spotlight.url == "http://config:8888/stream"
95+
assert "takes precedence over" in caplog.text
96+
97+
98+
def test_spotlight_config_url_same_as_env_no_warning(sentry_init, monkeypatch, caplog):
99+
"""No warning when config URL matches env URL"""
100+
import logging
101+
102+
with caplog.at_level(logging.WARNING, logger="sentry_sdk.errors"):
103+
monkeypatch.setenv("SENTRY_SPOTLIGHT", "http://same:9999/stream")
104+
sentry_init(spotlight="http://same:9999/stream", debug=True)
105+
106+
spotlight = sentry_sdk.get_client().spotlight
107+
assert spotlight is not None
108+
assert spotlight.url == "http://same:9999/stream"
109+
assert "takes precedence over" not in caplog.text
110+
111+
112+
def test_spotlight_receives_session_envelopes(sentry_init, capture_spotlight_envelopes):
113+
"""Spotlight should receive session envelopes, not just error events"""
114+
sentry_init(spotlight=True, release="test-release")
115+
envelopes = capture_spotlight_envelopes()
116+
117+
# Start and end a session
118+
sentry_sdk.get_isolation_scope().start_session()
119+
sentry_sdk.get_isolation_scope().end_session()
120+
sentry_sdk.flush()
121+
122+
# Should have received at least one envelope with session data
123+
assert len(envelopes) > 0

0 commit comments

Comments
 (0)