Address comments

Author: Kyle-Neale

spark/assets/configuration/spec.yaml

@@ -137,12 +137,16 @@ files:
           example: true
       - name: startup_wait_retries
         description: |
-          How many times to retry when Spark returns a startup message instead of JSON.
-          Set to -1 to disable, 0 to retry forever, or a positive number to limit retries.
+          Controls how to handle "Spark is starting up" responses from the driver.
+          When the Spark driver is not yet ready, it returns a non-JSON startup message
+          instead of the expected JSON response.
+
+          Set to 0 or less to disable this feature and treat startup messages as errors immediately.
+          Set to a positive integer to retry that many times before marking the check as failed.
         value:
           type: integer
-          display_default: 0
-          example: 0
+          display_default: 3
+          example: 3
       - template: instances/http
         overrides:
           auth_token.description: |

spark/changelog.d/22252.added

@@ -1 +1 @@
-Handle Spark driver startup message gracefully instead of raising a JSON parse error.
+Add `startup_wait_retries` option to handle Spark driver startup messages gracefully.

spark/datadog_checks/spark/config_models/defaults.py

@@ -101,7 +101,7 @@ def instance_spark_proxy_enabled():
 
 
 def instance_startup_wait_retries():
-    return 0
+    return 3
 
 
 def instance_streaming_metrics():

spark/datadog_checks/spark/data/conf.yaml.example

@@ -155,11 +155,15 @@ instances:
     #
     # disable_spark_stage_metrics: true
 
-    ## @param startup_wait_retries - integer - optional - default: 0
-    ## How many times to retry when Spark returns a startup message instead of JSON.
-    ## Set to -1 to disable, 0 to retry forever, or a positive number to limit retries.
+    ## @param startup_wait_retries - integer - optional - default: 3
+    ## Controls how to handle "Spark is starting up" responses from the driver.
+    ## When the Spark driver is not yet ready, it returns a non-JSON startup message
+    ## instead of the expected JSON response.
+    ##
+    ## Set to 0 or less to disable this feature and treat startup messages as errors immediately.
+    ## Set to a positive integer to retry that many times before marking the check as failed.
     #
-    # startup_wait_retries: 0
+    # startup_wait_retries: 3
 
     ## @param proxy - mapping - optional
     ## This overrides the `proxy` setting in `init_config`.

spark/datadog_checks/spark/spark.py

@@ -88,9 +88,8 @@ def __init__(self, name, init_config, instances):
 
         # Startup retry configuration:
         # -1: disable (treat startup messages as JSON parse errors immediately)
-        #  0: retry forever (always skip startup messages)
         # >0: retry N times before marking as broken
-        self._startup_wait_retries = int(self.instance.get('startup_wait_retries', 0))
+        self._startup_wait_retries = int(self.instance.get('startup_wait_retries', 3))
         self._startup_retry_count = 0
 
     def check(self, _):
@@ -711,15 +710,7 @@ def _rest_request_to_json(self, address, object_path, service_name, tags, *args,
             response_text = response.text.strip()
             if response_text and 'spark is starting up' in response_text.lower():
                 # Handle startup message based on retry configuration
-                if self._startup_wait_retries == -1:
-                    # Disabled: treat as error immediately
-                    pass
-                elif self._startup_wait_retries == 0:
-                    # Retry forever
-                    self.log.debug("Spark driver not ready yet at %s: %s", self._get_url_base(address), response_text)
-                    return None
-                else:
-                    # Retry N times before marking as broken
+                if self._startup_wait_retries > 0:
                     self._startup_retry_count += 1
                     if self._startup_retry_count <= self._startup_wait_retries:
                         self.log.debug(

spark/tests/test_spark.py

@@ -1188,30 +1188,44 @@ def test_do_not_crash_on_version_collection_failure():
 
 
 @pytest.mark.unit
-def test_driver_startup_message_retry_forever(caplog):
-    """Default behavior (startup_wait_retries=0): retry forever, never raise."""
+def test_driver_startup_message_default_retries(aggregator, caplog):
+    """Default behavior (startup_wait_retries=3): retry 3 times then raise."""
+    from simplejson import JSONDecodeError
+
     check = SparkCheck('spark', {}, [DRIVER_CONFIG])
     response = MockResponse(content="Spark is starting up. Please wait a while until it's ready.")
 
     with caplog.at_level(logging.DEBUG):
         with mock.patch.object(check, '_rest_request', return_value=response):
-            # Call multiple times - should always return None
-            for _ in range(5):
+            # First 3 attempts should return None (default is 3 retries)
+            for i in range(3):
                 result = check._rest_request_to_json(
                     DRIVER_CONFIG['spark_url'], SPARK_REST_PATH, SPARK_DRIVER_SERVICE_CHECK, []
                 )
-                assert result is None
+                assert result is None, f"Attempt {i + 1} should return None"
+
+            # 4th attempt should raise
+            with pytest.raises(JSONDecodeError):
+                check._rest_request_to_json(DRIVER_CONFIG['spark_url'], SPARK_REST_PATH, SPARK_DRIVER_SERVICE_CHECK, [])
 
     assert 'spark driver not ready yet' in caplog.text.lower()
+    assert 'retries exhausted' in caplog.text.lower()
+
+    aggregator.assert_service_check(
+        SPARK_DRIVER_SERVICE_CHECK,
+        status=SparkCheck.CRITICAL,
+        tags=['url:{}'.format(DRIVER_CONFIG['spark_url'])],
+    )
 
 
 @pytest.mark.unit
-def test_driver_startup_message_disabled(aggregator):
-    """When startup_wait_retries=-1, treat startup messages as errors immediately."""
+@pytest.mark.parametrize("retries_value", [0, -1, -5])
+def test_driver_startup_message_disabled(aggregator, retries_value):
+    """When startup_wait_retries<=0, treat startup messages as errors immediately."""
     from simplejson import JSONDecodeError
 
     config = DRIVER_CONFIG.copy()
-    config['startup_wait_retries'] = -1
+    config['startup_wait_retries'] = retries_value
     check = SparkCheck('spark', {}, [config])
     response = MockResponse(content="Spark is starting up. Please wait a while until it's ready.")