[SPARK-29032][CORE] Simplify Prometheus support by adding PrometheusServlet/Resource#25741
Closed
dongjoon-hyun wants to merge 7 commits intoapache:masterfrom
dongjoon-hyun:SPARK-29032
Closed
[SPARK-29032][CORE] Simplify Prometheus support by adding PrometheusServlet/Resource#25741dongjoon-hyun wants to merge 7 commits intoapache:masterfrom dongjoon-hyun:SPARK-29032
dongjoon-hyun wants to merge 7 commits intoapache:masterfrom
dongjoon-hyun:SPARK-29032
Conversation
Member
Author
Member
Author
|
Oops. I missed the task metrics. I'll update this PR later. |
This comment has been minimized.
This comment has been minimized.
srowen
reviewed
Sep 10, 2019
Member
srowen
left a comment
There was a problem hiding this comment.
What is prometheus BTW? is it worth adding special code for?
| classOf[Properties], classOf[MetricRegistry], classOf[SecurityManager]) | ||
| .newInstance(kv._2, registry, securityMgr) | ||
| metricsServlet = Some(servlet) | ||
| } else if (kv._1 == "prometheusServlet") { |
Member
There was a problem hiding this comment.
If you want you can make kv into case (key, value) here for clarity, but not necessary
core/src/main/scala/org/apache/spark/metrics/sink/PrometheusServlet.scala
Outdated
Show resolved
Hide resolved
Member
Author
|
Thank you for review, @srowen . Prometheus.io is a CNCF project which is used widely with K8s. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Test build #110491 has finished for PR 25741 at commit
|
Member
Author
|
Although this is not a long PR, but I decided to split this into two PRs because |
Member
Author
|
Sorry for this change~ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Prometheus.io is a CNCF project used widely with K8s.
This PR aims to simplify
Prometheussupport by addingPrometheusServletandPrometheusResource. The main use cases areK8sandSpark Standalonecluster environments.Note that this PR focus Spark-generated metrics. Also, we need to update the document later in a separate PR.
Why are the changes needed?
There exists a few ways to support
Prometheus. However, they requires extra configurations and new resources (port numbers to pullorgateways to push). And, the endpoints are widely spreader overMaster/Slave/Driver/CoarseGrainedExecutorBackend. This PR aims to export natively Spark's metrics which starts with metrics_ prefix from the existing (1) method atMaster/Worker/Driver. For task metrics, instead of usingCoarseGrainedExecutorBackend, we will reuse the collected information used for the Apache Spark REST API,api/v1.Does this PR introduce any user-facing change?
Yes. New web interfaces are added along with the existing JSON API.
How was this patch tested?
Manually connect the new end-points with
curl.Or, run
prometheus --config.file=config.yamlwith the following configuration and see through the Prometheus UI.config.yaml