Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Hadoop metrics receiver #351

Merged
merged 33 commits into from
Feb 15, 2022
Merged

Add Hadoop metrics receiver #351

merged 33 commits into from
Feb 15, 2022

Conversation

Mrod1598
Copy link

Add hadoop metrics receiver

@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Jan 24, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Jan 24, 2022
@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Jan 25, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Jan 25, 2022
@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Jan 25, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Jan 25, 2022
@Mrod1598 Mrod1598 marked this pull request as ready for review January 25, 2022 03:44
@Mrod1598 Mrod1598 requested a review from qingling128 January 25, 2022 03:44
@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 1, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 1, 2022
qingling128
qingling128 reviewed Feb 2, 2022
View reviewed changes
apps/hadoop.go Outdated Show resolved Hide resolved
@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 2, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 2, 2022
@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 2, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 2, 2022
@BinaryFissionGames BinaryFissionGames mentioned this pull request Feb 2, 2022
Merged
qingling128
qingling128 approved these changes Feb 8, 2022
View reviewed changes
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

apps/hadoop.go Outdated Show resolved Hide resolved
@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 8, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 8, 2022
qingling128
qingling128 approved these changes Feb 8, 2022
View reviewed changes
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Waiting for the integration tests before merging

@qingling128
Copy link
Contributor

third party apps integration test (Debian) is failing for debian 10 on tomcat, which is a known issue that is being fixed by a separate PR #344.

third party apps integration test (CentOS) passed in few other PRs but failed for this one. However the error seems to be a service account key issue. Might not be related to this PR.

@qingling128 qingling128 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 8, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 8, 2022
qingling128
qingling128 requested changes Feb 8, 2022
View reviewed changes
docs/hadoop.md Outdated
# `hadoop` Metrics Receiver

The `hadoop` metrics receiver can fetch stats from a Hadoop server's Java Virtual Machine (JVM) via [JMX](https://www.oracle.com/java/technologies/javase/javamanagement.html).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that Hadoop has some pending discussions going on in the Category 3 - Regional metrics and master metrics section of the Duplicate / Overlapping Metrics Issue from Clustered Applications doc.

We'll need to resolve that conversion before releasing this.

Copy link
Contributor

@qingling128 qingling128 Feb 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the recent decision in the Duplicate / Overlapping Metrics Issue from Clustered Applications, let's add some context here:

It collects metrics specific to the local region server, as well as metrics presented by the Master node if the node being monitored is indeed the Master. For High Availability configurations, it is recommended for every master node to report cluster metrics, which will have identical values, to avoid single point of failures when one master goes down.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 8, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 8, 2022
@Mrod1598 Mrod1598 added the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 8, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Forces kokoro to run integration tests on a CL label Feb 8, 2022
@Mrod1598 Mrod1598 requested a review from qingling128 February 14, 2022 18:05
@Mrod1598
Copy link
Author

third party apps integration test (Debian) is failing for debian 10 on tomcat, which is a known issue that is being fixed by a separate PR #344.

third party apps integration test (CentOS) passed in few other PRs but failed for this one. However the error seems to be a service account key issue. Might not be related to this PR.

It did seem to be a one off issue.
image

qingling128
qingling128 approved these changes Feb 14, 2022
View reviewed changes
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo 2 minor comments.

apps/hadoop.go Outdated Show resolved Hide resolved
docs/hadoop.md Outdated
# `hadoop` Metrics Receiver

The `hadoop` metrics receiver can fetch stats from a Hadoop server's Java Virtual Machine (JVM) via [JMX](https://www.oracle.com/java/technologies/javase/javamanagement.html).

Copy link
Contributor

@qingling128 qingling128 Feb 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the recent decision in the Duplicate / Overlapping Metrics Issue from Clustered Applications, let's add some context here:

It collects metrics specific to the local region server, as well as metrics presented by the Master node if the node being monitored is indeed the Master. For High Availability configurations, it is recommended for every master node to report cluster metrics, which will have identical values, to avoid single point of failures when one master goes down.

@Mrod1598 Mrod1598 requested a review from qingling128 February 14, 2022 22:56
qingling128
qingling128 approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@qingling128 qingling128 merged commit 0d5c6df into GoogleCloudPlatform:master Feb 15, 2022
@JonathanWamsley JonathanWamsley mentioned this pull request Feb 16, 2022
Merged
@Mrod1598 Mrod1598 deleted the hadoop-metrics branch March 14, 2022 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martijnvans martijnvans martijnvans left review comments

@quentinmit quentinmit quentinmit left review comments

@qingling128 qingling128 qingling128 approved these changes

Assignees
No one assigned
Labels
3rd-party-app
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Mrod1598 @qingling128 @quentinmit @martijnvans @kokoro-team