Skip to content

Commit 977b306

Browse files
committed
fix(spotlight): align behavior with SDK spec
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 5fc28a1 commit 977b306

File tree

4 files changed

+179
-34
lines changed

4 files changed

+179
-34
lines changed

sentry_sdk/client.py

Lines changed: 65 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,69 @@ 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+
spotlight_config = self.options.get("spotlight")
394+
spotlight_env_value = os.environ.get("SENTRY_SPOTLIGHT")
395+
396+
# Parse env var to determine if it's a boolean or URL
397+
spotlight_from_env = None # type: Optional[bool]
398+
spotlight_env_url = None # type: Optional[str]
399+
if spotlight_env_value:
400+
parsed = env_to_bool(spotlight_env_value, strict=True)
401+
if parsed is None:
402+
# It's a URL string
403+
spotlight_from_env = True
404+
spotlight_env_url = spotlight_env_value
405+
else:
406+
spotlight_from_env = parsed
407+
408+
# Apply precedence rules per spec
409+
if spotlight_config is False:
410+
# Config explicitly disables spotlight - warn if env var was set
411+
if spotlight_from_env:
412+
logger.warning(
413+
"Spotlight is disabled via spotlight=False config option, "
414+
"ignoring SENTRY_SPOTLIGHT environment variable."
415+
)
416+
self.options["spotlight"] = False
417+
elif spotlight_config is True:
418+
# Config enables spotlight with boolean true
419+
# If env var has URL, use env var URL per spec
420+
if spotlight_env_url:
421+
self.options["spotlight"] = spotlight_env_url
422+
else:
423+
self.options["spotlight"] = True
424+
elif isinstance(spotlight_config, str):
425+
# Config has URL string - use config URL, warn if env var differs
426+
if spotlight_env_value and spotlight_env_value != spotlight_config:
427+
logger.warning(
428+
"Spotlight URL from config (%s) takes precedence over "
429+
"SENTRY_SPOTLIGHT environment variable (%s).",
430+
spotlight_config,
431+
spotlight_env_value,
432+
)
433+
# self.options["spotlight"] already has the config URL
434+
elif spotlight_config is None:
435+
# No config - use env var
436+
if spotlight_env_url:
437+
self.options["spotlight"] = spotlight_env_url
438+
elif spotlight_from_env:
439+
self.options["spotlight"] = True
440+
# else: stays None (disabled)
441+
442+
if self.options.get("spotlight"):
443+
# This is intentionally here to prevent setting up spotlight
444+
# stuff we don't need unless spotlight is explicitly enabled
445+
from sentry_sdk.spotlight import setup_spotlight
446+
447+
self.spotlight = setup_spotlight(self.options)
448+
if not self.options["dsn"]:
449+
sample_all = lambda *_args, **_kwargs: 1.0
450+
self.options["send_default_pii"] = True
451+
self.options["error_sampler"] = sample_all
452+
self.options["traces_sampler"] = sample_all
453+
self.options["profiles_sampler"] = sample_all
454+
390455
self.session_flusher = SessionFlusher(capture_func=_capture_envelope)
391456

392457
self.log_batcher = None
@@ -437,29 +502,6 @@ def _record_lost_event(
437502
options=self.options,
438503
)
439504

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-
463505
sdk_name = get_sdk_name(list(self.integrations.keys()))
464506
SDK_INFO["name"] = sdk_name
465507
logger.debug("Setting SDK name to '%s'", sdk_name)

sentry_sdk/spotlight.py

Lines changed: 45 additions & 10 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,39 @@
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._error_logged = False
57+
self._retry_delay = self.INITIAL_RETRY_DELAY
58+
self._last_error_time = 0.0 # type: float
4259

4360
def capture_envelope(self, envelope):
4461
# type: (Envelope) -> None
62+
63+
# Check if we're in backoff period - skip sending to avoid blocking
64+
current_time = time.time()
65+
if self._last_error_time > 0:
66+
time_since_error = current_time - self._last_error_time
67+
if time_since_error < self._retry_delay:
68+
# Still in backoff period, skip this envelope
69+
return
70+
4571
body = io.BytesIO()
4672
envelope.serialize_into(body)
4773
try:
@@ -54,18 +80,27 @@ def capture_envelope(self, envelope):
5480
},
5581
)
5682
req.close()
57-
self.fails = 0
83+
# Success - reset backoff state
84+
self._error_logged = False
85+
self._retry_delay = self.INITIAL_RETRY_DELAY
86+
self._last_error_time = 0.0
5887
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
88+
current_time = time.time()
89+
self._last_error_time = current_time
90+
91+
# Log error only once per backoff cycle
92+
if not self._error_logged:
6493
sentry_logger.warning(
65-
"Looks like Spotlight is not running, will keep trying to send events but will not log errors."
94+
"Failed to send envelope to Spotlight at %s: %s. "
95+
"Will retry after %.1f seconds.",
96+
self.url,
97+
e,
98+
self._retry_delay,
6699
)
67-
# omitting self.fails += 1 in the `else:` case intentionally
68-
# to avoid overflowing the variable if Spotlight never becomes reachable
100+
self._error_logged = True
101+
102+
# Increase backoff delay exponentially
103+
self._retry_delay = min(self._retry_delay * 2, self.MAX_RETRY_DELAY)
69104

70105

71106
try:

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)