[release-2.2] #2525 spark metrics in depends on prometheus (#2543)

Signed-off-by: Hossein Torabi <[email protected]>
Co-authored-by: Hossein <[email protected]>

Author: google-oss-robot

internal/controller/sparkapplication/monitoring_config.go

@@ -35,7 +35,7 @@ import (
 
 func configPrometheusMonitoring(app *v1beta2.SparkApplication, client client.Client) error {
 	port := common.DefaultPrometheusJavaAgentPort
-	if app.Spec.Monitoring.Prometheus.Port != nil {
+	if app.Spec.Monitoring.Prometheus != nil && app.Spec.Monitoring.Prometheus.Port != nil {
 		port = *app.Spec.Monitoring.Prometheus.Port
 	}
 
@@ -63,12 +63,14 @@ func configPrometheusMonitoring(app *v1beta2.SparkApplication, client client.Cli
 
 	var javaOption string
 
-	javaOption = fmt.Sprintf(
-		"-javaagent:%s=%d:%s/%s",
-		app.Spec.Monitoring.Prometheus.JmxExporterJar,
-		port,
-		common.PrometheusConfigMapMountPath,
-		common.PrometheusConfigKey)
+	if app.Spec.Monitoring.Prometheus != nil {
+		javaOption = fmt.Sprintf(
+			"-javaagent:%s=%d:%s/%s",
+			app.Spec.Monitoring.Prometheus.JmxExporterJar,
+			port,
+			common.PrometheusConfigMapMountPath,
+			common.PrometheusConfigKey)
+	}
 
 	if util.HasPrometheusConfigFile(app) {
 		configFile := *app.Spec.Monitoring.Prometheus.ConfigFile
@@ -133,7 +135,7 @@ func buildPrometheusConfigMap(app *v1beta2.SparkApplication, prometheusConfigMap
 		configMapData[common.MetricsPropertiesKey] = metricsProperties
 	}
 
-	if !util.HasPrometheusConfigFile(app) {
+	if app.Spec.Monitoring.Prometheus != nil && !util.HasPrometheusConfigFile(app) {
 		prometheusConfig := common.DefaultPrometheusConfiguration
 		if app.Spec.Monitoring.Prometheus.Configuration != nil {
 			prometheusConfig = *app.Spec.Monitoring.Prometheus.Configuration

internal/controller/sparkapplication/monitoring_config_test.go

@@ -56,30 +56,30 @@ func TestConfigPrometheusMonitoring(t *testing.T) {
 		err = fakeClient.Get(context.TODO(), client.ObjectKeyFromObject(configMap), configMap)
 		assert.NoError(t, err, "failed to get ConfigMap %s", configMapName)
 
-		if test.app.Spec.Monitoring.Prometheus.ConfigFile == nil &&
+		if test.app.Spec.Monitoring.Prometheus != nil && test.app.Spec.Monitoring.Prometheus.ConfigFile == nil &&
 			test.app.Spec.Monitoring.MetricsPropertiesFile == nil {
 			assert.Len(t, configMap.Data, 2, "expected 2 data items")
 		}
 
-		if test.app.Spec.Monitoring.Prometheus.ConfigFile != nil &&
+		if test.app.Spec.Monitoring.Prometheus != nil && test.app.Spec.Monitoring.Prometheus.ConfigFile != nil &&
 			test.app.Spec.Monitoring.MetricsPropertiesFile == nil {
 			assert.Len(t, configMap.Data, 1, "expected 1 data item")
 		}
 
-		if test.app.Spec.Monitoring.Prometheus.ConfigFile == nil &&
+		if test.app.Spec.Monitoring.Prometheus != nil && test.app.Spec.Monitoring.Prometheus.ConfigFile == nil &&
 			test.app.Spec.Monitoring.MetricsPropertiesFile != nil {
 			assert.Len(t, configMap.Data, 1, "expected 1 data item")
 		}
 
-		if test.app.Spec.Monitoring.MetricsPropertiesFile == nil {
+		if test.app.Spec.Monitoring.Prometheus != nil && test.app.Spec.Monitoring.MetricsPropertiesFile == nil {
 			assert.Equal(t, test.metricsProperties, configMap.Data[common.MetricsPropertiesKey], "metrics.properties mismatch")
 		}
 
-		if test.app.Spec.Monitoring.Prometheus.ConfigFile == nil {
+		if test.app.Spec.Monitoring.Prometheus != nil && test.app.Spec.Monitoring.Prometheus.ConfigFile == nil {
 			assert.Equal(t, test.prometheusConfig, configMap.Data[common.PrometheusConfigKey], "prometheus.yaml mismatch")
 		}
 
-		if test.app.Spec.Monitoring.ExposeDriverMetrics {
+		if test.app.Spec.Monitoring.Prometheus != nil && test.app.Spec.Monitoring.ExposeDriverMetrics {
 			assert.Len(t, test.app.Spec.Driver.Annotations, 3, "expected 3 driver annotations")
 			assert.Equal(t, test.port, test.app.Spec.Driver.Annotations[common.PrometheusPortAnnotation], "java agent port mismatch")
 			assert.Equal(t, test.driverJavaOptions, *test.app.Spec.Driver.JavaOptions, "driver Java options mismatch")
@@ -309,6 +309,32 @@ func TestConfigPrometheusMonitoring(t *testing.T) {
 			driverJavaOptions:   "-javaagent:/prometheus/exporter.jar=1000:/etc/metrics/conf/prometheus.yaml",
 			executorJavaOptions: "-javaagent:/prometheus/exporter.jar=1000:/etc/metrics/conf/prometheus.yaml",
 		},
+		{
+			app: &v1beta2.SparkApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "app2",
+					Namespace: "default",
+				},
+				Spec: v1beta2.SparkApplicationSpec{
+					Driver: v1beta2.DriverSpec{
+						JavaOptions: util.StringPtr("-XX:+PrintGCDetails -XX:+PrintGCTimeStamps"),
+					},
+					Executor: v1beta2.ExecutorSpec{
+						JavaOptions: util.StringPtr("-XX:+PrintGCDetails -XX:+PrintGCTimeStamps"),
+					},
+					Monitoring: &v1beta2.MonitoringSpec{
+						ExposeDriverMetrics:   false,
+						ExposeExecutorMetrics: false,
+						MetricsProperties:     util.StringPtr("testcase2dummy"),
+					},
+				},
+			},
+			metricsProperties:   "testcase2dummy",
+			prometheusConfig:    "",
+			port:                "8090",
+			driverJavaOptions:   "-XX:+PrintGCDetails -XX:+PrintGCTimeStamps ",
+			executorJavaOptions: "-XX:+PrintGCDetails -XX:+PrintGCTimeStamps ",
+		},
 	}
 
 	for _, test := range testcases {

pkg/util/sparkapplication.go

@@ -252,15 +252,13 @@ func HasPrometheusConfigFile(app *v1beta2.SparkApplication) bool {
 
 // HasPrometheusConfig returns if Prometheus monitoring defines metricsProperties in the spec.
 func HasMetricsProperties(app *v1beta2.SparkApplication) bool {
-	return PrometheusMonitoringEnabled(app) &&
-		app.Spec.Monitoring.MetricsProperties != nil &&
+	return app.Spec.Monitoring.MetricsProperties != nil &&
 		*app.Spec.Monitoring.MetricsProperties != ""
 }
 
 // HasPrometheusConfigFile returns if Monitoring defines metricsPropertiesFile in the spec.
 func HasMetricsPropertiesFile(app *v1beta2.SparkApplication) bool {
-	return PrometheusMonitoringEnabled(app) &&
-		app.Spec.Monitoring.MetricsPropertiesFile != nil &&
+	return app.Spec.Monitoring.MetricsPropertiesFile != nil &&
 		*app.Spec.Monitoring.MetricsPropertiesFile != ""
 }