Added interface layer top of http client to use circuit rbeaker

Signed-off-by: Nikhil Suri <nikhil.suri@databricks.com>

Author: nikhilsuri-db

docs/parameters.md

@@ -254,73 +254,3 @@ You should only set `use_inline_params=True` in the following cases:
 4. Your client code uses [sequences as parameter values](#passing-sequences-as-parameter-values)
 
 We expect limitations (1) and (2) to be addressed in a future Databricks Runtime release.
-
-# Telemetry Circuit Breaker Configuration
-
-The Databricks SQL connector includes a circuit breaker pattern for telemetry requests to prevent telemetry failures from impacting main SQL operations. This feature is enabled by default and can be controlled through a connection parameter.
-
-## Overview
-
-The circuit breaker monitors telemetry request failures and automatically blocks telemetry requests when the failure rate exceeds a configured threshold. This prevents telemetry service issues from affecting your main SQL operations.
-
-## Configuration Parameter
-
-| Parameter | Type | Default | Description |
-|-----------|------|---------|-------------|
-| `telemetry_circuit_breaker_enabled` | bool | `True` | Enable or disable the telemetry circuit breaker |
-
-## Usage Examples
-
-### Default Configuration (Circuit Breaker Enabled)
-
-```python
-from databricks import sql
-
-# Circuit breaker is enabled by default
-with sql.connect(
-    server_hostname="your-host.cloud.databricks.com",
-    http_path="/sql/1.0/warehouses/your-warehouse-id",
-    access_token="your-token"
-) as conn:
-    # Your SQL operations here
-    pass
-```
-
-### Disable Circuit Breaker
-
-```python
-from databricks import sql
-
-# Disable circuit breaker entirely
-with sql.connect(
-    server_hostname="your-host.cloud.databricks.com",
-    http_path="/sql/1.0/warehouses/your-warehouse-id",
-    access_token="your-token",
-    telemetry_circuit_breaker_enabled=False
-) as conn:
-    # Your SQL operations here
-    pass
-```
-
-## Circuit Breaker States
-
-The circuit breaker operates in three states:
-
-1. **Closed**: Normal operation, telemetry requests are allowed
-2. **Open**: Circuit breaker is open, telemetry requests are blocked
-3. **Half-Open**: Testing state, limited telemetry requests are allowed
-
-
-## Performance Impact
-
-The circuit breaker has minimal performance impact on SQL operations:
-
-- Circuit breaker only affects telemetry requests, not SQL queries
-- When circuit breaker is open, telemetry requests are simply skipped
-- No additional latency is added to successful operations
-
-## Best Practices
-
-1. **Keep circuit breaker enabled**: The default configuration works well for most use cases
-2. **Don't disable unless necessary**: Circuit breaker provides important protection against telemetry failures
-3. **Monitor application logs**: Circuit breaker state changes are logged for troubleshooting

src/databricks/sql/auth/common.py

@@ -51,7 +51,6 @@ def __init__(
         pool_connections: Optional[int] = None,
         pool_maxsize: Optional[int] = None,
         user_agent: Optional[str] = None,
-        # Telemetry circuit breaker configuration
         telemetry_circuit_breaker_enabled: Optional[bool] = None,
     ):
         self.hostname = hostname
@@ -85,9 +84,7 @@ def __init__(
         self.pool_connections = pool_connections or 10
         self.pool_maxsize = pool_maxsize or 20
         self.user_agent = user_agent
-        
-        # Telemetry circuit breaker configuration
-        self.telemetry_circuit_breaker_enabled = telemetry_circuit_breaker_enabled if telemetry_circuit_breaker_enabled is not None else True
+        self.telemetry_circuit_breaker_enabled = telemetry_circuit_breaker_enabled if telemetry_circuit_breaker_enabled is not None else False
 
 
 def get_effective_azure_login_app_id(hostname) -> str:

src/databricks/sql/telemetry/circuit_breaker_manager.py

@@ -16,28 +16,53 @@
 
 logger = logging.getLogger(__name__)
 
+# Circuit Breaker Configuration Constants
+DEFAULT_FAILURE_THRESHOLD = 0.5
+DEFAULT_MINIMUM_CALLS = 20
+DEFAULT_TIMEOUT = 30
+DEFAULT_RESET_TIMEOUT = 30
+DEFAULT_EXPECTED_EXCEPTION = (Exception,)
+DEFAULT_NAME = "telemetry-circuit-breaker"
 
-@dataclass
+# Circuit Breaker State Constants
+CIRCUIT_BREAKER_STATE_OPEN = "open"
+CIRCUIT_BREAKER_STATE_CLOSED = "closed"
+CIRCUIT_BREAKER_STATE_HALF_OPEN = "half-open"
+CIRCUIT_BREAKER_STATE_DISABLED = "disabled"
+CIRCUIT_BREAKER_STATE_NOT_INITIALIZED = "not_initialized"
+
+# Logging Message Constants
+LOG_CIRCUIT_BREAKER_STATE_CHANGED = "Circuit breaker state changed from %s to %s for %s"
+LOG_CIRCUIT_BREAKER_OPENED = "Circuit breaker opened for %s - telemetry requests will be blocked"
+LOG_CIRCUIT_BREAKER_CLOSED = "Circuit breaker closed for %s - telemetry requests will be allowed"
+LOG_CIRCUIT_BREAKER_HALF_OPEN = "Circuit breaker half-open for %s - testing telemetry requests"
+
+
+@dataclass(frozen=True)
 class CircuitBreakerConfig:
-    """Configuration for circuit breaker behavior."""
+    """Configuration for circuit breaker behavior.
+    
+    This class is immutable to prevent modification of circuit breaker settings.
+    All configuration values are set to constants defined at the module level.
+    """
 
     # Failure threshold percentage (0.0 to 1.0)
-    failure_threshold: float = 0.5
+    failure_threshold: float = DEFAULT_FAILURE_THRESHOLD
 
     # Minimum number of calls before circuit can open
-    minimum_calls: int = 20
+    minimum_calls: int = DEFAULT_MINIMUM_CALLS
 
     # Time window for counting failures (in seconds)
-    timeout: int = 30
+    timeout: int = DEFAULT_TIMEOUT
 
     # Time to wait before trying to close circuit (in seconds)
-    reset_timeout: int = 30
+    reset_timeout: int = DEFAULT_RESET_TIMEOUT
 
     # Expected exception types that should trigger circuit breaker
-    expected_exception: tuple = (Exception,)
+    expected_exception: tuple = DEFAULT_EXPECTED_EXCEPTION
 
     # Name for the circuit breaker (for logging)
-    name: str = "telemetry-circuit-breaker"
+    name: str = DEFAULT_NAME
 
 
 class CircuitBreakerManager:
@@ -142,23 +167,23 @@ def _on_state_change(cls, old_state: str, new_state: str, breaker: CircuitBreake
             breaker: The circuit breaker instance
         """
         logger.info(
-            "Circuit breaker state changed from %s to %s for %s",
+            LOG_CIRCUIT_BREAKER_STATE_CHANGED,
             old_state, new_state, breaker.name
         )
 
-        if new_state == "open":
+        if new_state == CIRCUIT_BREAKER_STATE_OPEN:
             logger.warning(
-                "Circuit breaker opened for %s - telemetry requests will be blocked",
+                LOG_CIRCUIT_BREAKER_OPENED,
                 breaker.name
             )
-        elif new_state == "closed":
+        elif new_state == CIRCUIT_BREAKER_STATE_CLOSED:
             logger.info(
-                "Circuit breaker closed for %s - telemetry requests will be allowed",
+                LOG_CIRCUIT_BREAKER_CLOSED,
                 breaker.name
             )
-        elif new_state == "half-open":
+        elif new_state == CIRCUIT_BREAKER_STATE_HALF_OPEN:
             logger.info(
-                "Circuit breaker half-open for %s - testing telemetry requests",
+                LOG_CIRCUIT_BREAKER_HALF_OPEN,
                 breaker.name
             )
 
@@ -174,11 +199,11 @@ def get_circuit_breaker_state(cls, host: str) -> str:
             Current state of the circuit breaker
         """
         if not cls._config:
-            return "disabled"
+            return CIRCUIT_BREAKER_STATE_DISABLED
 
         with cls._lock:
             if host not in cls._instances:
-                return "not_initialized"
+                return CIRCUIT_BREAKER_STATE_NOT_INITIALIZED
 
             breaker = cls._instances[host]
             return breaker.current_state

src/databricks/sql/telemetry/telemetry_client.py

@@ -393,7 +393,6 @@ def close(self):
         """Flush remaining events before closing"""
         logger.debug("Closing TelemetryClient for connection %s", self._session_id_hex)
         self._flush()
-    
 
 
 class TelemetryClientFactory:

src/databricks/sql/telemetry/telemetry_push_client.py

@@ -16,7 +16,12 @@
 
 from databricks.sql.common.unified_http_client import UnifiedHttpClient
 from databricks.sql.common.http import HttpMethod
-from databricks.sql.telemetry.circuit_breaker_manager import CircuitBreakerConfig, is_circuit_breaker_error
+from databricks.sql.telemetry.circuit_breaker_manager import (
+    CircuitBreakerConfig, 
+    CircuitBreakerManager, 
+    is_circuit_breaker_error,
+    CIRCUIT_BREAKER_STATE_OPEN
+)
 
 logger = logging.getLogger(__name__)
 
@@ -133,7 +138,6 @@ def __init__(
         self._config = config
 
         # Initialize circuit breaker manager with config
-        from databricks.sql.telemetry.circuit_breaker_manager import CircuitBreakerManager
         CircuitBreakerManager.initialize(config)
 
         # Get circuit breaker for this host
@@ -200,14 +204,14 @@ def request_context(
 
     def get_circuit_breaker_state(self) -> str:
         """Get the current state of the circuit breaker."""
-        from databricks.sql.telemetry.circuit_breaker_manager import CircuitBreakerManager
         return CircuitBreakerManager.get_circuit_breaker_state(self._host)
 
     def is_circuit_breaker_open(self) -> bool:
         """Check if the circuit breaker is currently open."""
-        return self.get_circuit_breaker_state() == "open"
+        return self.get_circuit_breaker_state() == CIRCUIT_BREAKER_STATE_OPEN
 
     def reset_circuit_breaker(self) -> None:
         """Reset the circuit breaker to closed state."""
-        from databricks.sql.telemetry.circuit_breaker_manager import CircuitBreakerManager
         CircuitBreakerManager.reset_circuit_breaker(self._host)
+
+

tests/unit/test_circuit_breaker_http_client.py

@@ -4,7 +4,6 @@
 
 import pytest
 from unittest.mock import Mock, patch, MagicMock
-import urllib.parse
 
 from databricks.sql.telemetry.telemetry_push_client import (
     ITelemetryPushClient,

tests/unit/test_telemetry_circuit_breaker_integration.py

@@ -279,3 +279,5 @@ def make_request():
         assert len(results) + len(errors) == 5
         # Some should be CircuitBreakerError after circuit opens
         assert "CircuitBreakerError" in errors or len(errors) == 0
+
+