dongjoon-hyun commented on code in PR #794: URL: https://github.com/apache/spark-kubernetes-operator/pull/794#discussion_r4055526804
########## examples/cluster-with-jmx-exporter.yaml: ########## @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +apiVersion: v1 +kind: ConfigMap +metadata: + name: jmx-exporter-config +data: + jmx-exporter-config.yaml: | + includeObjectNames: ["metrics:type=gauges,*"] Review Comment: Minor: the example asks the user to bake their own `jmx_prometheus_javaagent` jar but never names a version, and `includeObjectNames` is the newer key name. Older agent releases used `whitelistObjectNames` and reject unrecognized config keys outright. If someone picks an old jar, the agent fails to load this config, `-javaagent` initialization fails and the worker JVM aborts, so the pods CrashLoopBackOff while the `SparkCluster` still reports `RunningHealthy` (the operator sets that as soon as the applies succeed). A minimum version next to the jar path comment would close this, e.g. "requires jmx_prometheus_javaagent 1.x; the `includeObjectNames` key is not accepted by older releases". I confirmed the key and the pattern work, but not which release introduced `includeObjectNames`, so please use whatever floor you are confident in. ########## docs/operations.md: ########## @@ -205,6 +205,58 @@ It is still honored: the NetworkPolicy is created when either key is `true`, so `enable: true` in a base values file wins over `enabled: false` and must be removed to turn the feature off. The same rule applies to `operatorConfiguration.dynamicConfig.enable`. +## Exposing SparkCluster Worker Metrics + +Every `SparkCluster` gets a generated worker `NetworkPolicy` that only admits ingress from pods +carrying the cluster label or the driver-role label, so a Prometheus scraper is locked out by +default. Opening the worker web UI port (`8081` by default) is not a safe fix: Spark's built-in +`PrometheusServlet` metrics endpoint is served by the same embedded HTTP server as the web UI, so +admitting that port to any source would expose the whole UI, not just metrics. + +Use the [Prometheus JMX Exporter](https://github.com/prometheus/jmx_exporter) +(`jmx_prometheus_javaagent`) to serve metrics on a dedicated HTTP port. See +[examples/cluster-with-jmx-exporter.yaml](../examples/cluster-with-jmx-exporter.yaml) for the full +`ConfigMap` and `SparkCluster` configuration. Replace the example's image with a custom Spark image +containing the exporter jar at `/opt/jmx_exporter/jmx_prometheus_javaagent.jar`; the stock +`apache/spark` image does not include it. + +The example mounts the exporter rules and attaches the agent through `SPARK_DAEMON_JAVA_OPTS`. +The operator sets `SPARK_WORKER_OPTS` itself, so a value there would be overwritten. It also sets Review Comment: This describes the wrong failure mode. The operator does not replace a user-provided `SPARK_WORKER_OPTS`; it appends a second env entry with the same name (`addNewEnv()` in `SparkClusterResourceSpec.java:340`), and `ClusterInitStep` applies the StatefulSet with `serverSideApply()`, which rejects duplicate keys in the `env` associative list. Checked against a live API server: ``` $ kubectl apply --server-side --dry-run=server -f dup-env.yaml Error from server: failed to create typed patch object (default/dup-env-probe; apps/v1, Kind=StatefulSet): .spec.template.spec.containers[name="worker"].env: duplicate entries for key [name="SPARK_WORKER_OPTS"] ``` So a reader who takes "would be overwritten" at face value and sets it anyway does not get a silently ignored value. The whole worker StatefulSet apply is rejected, the cluster goes to `SchedulingFailure` and then `Failed`, and the status message is a stack trace that never mentions the duplicate variable. The advice to use `SPARK_DAEMON_JAVA_OPTS` is right, only the reason is wrong. Something like: "`SPARK_WORKER_OPTS` must not be set there: the operator appends its own, and server-side apply rejects duplicate env names." -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
