Skip to content

Commit d28c471

Browse files
committed
fix: review findings -- type annotations, PII defaults to opt-in
1. Added return type annotations to all Metrics methods and Sentry helper functions (CLAUDE.md: type hints on all Python functions). 2. send_default_pii and include_local_variables now driven by env vars (SENTRY_SEND_PII, SENTRY_INCLUDE_LOCAL_VARS), defaulting to false. Deployers must explicitly opt in to send user emails to Sentry. Skipped: lazy singleton for metrics (same pattern as logger, pure in-memory, no side effects), async capture_http_exception (Sentry SDK already enqueues to background thread, non-blocking by design). 284 tests pass.
1 parent ad32024 commit d28c471

1 file changed

Lines changed: 16 additions & 12 deletions

File tree

backend/services/observability.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ class Metrics:
313313
metrics.get_stats() # raw counters/timings/gauges
314314
"""
315315

316-
def __init__(self):
316+
def __init__(self) -> None:
317317
self._counters: Dict[str, int] = {}
318318
self._timings: Dict[str, list] = {}
319319
self._gauges: Dict[str, float] = {}
@@ -324,23 +324,23 @@ def __init__(self):
324324
self._cache_hits: int = 0
325325
self._cache_misses: int = 0
326326

327-
def increment(self, name: str, value: int = 1, **tags):
327+
def increment(self, name: str, value: int = 1, **tags) -> None:
328328
"""Increment a counter"""
329329
self._counters[name] = self._counters.get(name, 0) + value
330330

331-
def timing(self, name: str, value_ms: float):
331+
def timing(self, name: str, value_ms: float) -> None:
332332
"""Record a timing measurement"""
333333
if name not in self._timings:
334334
self._timings[name] = []
335335
self._timings[name].append(value_ms)
336336
if len(self._timings[name]) > 1000:
337337
self._timings[name] = self._timings[name][-1000:]
338338

339-
def gauge(self, name: str, value: float):
339+
def gauge(self, name: str, value: float) -> None:
340340
"""Record a point-in-time value"""
341341
self._gauges[name] = value
342342

343-
def record_indexing(self, repo_id: str, duration: float, function_count: int):
343+
def record_indexing(self, repo_id: str, duration: float, function_count: int) -> None:
344344
"""Record indexing performance for dashboard metrics."""
345345
self._indexing_times.append({
346346
"repo_id": repo_id,
@@ -350,7 +350,7 @@ def record_indexing(self, repo_id: str, duration: float, function_count: int):
350350
"timestamp": datetime.now().isoformat(),
351351
})
352352

353-
def record_search(self, duration: float, cached: bool):
353+
def record_search(self, duration: float, cached: bool) -> None:
354354
"""Record search performance for dashboard metrics."""
355355
self._search_times.append({
356356
"duration": duration,
@@ -423,7 +423,7 @@ def get_stats(self) -> Dict:
423423
}
424424
return stats
425425

426-
def reset(self):
426+
def reset(self) -> None:
427427
"""Reset all metrics"""
428428
self._counters = {}
429429
self._timings = {}
@@ -452,20 +452,24 @@ def init_sentry() -> bool:
452452

453453
environment = os.getenv("ENVIRONMENT", "development")
454454

455+
# PII and debug settings -- opt-in via env vars, default to safe
456+
send_pii = os.getenv("SENTRY_SEND_PII", "false").lower() in ("true", "1")
457+
include_locals = os.getenv("SENTRY_INCLUDE_LOCAL_VARS", "false").lower() in ("true", "1")
458+
455459
sentry_sdk.init(
456460
dsn=sentry_dsn,
457461
environment=environment,
458462
traces_sample_rate=0.1 if environment == "production" else 1.0,
459463
profiles_sample_rate=0.1 if environment == "production" else 1.0,
460-
send_default_pii=True,
464+
send_default_pii=send_pii,
461465
integrations=[
462466
FastApiIntegration(transaction_style="endpoint"),
463467
StarletteIntegration(transaction_style="endpoint"),
464468
],
465469
before_send=_filter_events,
466470
debug=environment == "development",
467471
attach_stacktrace=True,
468-
include_local_variables=True,
472+
include_local_variables=include_locals,
469473
)
470474

471475
print(f"[OK] Sentry initialized (environment: {environment})")
@@ -479,7 +483,7 @@ def init_sentry() -> bool:
479483
return False
480484

481485

482-
def _filter_events(event, hint):
486+
def _filter_events(event: Dict[str, Any], hint: Optional[Dict[str, Any]]) -> Optional[Dict[str, Any]]:
483487
"""Filter out noisy events before sending to Sentry."""
484488
request_url = event.get("request", {}).get("url", "")
485489
if "/health" in request_url:
@@ -500,7 +504,7 @@ def _filter_events(event, hint):
500504
return event
501505

502506

503-
def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None):
507+
def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None) -> None:
504508
"""Set Sentry user context for error attribution."""
505509
try:
506510
import sentry_sdk
@@ -509,7 +513,7 @@ def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None)
509513
pass
510514

511515

512-
def capture_http_exception(request, exc: Exception, status_code: int):
516+
def capture_http_exception(request: Any, exc: Exception, status_code: int) -> None:
513517
"""Capture HTTP exception with request context for Sentry."""
514518
try:
515519
import sentry_sdk

0 commit comments

Comments
 (0)