[SPARK-12858] [CORE] Remove duplicated code in metrics

[BenFradet]

I noticed there is some duplicated code in the sinks regarding the poll period.
Also, parts of the metrics.properties template were unclear.
This PR aims to fix both issues.

[BenFradet]

I'll add a test suite for the HasPollingPeriod trait.

[SparkQA]

Test build #49527 has finished for PR 10787 at commit 12f1147.

• This patch fails to build.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • # \"class\" option to its fully qualified class name (see examples below).

[BenFradet]

The reason for the failed build is not reflected in the code change, weird.
Will relaunch a build once I'm done with the test suite.

[SparkQA]

Test build #49529 has finished for PR 10787 at commit 8f6bb1d.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • # \"class\" option to its fully qualified class name (see examples below).

[SparkQA]

Test build #49530 has finished for PR 10787 at commit c6d62d3.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class HasPollingPeriodSuite extends SparkFunSuite with PrivateMethodTester

[SparkQA]

Test build #49553 has finished for PR 10787 at commit bd73bcb.

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

[SparkQA]

Test build #49560 has finished for PR 10787 at commit b43353f.

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

[BenFradet]

Ping @JoshRosen and @rxin

[JoshRosen]

FYI, I think the review of the code changes here is going to take a low priority:

• AFAIK this isn't fixing any urgent bugs.
• Some of these metrics sinks aren't well tested, so doing big refactorings to them carries a lot of risk. Thus, we might need a more careful review to make sure this doesn't break anything.

[BenFradet]

Yup, no problem, thanks for the update.

[BenFradet]

Do you think it'd be worth investigating if tests are feasible/worthwhile?

[JoshRosen]

Suggestion: we can merge the documentation fixes separately. I'm not especially motivated to review these code changes if they're just internal cleanup / refactoring, since this isn't a high-activity component and I'd prefer to just leave known working code in place.

Therefore, here's my proposal:

• Close this PR and open a new one which contains only documentation fixes. This can be merged quickly.
• If there are bugfixes / behavior changes, submit only those changes as separate small and easy to review PRs.
• Don't do the giant refactoring for refactoring's sake; it's not worth the risk + review time.

[BenFradet]

Sure.
I was just curious about how the metrics worked and stumbled upon this.
My thinking was that it'd become clumsy to maintain 5 times the same code in the long run.

Anyway, should I repurpose the jira or create another one?

[JoshRosen]

Create new JIRAs as you submit PRs.