Circuit breaker changes using pybreaker #705

nikhilsuri-db · 2025-11-04T00:59:51Z

What type of PR is this?

Refactor
Feature
Bug Fix
Other

Description

This PR introduces a circuit breaker pattern to the telemetry system to prevent cascading failures and improve system resilience. The implementation uses the pybreaker library to monitor telemetry request failures and automatically open the circuit when failure rates exceed configurable thresholds, blocking further requests to protect downstream services. When the circuit is open, telemetry requests are temporarily blocked until the system recovers, at which point the circuit transitions to half-open for testing and eventually closes when normal operation resumes. The circuit breaker configuration is centralized with immutable settings, includes comprehensive logging for monitoring, and maintains full backward compatibility with existing telemetry functionality.

How is this tested?

Unit tests
E2E Tests
Manually
N/A

Related Tickets & Documents

https://docs.google.com/document/d/1ftRvby9bwDZzE3s1tOb4hJ4Pd9USiXskb9cDw-uQNPM/edit?usp=sharing

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

github-actions · 2025-11-04T01:00:01Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

github-actions · 2025-11-04T01:04:42Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

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

nikhilsuri-db · 2025-11-06T13:48:36Z

src/databricks/sql/telemetry/circuit_breaker_manager.py

+)
+
+
+class CircuitBreakerStateListener(CircuitBreakerListener):


Only used for logging purposed for now

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

github-actions · 2025-11-11T19:22:06Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

nikhilsuri-db · 2025-11-07T17:57:08Z

pyproject.toml

    { version = ">=18.0.0", python = ">=3.13", optional=true }
 ]
 pyjwt = "^2.0.0"
+pybreaker = "^1.0.0"


@vikrantpuppala Can we be sure that adding new library won't break any other client usage?

nikhilsuri-db · 2025-11-07T17:58:52Z

src/databricks/sql/auth/common.py

        pool_connections: Optional[int] = None,
        pool_maxsize: Optional[int] = None,
        user_agent: Optional[str] = None,
+        telemetry_circuit_breaker_enabled: Optional[bool] = None,


Will make it true in next PR post end to end testing

nikhilsuri-db · 2025-11-07T17:59:32Z

src/databricks/sql/common/unified_http_client.py

            logger.error("HTTP request failed after retries: %s", e)
-            raise RequestError(f"HTTP request failed: {e}")
+
+            # Try to extract HTTP status code from the MaxRetryError


setting http_code here which will be used by CircuitBreaker to consider regression in telemetry endpoint

nikhilsuri-db · 2025-11-07T18:00:33Z

src/databricks/sql/telemetry/circuit_breaker_manager.py

+
+
+@dataclass(frozen=True)
+class CircuitBreakerConfig:


Single object to store CB config

nikhilsuri-db · 2025-11-07T18:01:05Z

src/databricks/sql/telemetry/circuit_breaker_manager.py

+        return breaker
+
+    @classmethod
+    def _create_noop_circuit_breaker(cls) -> CircuitBreaker:


In cases where client config setup failed or used by test cases.

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

samikshya-db · 2025-11-13T19:40:41Z

src/databricks/sql/telemetry/telemetry_push_client.py

+
+        This allows telemetry to fail silently without raising exceptions.
+        """
+        from unittest.mock import Mock


umm, using test imports in production code is intended here?

samikshya-db · 2025-11-13T20:30:33Z

tests/unit/test_circuit_breaker_http_client.py

+
+        # Clear any existing state
+        CircuitBreakerManager._instances.clear()
+        CircuitBreakerManager.initialize(CircuitBreakerConfig())


Do we need to add a similar initialization in main function too?

samikshya-db · 2025-11-13T20:40:27Z

src/databricks/sql/auth/common.py

        self.pool_connections = pool_connections or 10
        self.pool_maxsize = pool_maxsize or 20
        self.user_agent = user_agent
+        self.telemetry_circuit_breaker_enabled = bool(telemetry_circuit_breaker_enabled)


should we not add this to build_client_context too?

samikshya-db · 2025-11-13T20:53:03Z

src/databricks/sql/telemetry/circuit_breaker_manager.py

+        """
+        config = cls._config
+        if config is None:
+            raise RuntimeError("CircuitBreakerManager not initialized")


we shouldn't be throwing errors at all, I suppose?

samikshya-db · 2025-11-13T21:00:01Z

src/databricks/sql/common/unified_http_client.py

