[SPARK-21195] Automatically register new metrics from sources and wire default registry

[robert3005]

What changes were proposed in this pull request?

Registers metric listeners on sources metrics that forward actions to metricssystem metric registry. Hooks into default metric registry for easier integration with non spark specific libraries

How was this patch tested?

Added tests

[jiangxb1987]

ok to test

[ash211]

Jenkins this is ok to test

[SparkQA]

Test build #78649 has finished for PR 18406 at commit 1457cdf.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[ash211]

@robert3005 looks like a bunch of tests are failing with
java.lang.IllegalArgumentException: A metric named local-1498509661743.driver.HiveExternalCatalog.fileCacheHits already exists

[jerryshao]

@robert3005 would you please elaborate the usage scenario of your proposal, AFAIK Spark internally doesn't have such requirement.

[SparkQA]

Test build #78696 has finished for PR 18406 at commit 36e7572.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[robert3005]

This is to facilitate using metrics in libraries that integrate in spark. Since spark already has metric reporting infrastructure and lets you register sources with it it seems natural extension to not require preregistering metrics. I am using instrumentation libraries to create metrics and those can register metrics lazily when necessary

[SparkQA]

Test build #78721 has finished for PR 18406 at commit 71c5491.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #78726 has finished for PR 18406 at commit fcd72c1.

• This patch fails due to an unknown error code, -10.
• This patch merges cleanly.
• This patch adds no public classes.

[jerryshao]

@robert3005 based on your description, this feature is more like your own customized requirement, not Spark itself. I'm wondering can it be worked out of Spark?

[robert3005]

I don't see how this can be worked out. Let's say I am parquet and I want to register my metrics since they're part of application execution. Right now I have to statically define all metrics upfront which is a lot of unnecessary boilerplate

[jerryshao]

So the key point is that should metrics system understand dynamic registered metrics, am I understanding right? If the registered metrics are static ones, then I think current code should be enough to work?

[SparkQA]

Test build #78735 has finished for PR 18406 at commit 5108ba2.

• This patch fails PySpark pip packaging tests.
• This patch merges cleanly.
• This patch adds no public classes.

[robert3005]

Yes, the key point is to register dynamic metrics since enumerating all of them can be a lot of hassle and needs to be kept in sync with external libraries

[robert3005]

@jerryshao sorry I missed your comment. Somehow didn't get notification for it

[SparkQA]

Test build #83488 has finished for PR 18406 at commit cee5de7.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #88730 has finished for PR 18406 at commit 21135f2.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Can one of the admins verify this patch?

[github-actions]

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!