#2525 spark metrics in depends on prometheus

[blcksrx]

Purpose of this PR

Proposed changes:

• <Change 1>
• <Change 2>
• <Change 3>

Change Category

• Bugfix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that could affect existing functionality)
• Documentation update

Rationale

Checklist

• I have conducted a self-review of my own code.
• I have updated documentation accordingly.
• I have added tests that prove my changes are effective or that my feature works.
• Existing unit tests pass locally with my changes.

Additional Notes

#2525

[nabuskey]

A bit confused here. This PR doesn't seem to address the issue linked. The functions modified are called when building prometheus configurations only. This means it doesn't allow for other monitoring solutions to work like mentioned in the issue.

[blcksrx]

I understand the confusion, it just naming and I have avoid to touch/rename more than that to make it simple
Although the function name is configPrometheusMonitoring but it builds the configMap file for metrics.properties whetever your MetricProperties or MetricsPropertiesFile are

https://github.com/kubeflow/spark-operator/blob/master/internal/controller/sparkapplication/monitoring_config.go#L125

[nabuskey]

Hmm as far as I can tell, configPrometheusMonitoring is called if the prometheus field is not nil. So this means you have to put something in the prometheus field. I think writings tests here to show your intentions and results help here.

[blcksrx]

yes Exactly, my intention was to write the test but I saw that all the monitoring_test has been commented out

[blcksrx]

@nabuskey I've rebased my PR with your recent fix and added my test case there

[blcksrx]

@nabuskey I've reflected the comments,
Appreciate the review again

[nabuskey]

/lgtm

[vara-bonthu]

/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vara-bonthu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [vara-bonthu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ChenYi015]

/cherrypick release-2.2

[google-oss-robot]

@ChenYi015: new pull request created: #2543

In response to this:

/cherrypick release-2.2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.