+                # The reason may contain a response object with status
+                http_code = getattr(e.reason.response, "status", None)
+            elif (
+                hasattr(e, "response")


nit : these ~30 lines feels like it would live more happily in a helper

samikshya-db · 2025-11-13T21:01:59Z

src/databricks/sql/common/unified_http_client.py

+            # Try to extract HTTP status code from the MaxRetryError
+            http_code = None
+            if (
+                hasattr(e, "reason")


great to see the defensive approach with urlLib object structure ! we have had several compatibility issue with this library before (I also remember a sev1 once), also can you double check if this is the expected structure once?

(I meant across versions. see internal doc for more context on the type of issues we get)

samikshya-db · 2025-11-13T21:13:02Z

src/databricks/sql/telemetry/circuit_breaker_manager.py

+        Returns:
+            CircuitBreaker instance for the host
+        """
+        if not cls._config:


Can we have this inside lock too?

nikhilsuri-db added 4 commits October 29, 2025 11:56

Added driver connection params

37ec282

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

Added model fields for chunk/result latency

2504053

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

fixed linting issues

ef41f4c

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

lint issue fixing

2f54be8

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

nikhilsuri-db self-assigned this Nov 4, 2025

nikhilsuri-db had a problem deploying to azure-prod November 4, 2025 00:59 — with GitHub Actions Failure

nikhilsuri-db requested a review from samikshya-db November 4, 2025 01:01

nikhilsuri-db temporarily deployed to azure-prod November 4, 2025 01:04 — with GitHub Actions Inactive

nikhilsuri-db had a problem deploying to azure-prod November 4, 2025 01:04 — with GitHub Actions Failure

nikhilsuri-db temporarily deployed to azure-prod November 4, 2025 05:29 — with GitHub Actions Inactive

nikhilsuri-db added 10 commits November 5, 2025 10:18

circuit breaker changes using pybreaker

db93974

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

Added interface layer top of http client to use circuit rbeaker

1f9c4d3

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

Added test cases to validate ciruit breaker

939b548

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

fixing broken tests

6c72f86

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

fixed linting issues

ac845a5

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

fixed failing test cases

a602c39

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

fixed urllib3 issue

c1b6e25

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

added more test cases for telemetry

e3d85f4

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

simplified CB config

9dfb623

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

poetry lock

e7e8b4b

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

nikhilsuri-db force-pushed the PECOBLR-993 branch from c83cba7 to e7e8b4b Compare November 5, 2025 18:20

nikhilsuri-db temporarily deployed to azure-prod November 5, 2025 18:20 — with GitHub Actions Inactive

fix minor issues & improvement

dab4b38

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

nikhilsuri-db had a problem deploying to azure-prod November 5, 2025 18:54 — with GitHub Actions Failure

nikhilsuri-db temporarily deployed to azure-prod November 5, 2025 18:54 — with GitHub Actions Inactive

nikhilsuri-db commented Nov 6, 2025

View reviewed changes

nikhilsuri-db had a problem deploying to azure-prod November 7, 2025 17:50 — with GitHub Actions Failure

linting issue fixed

b527e7c

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

nikhilsuri-db had a problem deploying to azure-prod November 7, 2025 17:55 — with GitHub Actions Failure

nikhilsuri-db had a problem deploying to azure-prod November 11, 2025 19:22 — with GitHub Actions Failure

nikhilsuri-db commented Nov 11, 2025

View reviewed changes

raise CB only for 429/503

2b45814

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

nikhilsuri-db force-pushed the PECOBLR-993 branch from 92eee84 to 2b45814 Compare November 11, 2025 19:27

nikhilsuri-db had a problem deploying to azure-prod November 11, 2025 19:27 — with GitHub Actions Failure

fix broken test cases

1193af7

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

nikhilsuri-db had a problem deploying to azure-prod November 11, 2025 19:35 — with GitHub Actions Failure

fixed untyped references

aa459e9

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

nikhilsuri-db had a problem deploying to azure-prod November 12, 2025 06:00 — with GitHub Actions Failure

Merge remote-tracking branch 'origin/main' into PECOBLR-993

4cb87b1

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

nikhilsuri-db had a problem deploying to azure-prod November 12, 2025 08:03 — with GitHub Actions Failure

nikhilsuri-db had a problem deploying to azure-prod November 13, 2025 11:06 — with GitHub Actions Failure

added more test to verify the changes

7cbc4c8

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

nikhilsuri-db had a problem deploying to azure-prod November 13, 2025 15:57 — with GitHub Actions Failure

description changed

c646335

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

nikhilsuri-db temporarily deployed to azure-prod November 13, 2025 20:37 — with GitHub Actions Inactive

samikshya-db reviewed Nov 13, 2025

View reviewed changes

		)


		class CircuitBreakerStateListener(CircuitBreakerListener):



		@dataclass(frozen=True)
		class CircuitBreakerConfig:

Circuit breaker changes using pybreaker #705

Are you sure you want to change the base?

Circuit breaker changes using pybreaker #705

Uh oh!

Conversation

nikhilsuri-db commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

Description

How is this tested?

Related Tickets & Documents

Uh oh!

github-actions bot commented Nov 4, 2025

Uh oh!

github-actions bot commented Nov 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikhilsuri-db Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nikhilsuri-db commented Nov 4, 2025 •

edited

Loading

nikhilsuri-db Nov 7, 2025 •

edited

Loading