You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dongjoon-hyun (via GitHub)" <gi...@apache.org> on 2026/10/01 18:07:24 UTC
[PR] [SPARK-59927] Fix the Kubernetes client metric names in `configuration.md` [spark-kubernetes-operator]
dongjoon-hyun opened a new pull request, #917:
URL: https://github.com/apache/spark-kubernetes-operator/pull/917
### What changes were proposed in this pull request?
This PR fixes the "Kubernetes Client Metrics" table in `docs/configuration.md` so that the metric names and descriptions match what `KubernetesMetricsInterceptor` actually registers (source name `kubernetes.client` + the registry-local name).
- `kubernetes.client.http.response.failed` -> `kubernetes.client.failed`. The description is also corrected: the meter is marked on both a connection failure (no response) and a non-successful response (e.g. `404`).
- `kubernetes.client.http.response.{1,2,3,4,5}xx` -> `kubernetes.client.{1,2,3,4,5}xx`. The misleading "per response code" wording is removed, and the table now notes that these meters exist only when `spark.kubernetes.operator.metrics.clientMetricsGroupByResponseCodeEnabled` is `true` (default).
- The request method, resource name, and namespace name segments are always lower-cased in the registry, so the placeholders are now written in lower case with examples (`get`, `configmaps.get`, `spark-test.sparkapplications.post`).
- The paragraph below the table now also says that a request which gets an error response is counted in `kubernetes.client.failed` and `kubernetes.client.http.response`, so that it is consistent with the table.
### Why are the changes needed?
The documented names do not exist at runtime, so a dashboard or an alert built from the documentation would never match. Also, the paragraph below the table already referred to `kubernetes.client.failed`, which contradicted the table row `kubernetes.client.http.response.failed`.
I verified the actual names by running the existing `KubernetesMetricsInterceptorTest` scenario (create a `SparkApplication`, get a `ConfigMap` that returns `404`) with a temporary probe that printed the registry:
```
http.request = 2
http.response = 2
http.response.201 = 1
http.response.404 = 1
http.request.get = 1
http.request.post = 1
failed = 1 <- from the 404 response
1xx = 0, 2xx = 1, 3xx = 0, 4xx = 1, 5xx = 0
configmaps.get = 1
sparkapplications.post = 1
spark-test.sparkapplications.post = 1
spark-system.configmaps.get = 1
http.response.latency.nanos = Histogram
```
### Does this PR introduce _any_ user-facing change?
Yes, the documentation is fixed. There is no behavior change, and the operator code is untouched.
### How was this patch tested?
Manually. I compared the table against `KubernetesMetricsInterceptor` and the registry output shown above. The temporary probe test was removed afterwards.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Sonnet 5.5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59927] Fix the Kubernetes client metric names in `configuration.md` [spark-kubernetes-operator]
Posted by "ykisana (via GitHub)" <gi...@apache.org>.
ykisana commented on code in PR #917:
URL: https://github.com/apache/spark-kubernetes-operator/pull/917#discussion_r4159009354
##########
docs/configuration.md:
##########
@@ -213,26 +213,27 @@ via [Codahale JVM Metrics](https://javadoc.io/doc/com.codahale.metrics/metrics-j
### Kubernetes Client Metrics
-| Metrics Name | Type | Description |
-|-----------------------------------------------------------|------------|--------------------------------------------------------------------------------------------------------------------------|
-| kubernetes.client.http.request | Meter | Tracking the rates of HTTP request sent to the Kubernetes API Server |
-| kubernetes.client.http.response | Meter | Tracking the rates of HTTP response from the Kubernetes API Server |
-| kubernetes.client.http.response.failed | Meter | Tracking the rates of HTTP requests which have no response from the Kubernetes API Server |
-| kubernetes.client.http.response.latency.nanos | Histograms | Measures the statistical distribution of HTTP response latency from the Kubernetes API Server |
-| kubernetes.client.http.response.`ResponseCode` | Meter | Tracking the rates of HTTP response based on response code from the Kubernetes API Server |
-| kubernetes.client.http.request.`RequestMethod` | Meter | Tracking the rates of HTTP request based type of method to the Kubernetes API Server |
-| kubernetes.client.http.response.1xx | Meter | Tracking the rates of HTTP Code 1xx responses (informational) received from the Kubernetes API Server per response code. |
-| kubernetes.client.http.response.2xx | Meter | Tracking the rates of HTTP Code 2xx responses (success) received from the Kubernetes API Server per response code. |
-| kubernetes.client.http.response.3xx | Meter | Tracking the rates of HTTP Code 3xx responses (redirection) received from the Kubernetes API Server per response code. |
-| kubernetes.client.http.response.4xx | Meter | Tracking the rates of HTTP Code 4xx responses (client error) received from the Kubernetes API Server per response code. |
-| kubernetes.client.http.response.5xx | Meter | Tracking the rates of HTTP Code 5xx responses (server error) received from the Kubernetes API Server per response code. |
-| kubernetes.client.`ResourceName`.`Method` | Meter | Tracking the rates of HTTP request for a combination of one Kubernetes resource and one http method |
-| kubernetes.client.`NamespaceName`.`ResourceName`.`Method` | Meter | Tracking the rates of HTTP request for a combination of one namespace-scoped Kubernetes resource and one http method |
+| Metrics Name | Type | Description |
+|-----------------------------------------------------------|------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| kubernetes.client.http.request | Meter | Tracking the rates of HTTP request sent to the Kubernetes API Server |
+| kubernetes.client.http.response | Meter | Tracking the rates of HTTP response from the Kubernetes API Server |
+| kubernetes.client.failed | Meter | Tracking the rates of HTTP requests which have no response, or an error response, from the Kubernetes API Server |
Review Comment:
`non-2xx response` is more exact here than `error response`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59927] Fix the Kubernetes client metric names in `configuration.md` [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #917:
URL: https://github.com/apache/spark-kubernetes-operator/pull/917#issuecomment-5937897248
Thank you for review and filing [SPARK-59928](https://issues.apache.org/jira/browse/SPARK-59928), @peter-toth . I'll dig more.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59927] Fix the Kubernetes client metric names in `configuration.md` [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub) via reviews" <re...@spark.apache.org>.
dongjoon-hyun commented on PR #917:
URL: https://github.com/apache/spark-kubernetes-operator/pull/917#issuecomment-6563266092
I'll revisit this later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59927] Fix the Kubernetes client metric names in `configuration.md` [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #917:
URL: https://github.com/apache/spark-kubernetes-operator/pull/917#issuecomment-5937529571
Thank you for reporting, @peter-toth .
cc @jiangzho , @TQJADE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59927] Fix the Kubernetes client metric names in `configuration.md` [spark-kubernetes-operator]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on code in PR #917:
URL: https://github.com/apache/spark-kubernetes-operator/pull/917#discussion_r4158980368
##########
docs/configuration.md:
##########
@@ -213,26 +213,27 @@ via [Codahale JVM Metrics](https://javadoc.io/doc/com.codahale.metrics/metrics-j
### Kubernetes Client Metrics
-| Metrics Name | Type | Description |
-|-----------------------------------------------------------|------------|--------------------------------------------------------------------------------------------------------------------------|
-| kubernetes.client.http.request | Meter | Tracking the rates of HTTP request sent to the Kubernetes API Server |
-| kubernetes.client.http.response | Meter | Tracking the rates of HTTP response from the Kubernetes API Server |
-| kubernetes.client.http.response.failed | Meter | Tracking the rates of HTTP requests which have no response from the Kubernetes API Server |
-| kubernetes.client.http.response.latency.nanos | Histograms | Measures the statistical distribution of HTTP response latency from the Kubernetes API Server |
-| kubernetes.client.http.response.`ResponseCode` | Meter | Tracking the rates of HTTP response based on response code from the Kubernetes API Server |
-| kubernetes.client.http.request.`RequestMethod` | Meter | Tracking the rates of HTTP request based type of method to the Kubernetes API Server |
-| kubernetes.client.http.response.1xx | Meter | Tracking the rates of HTTP Code 1xx responses (informational) received from the Kubernetes API Server per response code. |
-| kubernetes.client.http.response.2xx | Meter | Tracking the rates of HTTP Code 2xx responses (success) received from the Kubernetes API Server per response code. |
-| kubernetes.client.http.response.3xx | Meter | Tracking the rates of HTTP Code 3xx responses (redirection) received from the Kubernetes API Server per response code. |
-| kubernetes.client.http.response.4xx | Meter | Tracking the rates of HTTP Code 4xx responses (client error) received from the Kubernetes API Server per response code. |
-| kubernetes.client.http.response.5xx | Meter | Tracking the rates of HTTP Code 5xx responses (server error) received from the Kubernetes API Server per response code. |
-| kubernetes.client.`ResourceName`.`Method` | Meter | Tracking the rates of HTTP request for a combination of one Kubernetes resource and one http method |
-| kubernetes.client.`NamespaceName`.`ResourceName`.`Method` | Meter | Tracking the rates of HTTP request for a combination of one namespace-scoped Kubernetes resource and one http method |
+| Metrics Name | Type | Description |
+|-----------------------------------------------------------|------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| kubernetes.client.http.request | Meter | Tracking the rates of HTTP request sent to the Kubernetes API Server |
+| kubernetes.client.http.response | Meter | Tracking the rates of HTTP response from the Kubernetes API Server |
+| kubernetes.client.failed | Meter | Tracking the rates of HTTP requests which have no response, or an error response, from the Kubernetes API Server |
+| kubernetes.client.http.response.latency.nanos | Histograms | Measures the statistical distribution of HTTP response latency from the Kubernetes API Server |
+| kubernetes.client.http.response.`ResponseCode` | Meter | Tracking the rates of HTTP response based on response code from the Kubernetes API Server |
+| kubernetes.client.http.request.`requestmethod` | Meter | Tracking the rates of HTTP request based type of method (in lower case, e.g. `get`) to the Kubernetes API Server |
+| kubernetes.client.1xx | Meter | Tracking the rates of HTTP Code 1xx responses (informational) received from the Kubernetes API Server. Only available when `spark.kubernetes.operator.metrics.clientMetricsGroupByResponseCodeEnabled` is true (default). |
+| kubernetes.client.2xx | Meter | Tracking the rates of HTTP Code 2xx responses (success) received from the Kubernetes API Server. Only available when `spark.kubernetes.operator.metrics.clientMetricsGroupByResponseCodeEnabled` is true (default). |
+| kubernetes.client.3xx | Meter | Tracking the rates of HTTP Code 3xx responses (redirection) received from the Kubernetes API Server. Only available when `spark.kubernetes.operator.metrics.clientMetricsGroupByResponseCodeEnabled` is true (default). |
+| kubernetes.client.4xx | Meter | Tracking the rates of HTTP Code 4xx responses (client error) received from the Kubernetes API Server. Only available when `spark.kubernetes.operator.metrics.clientMetricsGroupByResponseCodeEnabled` is true (default). |
+| kubernetes.client.5xx | Meter | Tracking the rates of HTTP Code 5xx responses (server error) received from the Kubernetes API Server. Only available when `spark.kubernetes.operator.metrics.clientMetricsGroupByResponseCodeEnabled` is true (default). |
+| kubernetes.client.`resourcename`.`method` | Meter | Tracking the rates of HTTP request for a combination of one Kubernetes resource and one http method (both in lower case, e.g. `configmaps.get`) |
Review Comment:
**Finding 1.** This row only counts a request whose path names a namespace. `updateRequestMetrics` marks it only when `parseNamespaceScopedResource` finds `namespaces` in the path (`KubernetesMetricsInterceptor.java:173-181`). So two kinds of request are left out:
- **A cluster-scoped resource.** In a probe, a GET of `/api/v1/nodes/n-1` created no `nodes.get` meter.
- **A list or watch across all namespaces.** An example is `/apis/spark.apache.org/v1/sparkapplications`, which is what the informers request with `watchedNamespaces=*`.
Both are still counted in `kubernetes.client.http.request` and `kubernetes.client.http.request.get`. The next row already says "namespace-scoped", so without it this one reads as if it covered every request.
```suggestion
| kubernetes.client.`resourcename`.`method` | Meter | Tracking the rates of HTTP request for a combination of one Kubernetes resource and one http method (both in lower case, e.g. `configmaps.get`), only for a request whose path names a namespace |
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59927] Fix the Kubernetes client metric names in `configuration.md` [spark-kubernetes-operator]
Posted by "dongjoon-hyun (via GitHub) via reviews" <re...@spark.apache.org>.
dongjoon-hyun closed pull request #917: [SPARK-59927] Fix the Kubernetes client metric names in `configuration.md`
URL: https://github.com/apache/spark-kubernetes-operator/pull/917
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org