From 84d7cee369634dededbe62a784d8cd1bfcae0cb4 Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Mon, 23 Feb 2026 16:59:09 -0500 Subject: [PATCH 1/5] refactor: consolidate sentry.py + performance_metrics.py into observability.py Three separate observability systems merged into one: BEFORE: services/sentry.py (175 lines) -- Sentry init, user context, error capture services/performance_metrics.py (79 lines) -- search/indexing stats services/observability.py (366 lines) -- logger, metrics, tracing Two 'metrics' objects with naming collision. 4 duplicate functions across sentry.py and observability.py. 3 files still importing deprecated functions from sentry.py. AFTER: services/observability.py (532 lines) -- everything, one import path Single import: from services.observability import logger, metrics, ... Changes: - Metrics class now includes record_search(), record_indexing(), get_metrics() (absorbed from PerformanceMetrics) - Added init_sentry(), _filter_events(), set_user_context(), capture_http_exception() (moved from sentry.py) - Updated 7 files to import from observability instead of sentry - dependencies.py: removed PerformanceMetrics, uses observability.metrics - Deleted services/sentry.py (175 lines) - Deleted services/performance_metrics.py (79 lines) Net: -92 lines, 2 files deleted, 1 naming collision eliminated. 284 tests pass. Closes OPE-87 --- backend/dependencies.py | 3 +- backend/main.py | 4 +- backend/middleware/auth.py | 4 +- backend/services/observability.py | 200 ++++++++++++++++++++++-- backend/services/performance_metrics.py | 79 ---------- backend/services/playground_limiter.py | 3 +- backend/services/repo_validator.py | 3 +- backend/services/sentry.py | 175 --------------------- backend/services/user_limits.py | 3 +- 9 files changed, 191 insertions(+), 283 deletions(-) delete mode 100644 backend/services/performance_metrics.py delete mode 100644 backend/services/sentry.py diff --git a/backend/dependencies.py b/backend/dependencies.py index 26937c9..6891d67 100644 --- a/backend/dependencies.py +++ b/backend/dependencies.py @@ -9,13 +9,13 @@ from services.cache import CacheService from services.dependency_analyzer import DependencyAnalyzer from services.style_analyzer import StyleAnalyzer -from services.performance_metrics import PerformanceMetrics from services.dna_extractor import DNAExtractor from services.rate_limiter import RateLimiter, APIKeyManager from services.supabase_service import get_supabase_service from services.input_validator import InputValidator, CostController from services.user_limits import init_user_limits_service, get_user_limits_service from services.repo_validator import get_repo_validator +from services.observability import metrics # Service instances (singleton pattern) indexer = OptimizedCodeIndexer() @@ -24,7 +24,6 @@ dependency_analyzer = DependencyAnalyzer() style_analyzer = StyleAnalyzer() dna_extractor = DNAExtractor() -metrics = PerformanceMetrics() # Rate limiting and API key management rate_limiter = RateLimiter(redis_client=cache.redis if cache.redis else None) diff --git a/backend/main.py b/backend/main.py index 4eafc49..02141a3 100644 --- a/backend/main.py +++ b/backend/main.py @@ -15,7 +15,7 @@ import os # Initialize Sentry FIRST (before other imports to catch all errors) -from services.sentry import init_sentry +from services.observability import init_sentry init_sentry() # Import API config (single source of truth for versioning) @@ -146,7 +146,7 @@ async def generic_exception_handler(request: Request, exc: Exception): Catch-all handler for unhandled exceptions. Captures to Sentry and returns 500. """ - from services.sentry import capture_http_exception + from services.observability import capture_http_exception capture_http_exception(request, exc, 500) return JSONResponse( diff --git a/backend/middleware/auth.py b/backend/middleware/auth.py index 7aff83a..511fa40 100644 --- a/backend/middleware/auth.py +++ b/backend/middleware/auth.py @@ -117,7 +117,7 @@ def _authenticate(token: str) -> AuthContext: ctx = _validate_jwt(token) if ctx: # Set Sentry user context for error tracking - from services.sentry import set_user_context + from services.observability import set_user_context set_user_context(user_id=ctx.user_id, email=ctx.email) return ctx @@ -125,7 +125,7 @@ def _authenticate(token: str) -> AuthContext: ctx = _validate_api_key(token) if ctx: # Set Sentry user context for error tracking - from services.sentry import set_user_context + from services.observability import set_user_context set_user_context(user_id=ctx.user_id or ctx.api_key_name) return ctx diff --git a/backend/services/observability.py b/backend/services/observability.py index 645ceec..f526e02 100644 --- a/backend/services/observability.py +++ b/backend/services/observability.py @@ -1,11 +1,12 @@ """ Observability Module -Centralized logging, tracing, and metrics for CodeIntel +Centralized logging, tracing, metrics, and error tracking for CodeIntel -Usage: - from services.observability import logger, trace_operation, track_time +Single import for all observability needs: + from services.observability import logger, metrics, capture_exception, track_time logger.info("Starting indexing", repo_id="abc", files=100) + metrics.record_search(duration, cached=True) @trace_operation("indexing") async def index_repo(repo_id: str): @@ -23,6 +24,7 @@ async def index_repo(repo_id: str): from functools import wraps from contextlib import contextmanager from datetime import datetime +from collections import deque # Environment ENVIRONMENT = os.getenv("ENVIRONMENT", "development") @@ -301,58 +303,130 @@ def sync_wrapper(*args, **kwargs): return decorator -# SIMPLE METRICS (in-memory counters) +# METRICS (unified counters + performance tracking) class Metrics: """ - Simple in-memory metrics counters. + Unified metrics: generic counters/timings/gauges plus + domain-specific search and indexing performance tracking. Usage: - metrics.increment("search_requests", repo_id="abc") + metrics.increment("search_requests") metrics.timing("search_latency_ms", 150) - metrics.get_stats() # Returns all metrics + metrics.record_search(duration, cached=True) + metrics.record_indexing(repo_id, duration, function_count) + metrics.get_metrics() # dashboard-friendly summary + metrics.get_stats() # raw counters/timings/gauges """ def __init__(self): self._counters: Dict[str, int] = {} self._timings: Dict[str, list] = {} self._gauges: Dict[str, float] = {} + # Domain-specific tracking (replaces PerformanceMetrics) + self._indexing_times: deque = deque(maxlen=100) + self._search_times: deque = deque(maxlen=100) + self._total_searches: int = 0 + self._cache_hits: int = 0 + self._cache_misses: int = 0 def increment(self, name: str, value: int = 1, **tags): """Increment a counter""" - key = f"{name}" - self._counters[key] = self._counters.get(key, 0) + value + self._counters[name] = self._counters.get(name, 0) + value def timing(self, name: str, value_ms: float): """Record a timing measurement""" if name not in self._timings: self._timings[name] = [] self._timings[name].append(value_ms) - # Keep only last 1000 timings if len(self._timings[name]) > 1000: self._timings[name] = self._timings[name][-1000:] def gauge(self, name: str, value: float): - """Record a point-in-time value (like avg score, current queue size)""" + """Record a point-in-time value""" self._gauges[name] = value + def record_indexing(self, repo_id: str, duration: float, function_count: int): + """Record indexing performance for dashboard metrics.""" + self._indexing_times.append({ + "repo_id": repo_id, + "duration": duration, + "function_count": function_count, + "speed": function_count / duration if duration > 0 else 0, + "timestamp": datetime.now().isoformat(), + }) + + def record_search(self, duration: float, cached: bool): + """Record search performance for dashboard metrics.""" + self._search_times.append({ + "duration": duration, + "cached": cached, + "timestamp": datetime.now().isoformat(), + }) + self._total_searches += 1 + if cached: + self._cache_hits += 1 + else: + self._cache_misses += 1 + + def get_metrics(self) -> Dict: + """Dashboard-friendly performance summary (used by /health and /metrics).""" + indexing_speeds = [m["speed"] for m in self._indexing_times] + search_durations = [m["duration"] for m in self._search_times] + cache_hit_rate = ( + (self._cache_hits / self._total_searches * 100) + if self._total_searches > 0 else 0 + ) + + return { + "indexing": { + "total_operations": len(self._indexing_times), + "avg_speed_functions_per_sec": ( + sum(indexing_speeds) / len(indexing_speeds) + if indexing_speeds else 0 + ), + "max_speed": max(indexing_speeds) if indexing_speeds else 0, + "min_speed": min(indexing_speeds) if indexing_speeds else 0, + "recent_operations": list(self._indexing_times)[-10:], + }, + "search": { + "total_searches": self._total_searches, + "cache_hit_rate": f"{cache_hit_rate:.1f}%", + "cache_hits": self._cache_hits, + "cache_misses": self._cache_misses, + "avg_duration_ms": ( + sum(search_durations) / len(search_durations) * 1000 + if search_durations else 0 + ), + "recent_searches": list(self._search_times)[-10:], + }, + "summary": { + "health": "healthy", + "cache_working": cache_hit_rate > 0, + "indexing_performance": ( + "good" if ( + sum(indexing_speeds) / len(indexing_speeds) + if indexing_speeds else 0 + ) > 10 else "needs_improvement" + ), + }, + } + def get_stats(self) -> Dict: - """Get all metrics with basic stats""" + """Raw counters, timings, and gauges for internal debugging.""" stats = { "counters": self._counters.copy(), "gauges": self._gauges.copy(), - "timings": {} + "timings": {}, } - for name, values in self._timings.items(): if values: stats["timings"][name] = { "count": len(values), "avg_ms": round(sum(values) / len(values), 2), "min_ms": round(min(values), 2), - "max_ms": round(max(values), 2) + "max_ms": round(max(values), 2), } - return stats def reset(self): @@ -360,7 +434,99 @@ def reset(self): self._counters = {} self._timings = {} self._gauges = {} + self._indexing_times.clear() + self._search_times.clear() + self._total_searches = 0 + self._cache_hits = 0 + self._cache_misses = 0 + + +# SENTRY INITIALIZATION (moved from services/sentry.py) + +def init_sentry() -> bool: + """Initialize Sentry SDK if SENTRY_DSN is configured.""" + sentry_dsn = os.getenv("SENTRY_DSN") + + if not sentry_dsn: + print("[INFO] Sentry DSN not configured - error tracking disabled") + return False + + try: + import sentry_sdk + from sentry_sdk.integrations.fastapi import FastApiIntegration + from sentry_sdk.integrations.starlette import StarletteIntegration + + environment = os.getenv("ENVIRONMENT", "development") + + sentry_sdk.init( + dsn=sentry_dsn, + environment=environment, + traces_sample_rate=0.1 if environment == "production" else 1.0, + profiles_sample_rate=0.1 if environment == "production" else 1.0, + send_default_pii=True, + integrations=[ + FastApiIntegration(transaction_style="endpoint"), + StarletteIntegration(transaction_style="endpoint"), + ], + before_send=_filter_events, + debug=environment == "development", + attach_stacktrace=True, + include_local_variables=True, + ) + + print(f"[OK] Sentry initialized (environment: {environment})") + return True + + except ImportError: + print("[WARN] sentry-sdk not installed - error tracking disabled") + return False + except Exception as e: + print(f"[WARN] Failed to initialize Sentry: {e}") + return False + + +def _filter_events(event, hint): + """Filter out noisy events before sending to Sentry.""" + request_url = event.get("request", {}).get("url", "") + if "/health" in request_url: + return None + + exception_values = event.get("exception", {}).get("values", []) + if exception_values: + exception_value = str(exception_values[0].get("value", "")) + bot_paths = ["/wp-admin", "/wp-login", "/.env", "/config", "/admin", "/phpmyadmin", "/.git"] + if any(path in exception_value for path in bot_paths): + return None + + if exception_values: + exception_type = exception_values[0].get("type", "") + if exception_type in ("RequestValidationError", "ValidationError"): + return None + + return event + + +def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None): + """Set Sentry user context for error attribution.""" + try: + import sentry_sdk + sentry_sdk.set_user({"id": user_id, "email": email}) + except ImportError: + pass + + +def capture_http_exception(request, exc: Exception, status_code: int): + """Capture HTTP exception with request context for Sentry.""" + try: + import sentry_sdk + with sentry_sdk.push_scope() as scope: + scope.set_extra("status_code", status_code) + scope.set_extra("path", str(request.url.path)) + scope.set_extra("method", request.method) + sentry_sdk.capture_exception(exc) + except ImportError: + pass -# Global metrics instance +# Global instances metrics = Metrics() diff --git a/backend/services/performance_metrics.py b/backend/services/performance_metrics.py deleted file mode 100644 index 77363ec..0000000 --- a/backend/services/performance_metrics.py +++ /dev/null @@ -1,79 +0,0 @@ -""" -Performance Metrics Tracker -Tracks indexing performance, cache hits, and API latency -""" -from typing import Dict, List -from datetime import datetime -from collections import deque -import time - -from services.observability import logger - - -class PerformanceMetrics: - """Track performance metrics for monitoring""" - - def __init__(self): - # Store recent metrics (last 100 operations) - self.indexing_times = deque(maxlen=100) - self.search_times = deque(maxlen=100) - self.cache_hits = 0 - self.cache_misses = 0 - self.total_searches = 0 - - logger.debug("PerformanceMetrics initialized") - - def record_indexing(self, repo_id: str, duration: float, function_count: int): - """Record indexing performance""" - self.indexing_times.append({ - "repo_id": repo_id, - "duration": duration, - "function_count": function_count, - "speed": function_count / duration if duration > 0 else 0, - "timestamp": datetime.now().isoformat() - }) - - def record_search(self, duration: float, cached: bool): - """Record search performance""" - self.search_times.append({ - "duration": duration, - "cached": cached, - "timestamp": datetime.now().isoformat() - }) - - self.total_searches += 1 - if cached: - self.cache_hits += 1 - else: - self.cache_misses += 1 - - def get_metrics(self) -> Dict: - """Get current performance metrics""" - # Calculate statistics - indexing_speeds = [m["speed"] for m in self.indexing_times] - search_durations = [m["duration"] for m in self.search_times] - - cache_hit_rate = (self.cache_hits / self.total_searches * 100) if self.total_searches > 0 else 0 - - return { - "indexing": { - "total_operations": len(self.indexing_times), - "avg_speed_functions_per_sec": sum(indexing_speeds) / len(indexing_speeds) if indexing_speeds else 0, - "max_speed": max(indexing_speeds) if indexing_speeds else 0, - "min_speed": min(indexing_speeds) if indexing_speeds else 0, - "recent_operations": list(self.indexing_times)[-10:] - }, - "search": { - "total_searches": self.total_searches, - "cache_hit_rate": f"{cache_hit_rate:.1f}%", - "cache_hits": self.cache_hits, - "cache_misses": self.cache_misses, - "avg_duration_ms": sum(search_durations) / len(search_durations) * 1000 if search_durations else 0, - "recent_searches": list(self.search_times)[-10:] - }, - "summary": { - "health": "healthy", - "cache_working": cache_hit_rate > 0, - "indexing_performance": "good" if (sum(indexing_speeds) / len(indexing_speeds) if indexing_speeds else 0) > 10 else "needs_improvement" - } - } diff --git a/backend/services/playground_limiter.py b/backend/services/playground_limiter.py index 817c52b..6465e05 100644 --- a/backend/services/playground_limiter.py +++ b/backend/services/playground_limiter.py @@ -21,8 +21,7 @@ from typing import Optional, Tuple, Dict, Any from dataclasses import dataclass -from services.observability import logger, metrics, track_time -from services.sentry import capture_exception +from services.observability import logger, metrics, track_time, capture_exception # DATA CLASSES diff --git a/backend/services/repo_validator.py b/backend/services/repo_validator.py index 2a32eaa..65213b1 100644 --- a/backend/services/repo_validator.py +++ b/backend/services/repo_validator.py @@ -9,8 +9,7 @@ from typing import Set, Optional import random -from services.observability import logger -from services.sentry import capture_exception +from services.observability import logger, capture_exception @dataclass diff --git a/backend/services/sentry.py b/backend/services/sentry.py deleted file mode 100644 index bfbddc5..0000000 --- a/backend/services/sentry.py +++ /dev/null @@ -1,175 +0,0 @@ -""" -Sentry Error Tracking Integration -Provides production error visibility and performance monitoring - -NOTE: This module initializes Sentry. For logging and tracing, -use the observability module: from services.observability import get_logger, trace_operation -""" -import os -from typing import Optional - - -def init_sentry() -> bool: - """ - Initialize Sentry SDK if SENTRY_DSN is configured. - - Returns: - bool: True if Sentry was initialized, False otherwise - """ - sentry_dsn = os.getenv("SENTRY_DSN") - - if not sentry_dsn: - print("[INFO] Sentry DSN not configured - error tracking disabled") - return False - - try: - import sentry_sdk - from sentry_sdk.integrations.fastapi import FastApiIntegration - from sentry_sdk.integrations.starlette import StarletteIntegration - - environment = os.getenv("ENVIRONMENT", "development") - - sentry_sdk.init( - dsn=sentry_dsn, - environment=environment, - - # Performance monitoring - sample rate based on environment - traces_sample_rate=0.1 if environment == "production" else 1.0, - - # Profile sampled transactions - profiles_sample_rate=0.1 if environment == "production" else 1.0, - - # Send PII for debugging (user IDs, emails) - send_default_pii=True, - - # Integrations - integrations=[ - FastApiIntegration(transaction_style="endpoint"), - StarletteIntegration(transaction_style="endpoint"), - ], - - # Filter noisy events - before_send=_filter_events, - - # Debug mode for development - debug=environment == "development", - - # Attach stack traces to messages - attach_stacktrace=True, - - # Include local variables in stack traces - include_local_variables=True, - ) - - print(f"[OK] Sentry initialized (environment: {environment})") - return True - - except ImportError: - print("[WARN] sentry-sdk not installed - error tracking disabled") - return False - except Exception as e: - print(f"[WARN] Failed to initialize Sentry: {e}") - return False - - -def _filter_events(event, hint): - """Filter out noisy events before sending to Sentry.""" - - # Don't send health check errors - request_url = event.get("request", {}).get("url", "") - if "/health" in request_url: - return None - - # Don't send 404s for common bot paths - exception_values = event.get("exception", {}).get("values", []) - if exception_values: - exception_value = str(exception_values[0].get("value", "")) - bot_paths = ["/wp-admin", "/wp-login", "/.env", "/config", "/admin", "/phpmyadmin", "/.git"] - if any(path in exception_value for path in bot_paths): - return None - - # Don't send validation errors (they're expected) - if exception_values: - exception_type = exception_values[0].get("type", "") - if exception_type in ("RequestValidationError", "ValidationError"): - return None - - return event - - -# LEGACY FUNCTIONS - Use observability module for new code - -def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None): - """ - Set user context for error tracking. - - DEPRECATED: Use from services.observability import set_user_context - """ - try: - import sentry_sdk - sentry_sdk.set_user({"id": user_id, "email": email}) - except ImportError: - pass - - -def capture_exception(error: Exception, **extra_context): - """ - Manually capture an exception with additional context. - - DEPRECATED: Use from services.observability import capture_exception - """ - try: - import sentry_sdk - with sentry_sdk.push_scope() as scope: - for key, value in extra_context.items(): - scope.set_extra(key, value) - sentry_sdk.capture_exception(error) - except ImportError: - pass - - -def capture_message(message: str, level: str = "info", **extra_context): - """ - Capture a message (not an exception) for tracking. - - DEPRECATED: Use from services.observability import get_logger - """ - try: - import sentry_sdk - with sentry_sdk.push_scope() as scope: - for key, value in extra_context.items(): - scope.set_extra(key, value) - sentry_sdk.capture_message(message, level=level) - except ImportError: - pass - - -def set_operation_context(operation: str, **tags): - """ - Set operation context for the current scope. - - DEPRECATED: Use from services.observability import trace_operation - """ - try: - import sentry_sdk - sentry_sdk.set_tag("operation", operation) - for key, value in tags.items(): - sentry_sdk.set_tag(key, str(value)) - except ImportError: - pass - - -def capture_http_exception(request, exc: Exception, status_code: int): - """ - Capture HTTP exception with request context for error tracking. - """ - try: - import sentry_sdk - with sentry_sdk.push_scope() as scope: - scope.set_extra("status_code", status_code) - scope.set_extra("path", str(request.url.path)) - scope.set_extra("method", request.method) - sentry_sdk.capture_exception(exc) - except ImportError: - pass - pass diff --git a/backend/services/user_limits.py b/backend/services/user_limits.py index 8075c13..fe63cda 100644 --- a/backend/services/user_limits.py +++ b/backend/services/user_limits.py @@ -16,8 +16,7 @@ from typing import Optional, Dict, Any from enum import Enum -from services.observability import logger, metrics -from services.sentry import capture_exception +from services.observability import logger, metrics, capture_exception class UserTier(str, Enum): From ad320245b5eecb7d1ebae77e01b08c5c5319d103 Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Mon, 23 Feb 2026 17:08:02 -0500 Subject: [PATCH 2/5] fix: remove double-logging from capture_exception -- saves log ingestion cost capture_exception() was calling logger.error() internally, but every caller (50 call sites) also calls logger.error() before/after it. Result: every error printed to stdout twice, doubling log volume for errors in production log aggregators. Now capture_exception() only sends to Sentry. Callers handle their own logging. Fallback logger.error() only fires when Sentry SDK is not installed (so errors aren't silently lost in local dev). 284 tests pass. --- backend/services/observability.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/backend/services/observability.py b/backend/services/observability.py index f526e02..3c50435 100644 --- a/backend/services/observability.py +++ b/backend/services/observability.py @@ -155,11 +155,10 @@ def add_breadcrumb(message: str, category: str = "custom", level: str = "info", def capture_exception(error: Exception, **context): """ - Capture exception with additional context. + Capture exception to Sentry with additional context. - Args: - error: The exception to capture - **context: Additional context to attach + Does NOT log to stdout -- callers are responsible for logging. + This avoids double-logging when callers do logger.error() + capture_exception(). """ try: import sentry_sdk @@ -167,14 +166,9 @@ def capture_exception(error: Exception, **context): for key, value in context.items(): scope.set_extra(key, value) sentry_sdk.capture_exception(error) - - # Also log it - logger.error( - f"Exception captured: {type(error).__name__}: {str(error)}", - **context - ) except ImportError: - logger.error(f"Exception: {error}", **context) + # No Sentry -- log as fallback so errors aren't silently lost + logger.error(f"Exception (no Sentry): {type(error).__name__}: {error}", **context) def capture_message(message: str, level: str = "info", **context): From d28c4714cbf8bf629a68c0d2b8ce4427ace2c9a0 Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Mon, 23 Feb 2026 17:28:33 -0500 Subject: [PATCH 3/5] 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. --- backend/services/observability.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/backend/services/observability.py b/backend/services/observability.py index 3c50435..a7ed428 100644 --- a/backend/services/observability.py +++ b/backend/services/observability.py @@ -313,7 +313,7 @@ class Metrics: metrics.get_stats() # raw counters/timings/gauges """ - def __init__(self): + def __init__(self) -> None: self._counters: Dict[str, int] = {} self._timings: Dict[str, list] = {} self._gauges: Dict[str, float] = {} @@ -324,11 +324,11 @@ def __init__(self): self._cache_hits: int = 0 self._cache_misses: int = 0 - def increment(self, name: str, value: int = 1, **tags): + def increment(self, name: str, value: int = 1, **tags) -> None: """Increment a counter""" self._counters[name] = self._counters.get(name, 0) + value - def timing(self, name: str, value_ms: float): + def timing(self, name: str, value_ms: float) -> None: """Record a timing measurement""" if name not in self._timings: self._timings[name] = [] @@ -336,11 +336,11 @@ def timing(self, name: str, value_ms: float): if len(self._timings[name]) > 1000: self._timings[name] = self._timings[name][-1000:] - def gauge(self, name: str, value: float): + def gauge(self, name: str, value: float) -> None: """Record a point-in-time value""" self._gauges[name] = value - def record_indexing(self, repo_id: str, duration: float, function_count: int): + def record_indexing(self, repo_id: str, duration: float, function_count: int) -> None: """Record indexing performance for dashboard metrics.""" self._indexing_times.append({ "repo_id": repo_id, @@ -350,7 +350,7 @@ def record_indexing(self, repo_id: str, duration: float, function_count: int): "timestamp": datetime.now().isoformat(), }) - def record_search(self, duration: float, cached: bool): + def record_search(self, duration: float, cached: bool) -> None: """Record search performance for dashboard metrics.""" self._search_times.append({ "duration": duration, @@ -423,7 +423,7 @@ def get_stats(self) -> Dict: } return stats - def reset(self): + def reset(self) -> None: """Reset all metrics""" self._counters = {} self._timings = {} @@ -452,12 +452,16 @@ def init_sentry() -> bool: environment = os.getenv("ENVIRONMENT", "development") + # PII and debug settings -- opt-in via env vars, default to safe + send_pii = os.getenv("SENTRY_SEND_PII", "false").lower() in ("true", "1") + include_locals = os.getenv("SENTRY_INCLUDE_LOCAL_VARS", "false").lower() in ("true", "1") + sentry_sdk.init( dsn=sentry_dsn, environment=environment, traces_sample_rate=0.1 if environment == "production" else 1.0, profiles_sample_rate=0.1 if environment == "production" else 1.0, - send_default_pii=True, + send_default_pii=send_pii, integrations=[ FastApiIntegration(transaction_style="endpoint"), StarletteIntegration(transaction_style="endpoint"), @@ -465,7 +469,7 @@ def init_sentry() -> bool: before_send=_filter_events, debug=environment == "development", attach_stacktrace=True, - include_local_variables=True, + include_local_variables=include_locals, ) print(f"[OK] Sentry initialized (environment: {environment})") @@ -479,7 +483,7 @@ def init_sentry() -> bool: return False -def _filter_events(event, hint): +def _filter_events(event: Dict[str, Any], hint: Optional[Dict[str, Any]]) -> Optional[Dict[str, Any]]: """Filter out noisy events before sending to Sentry.""" request_url = event.get("request", {}).get("url", "") if "/health" in request_url: @@ -500,7 +504,7 @@ def _filter_events(event, hint): return event -def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None): +def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None) -> None: """Set Sentry user context for error attribution.""" try: import sentry_sdk @@ -509,7 +513,7 @@ def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None) pass -def capture_http_exception(request, exc: Exception, status_code: int): +def capture_http_exception(request: Any, exc: Exception, status_code: int) -> None: """Capture HTTP exception with request context for Sentry.""" try: import sentry_sdk From b0acc9c65c8a1613ba223df5eb9300fdc1674e1b Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Mon, 23 Feb 2026 17:39:57 -0500 Subject: [PATCH 4/5] fix: review round 3 -- 8 fixes across observability.py 1. Double cache counting: record_search no longer tracks cache_hits/ cache_misses (cache.py already does via metrics.increment). get_metrics now reads from _counters. Eliminates double counting. 2. push_scope -> new_scope: 3 occurrences updated for sentry-sdk 2.x compatibility. Test warnings dropped from 54 to 44. 3. datetime.utcnow() -> datetime.now(timezone.utc): 3 occurrences. utcnow() deprecated in Python 3.12+. 4. Type annotations: added -> None to 13 functions (StructuredLogger methods + Sentry helpers) per CLAUDE.md. 5. deque for _timings: O(1) append instead of O(n) slice for cap. 6. get_metrics: avg_indexing_speed computed once, reused in summary. 7. _filter_events: merged duplicate if blocks into one. Skipped (3rd+ time): lazy singleton, contextvars, async/sync wrapper factoring. 284 tests pass. 10 fewer warnings. --- backend/services/observability.py | 79 +++++++++++++------------------ 1 file changed, 34 insertions(+), 45 deletions(-) diff --git a/backend/services/observability.py b/backend/services/observability.py index a7ed428..60ad818 100644 --- a/backend/services/observability.py +++ b/backend/services/observability.py @@ -23,7 +23,7 @@ async def index_repo(repo_id: str): from typing import Optional, Any, Dict from functools import wraps from contextlib import contextmanager -from datetime import datetime +from datetime import datetime, timezone from collections import deque # Environment @@ -43,7 +43,7 @@ class StructuredLogger: logger.error("Failed to index", repo_id="xyz", error=str(e)) """ - def __init__(self, name: str = "codeintel"): + def __init__(self, name: str = "codeintel") -> None: self.name = name self.level = getattr(logging, LOG_LEVEL.upper(), logging.INFO) self._context: Dict[str, Any] = {} @@ -51,7 +51,7 @@ def __init__(self, name: str = "codeintel"): def _format_message(self, level: str, message: str, **kwargs) -> str: """Format log message based on environment""" data = { - "timestamp": datetime.utcnow().isoformat(), + "timestamp": datetime.now(timezone.utc).isoformat(), "level": level, "service": self.name, "message": message, @@ -73,7 +73,7 @@ def _format_message(self, level: str, message: str, **kwargs) -> str: parts.append(extras) return " ".join(parts) - def _log(self, level: str, level_num: int, message: str, **kwargs): + def _log(self, level: str, level_num: int, message: str, **kwargs) -> None: """Internal log method""" if level_num < self.level: return @@ -84,27 +84,27 @@ def _log(self, level: str, level_num: int, message: str, **kwargs): output = sys.stderr if level_num >= logging.ERROR else sys.stdout print(formatted, file=output) - def set_context(self, **kwargs): + def set_context(self, **kwargs) -> None: """Set persistent context for all subsequent logs""" self._context.update(kwargs) - def clear_context(self): + def clear_context(self) -> None: """Clear all context""" self._context = {} - def debug(self, message: str, **kwargs): + def debug(self, message: str, **kwargs) -> None: self._log("DEBUG", logging.DEBUG, message, **kwargs) - def info(self, message: str, **kwargs): + def info(self, message: str, **kwargs) -> None: self._log("INFO", logging.INFO, message, **kwargs) - def warning(self, message: str, **kwargs): + def warning(self, message: str, **kwargs) -> None: self._log("WARNING", logging.WARNING, message, **kwargs) - def error(self, message: str, **kwargs): + def error(self, message: str, **kwargs) -> None: self._log("ERROR", logging.ERROR, message, **kwargs) - def critical(self, message: str, **kwargs): + def critical(self, message: str, **kwargs) -> None: self._log("CRITICAL", logging.CRITICAL, message, **kwargs) @@ -114,7 +114,7 @@ def critical(self, message: str, **kwargs): # SENTRY INTEGRATION HELPERS -def set_operation_context(operation: str, **kwargs): +def set_operation_context(operation: str, **kwargs) -> None: """ Set Sentry context for current operation. @@ -135,7 +135,7 @@ def set_operation_context(operation: str, **kwargs): pass -def add_breadcrumb(message: str, category: str = "custom", level: str = "info", **data): +def add_breadcrumb(message: str, category: str = "custom", level: str = "info", **data) -> None: """ Add breadcrumb for Sentry error context. @@ -153,7 +153,7 @@ def add_breadcrumb(message: str, category: str = "custom", level: str = "info", pass -def capture_exception(error: Exception, **context): +def capture_exception(error: Exception, **context) -> None: """ Capture exception to Sentry with additional context. @@ -162,7 +162,7 @@ def capture_exception(error: Exception, **context): """ try: import sentry_sdk - with sentry_sdk.push_scope() as scope: + with sentry_sdk.new_scope() as scope: for key, value in context.items(): scope.set_extra(key, value) sentry_sdk.capture_exception(error) @@ -171,11 +171,11 @@ def capture_exception(error: Exception, **context): logger.error(f"Exception (no Sentry): {type(error).__name__}: {error}", **context) -def capture_message(message: str, level: str = "info", **context): +def capture_message(message: str, level: str = "info", **context) -> None: """Capture a message (not exception) to Sentry""" try: import sentry_sdk - with sentry_sdk.push_scope() as scope: + with sentry_sdk.new_scope() as scope: for key, value in context.items(): scope.set_extra(key, value) sentry_sdk.capture_message(message, level=level) @@ -321,8 +321,6 @@ def __init__(self) -> None: self._indexing_times: deque = deque(maxlen=100) self._search_times: deque = deque(maxlen=100) self._total_searches: int = 0 - self._cache_hits: int = 0 - self._cache_misses: int = 0 def increment(self, name: str, value: int = 1, **tags) -> None: """Increment a counter""" @@ -331,10 +329,8 @@ def increment(self, name: str, value: int = 1, **tags) -> None: def timing(self, name: str, value_ms: float) -> None: """Record a timing measurement""" if name not in self._timings: - self._timings[name] = [] + self._timings[name] = deque(maxlen=1000) self._timings[name].append(value_ms) - if len(self._timings[name]) > 1000: - self._timings[name] = self._timings[name][-1000:] def gauge(self, name: str, value: float) -> None: """Record a point-in-time value""" @@ -347,7 +343,7 @@ def record_indexing(self, repo_id: str, duration: float, function_count: int) -> "duration": duration, "function_count": function_count, "speed": function_count / duration if duration > 0 else 0, - "timestamp": datetime.now().isoformat(), + "timestamp": datetime.now(timezone.utc).isoformat(), }) def record_search(self, duration: float, cached: bool) -> None: @@ -355,30 +351,29 @@ def record_search(self, duration: float, cached: bool) -> None: self._search_times.append({ "duration": duration, "cached": cached, - "timestamp": datetime.now().isoformat(), + "timestamp": datetime.now(timezone.utc).isoformat(), }) self._total_searches += 1 - if cached: - self._cache_hits += 1 - else: - self._cache_misses += 1 + # cache hit/miss counting handled by cache.py via metrics.increment() + # to avoid double counting now that we're a single Metrics instance def get_metrics(self) -> Dict: """Dashboard-friendly performance summary (used by /health and /metrics).""" indexing_speeds = [m["speed"] for m in self._indexing_times] search_durations = [m["duration"] for m in self._search_times] - cache_hit_rate = ( - (self._cache_hits / self._total_searches * 100) - if self._total_searches > 0 else 0 + avg_indexing_speed = ( + sum(indexing_speeds) / len(indexing_speeds) if indexing_speeds else 0 ) + # Cache hits/misses come from _counters (set by cache.py) + cache_hits = self._counters.get("cache_hits", 0) + cache_misses = self._counters.get("cache_misses", 0) + cache_total = cache_hits + cache_misses + cache_hit_rate = (cache_hits / cache_total * 100) if cache_total > 0 else 0 return { "indexing": { "total_operations": len(self._indexing_times), - "avg_speed_functions_per_sec": ( - sum(indexing_speeds) / len(indexing_speeds) - if indexing_speeds else 0 - ), + "avg_speed_functions_per_sec": avg_indexing_speed, "max_speed": max(indexing_speeds) if indexing_speeds else 0, "min_speed": min(indexing_speeds) if indexing_speeds else 0, "recent_operations": list(self._indexing_times)[-10:], @@ -386,8 +381,8 @@ def get_metrics(self) -> Dict: "search": { "total_searches": self._total_searches, "cache_hit_rate": f"{cache_hit_rate:.1f}%", - "cache_hits": self._cache_hits, - "cache_misses": self._cache_misses, + "cache_hits": cache_hits, + "cache_misses": cache_misses, "avg_duration_ms": ( sum(search_durations) / len(search_durations) * 1000 if search_durations else 0 @@ -398,10 +393,7 @@ def get_metrics(self) -> Dict: "health": "healthy", "cache_working": cache_hit_rate > 0, "indexing_performance": ( - "good" if ( - sum(indexing_speeds) / len(indexing_speeds) - if indexing_speeds else 0 - ) > 10 else "needs_improvement" + "good" if avg_indexing_speed > 10 else "needs_improvement" ), }, } @@ -431,8 +423,6 @@ def reset(self) -> None: self._indexing_times.clear() self._search_times.clear() self._total_searches = 0 - self._cache_hits = 0 - self._cache_misses = 0 # SENTRY INITIALIZATION (moved from services/sentry.py) @@ -496,7 +486,6 @@ def _filter_events(event: Dict[str, Any], hint: Optional[Dict[str, Any]]) -> Opt if any(path in exception_value for path in bot_paths): return None - if exception_values: exception_type = exception_values[0].get("type", "") if exception_type in ("RequestValidationError", "ValidationError"): return None @@ -517,7 +506,7 @@ def capture_http_exception(request: Any, exc: Exception, status_code: int) -> No """Capture HTTP exception with request context for Sentry.""" try: import sentry_sdk - with sentry_sdk.push_scope() as scope: + with sentry_sdk.new_scope() as scope: scope.set_extra("status_code", status_code) scope.set_extra("path", str(request.url.path)) scope.set_extra("method", request.method) From d6d88a01375a952cb6437883cc7f10d125dd70c2 Mon Sep 17 00:00:00 2001 From: Devanshu Rajesh Chicholikar Date: Mon, 23 Feb 2026 18:10:50 -0500 Subject: [PATCH 5/5] fix: review round 4 -- type accuracy, PII enforcement, indexing counter 1. _timings type: Dict[str, list] -> Dict[str, Deque[float]] to match actual deque storage. Added Deque to typing imports. 2. set_user_context: email now only sent to Sentry when SENTRY_SEND_PII is explicitly enabled. Was bypassing the PII flag we added. 3. startup_checks: added SENTRY_SEND_PII and SENTRY_INCLUDE_LOCAL_VARS to OPTIONAL_VARS so missing config gets logged at startup. 4. get_metrics/get_stats: return type -> Dict[str, Any] for precision. 5. _total_indexing_ops: persistent counter replaces len(deque) which was capped at 100. Dashboard now shows true total. Skipped: lazy singleton (4th time), print->logger in init_sentry (runs before logger is ready at bootstrap). 284 tests pass. --- backend/config/startup_checks.py | 2 ++ backend/services/observability.py | 20 +++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/backend/config/startup_checks.py b/backend/config/startup_checks.py index 8e75aaa..9b3cd43 100644 --- a/backend/config/startup_checks.py +++ b/backend/config/startup_checks.py @@ -23,6 +23,8 @@ ("COHERE_API_KEY", "Cohere API key for reranking", "Search reranking disabled"), ("VOYAGE_API_KEY", "Voyage AI key for code embeddings", "Using OpenAI embeddings"), ("SENTRY_DSN", "Sentry DSN for error tracking", "Error tracking disabled"), + ("SENTRY_SEND_PII", "Send user emails to Sentry", "PII disabled (privacy safe)"), + ("SENTRY_INCLUDE_LOCAL_VARS", "Include local vars in Sentry traces", "Local vars excluded"), ("REDIS_HOST", "Redis host for caching", "Using default localhost"), ] diff --git a/backend/services/observability.py b/backend/services/observability.py index 60ad818..5ef95b5 100644 --- a/backend/services/observability.py +++ b/backend/services/observability.py @@ -20,7 +20,7 @@ async def index_repo(repo_id: str): import time import logging import json -from typing import Optional, Any, Dict +from typing import Optional, Any, Dict, Deque from functools import wraps from contextlib import contextmanager from datetime import datetime, timezone @@ -315,12 +315,13 @@ class Metrics: def __init__(self) -> None: self._counters: Dict[str, int] = {} - self._timings: Dict[str, list] = {} + self._timings: Dict[str, Deque[float]] = {} self._gauges: Dict[str, float] = {} # Domain-specific tracking (replaces PerformanceMetrics) self._indexing_times: deque = deque(maxlen=100) self._search_times: deque = deque(maxlen=100) self._total_searches: int = 0 + self._total_indexing_ops: int = 0 def increment(self, name: str, value: int = 1, **tags) -> None: """Increment a counter""" @@ -345,6 +346,7 @@ def record_indexing(self, repo_id: str, duration: float, function_count: int) -> "speed": function_count / duration if duration > 0 else 0, "timestamp": datetime.now(timezone.utc).isoformat(), }) + self._total_indexing_ops += 1 def record_search(self, duration: float, cached: bool) -> None: """Record search performance for dashboard metrics.""" @@ -357,14 +359,13 @@ def record_search(self, duration: float, cached: bool) -> None: # cache hit/miss counting handled by cache.py via metrics.increment() # to avoid double counting now that we're a single Metrics instance - def get_metrics(self) -> Dict: + def get_metrics(self) -> Dict[str, Any]: """Dashboard-friendly performance summary (used by /health and /metrics).""" indexing_speeds = [m["speed"] for m in self._indexing_times] search_durations = [m["duration"] for m in self._search_times] avg_indexing_speed = ( sum(indexing_speeds) / len(indexing_speeds) if indexing_speeds else 0 ) - # Cache hits/misses come from _counters (set by cache.py) cache_hits = self._counters.get("cache_hits", 0) cache_misses = self._counters.get("cache_misses", 0) cache_total = cache_hits + cache_misses @@ -372,7 +373,7 @@ def get_metrics(self) -> Dict: return { "indexing": { - "total_operations": len(self._indexing_times), + "total_operations": self._total_indexing_ops, "avg_speed_functions_per_sec": avg_indexing_speed, "max_speed": max(indexing_speeds) if indexing_speeds else 0, "min_speed": min(indexing_speeds) if indexing_speeds else 0, @@ -398,7 +399,7 @@ def get_metrics(self) -> Dict: }, } - def get_stats(self) -> Dict: + def get_stats(self) -> Dict[str, Any]: """Raw counters, timings, and gauges for internal debugging.""" stats = { "counters": self._counters.copy(), @@ -423,6 +424,7 @@ def reset(self) -> None: self._indexing_times.clear() self._search_times.clear() self._total_searches = 0 + self._total_indexing_ops = 0 # SENTRY INITIALIZATION (moved from services/sentry.py) @@ -497,7 +499,11 @@ def set_user_context(user_id: Optional[str] = None, email: Optional[str] = None) """Set Sentry user context for error attribution.""" try: import sentry_sdk - sentry_sdk.set_user({"id": user_id, "email": email}) + user_data: Dict[str, Any] = {"id": user_id} + # Only include email if PII opt-in is enabled + if email and os.getenv("SENTRY_SEND_PII", "false").lower() in ("true", "1"): + user_data["email"] = email + sentry_sdk.set_user(user_data) except ImportError: pass