-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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.
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
HDDS-919. Enable prometheus endpoints for Ozone datanodes #502
Conversation
@elek |
@@ -37,6 +37,10 @@ http://maven.apache.org/xsd/maven-4.0.0.xsd"> | |||
<groupId>org.apache.hadoop</groupId> | |||
<artifactId>hadoop-hdds-server-framework</artifactId> | |||
</dependency> | |||
<dependency> | |||
<groupId>org.apache.hadoop</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is duplicated, as above lines 37-38 already have added this dependency.
<property> | ||
<name>hdds.datanode.https-bind-host</name> | ||
<value>0.0.0.0</value> | ||
<tag>OZONE, MANAGEMENT</tag> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for https keys, can we add SECURITY tag also?
Thanks @bharatviswa504 the review.
|
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Thank You @elek for addressing comments. |
Thanks for the warning @bharatviswa504. It's a rebase error they shouldn't be there. Let me rebase the patch and remove the unrelated formatting changes... |
💔 -1 overall
This message was automatically generated. |
Thank You @elek for the update. One minor comment: We dont need the change in hadoop-hdds/container-service/pom.xml. As we have already those dependencies in Line 36-39. You can take care of this during commit.
|
Ups, thanks. I removed it. |
Thank You @elek for the update. |
Hi @elek I think this patch needs some more work, see below error.
|
29f4d60
to
5aff6c9
Compare
Shame on me. I created the .keep file locally but it's ignored by the .gitignore file. I had it locally but it was not pushed. Now it's also fixed. |
…d by Elek, Marton.
+1 LGTM. |
@elek seems some of the test failures are related. Could you please take a look. |
I don't think test failures are not by this patch, not sure if i am missing something here. |
public static final int HDDS_DATANODE_HTTPS_BIND_PORT_DEFAULT = 9883; | ||
public static final String | ||
HDDS_DATANODE_HTTP_KERBEROS_PRINCIPAL_KEY = | ||
"hdds.datanode.http.kerberos.principal"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just reuse the corresponding HDFS key here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arp7 i think idea is to allow our hdds plugin to have separate identity.
@bharatviswa504 you are right, all of the passed tests passed locally except TestFailureHandlingByClient#testBlockWritesWithDnFailures. testBlockWritesWithDnFailures seems to fail even in trunk, we can work on it separate jira. |
…amApp). Author: Shanthoosh Venkataraman <santhoshvenkat1988@gmail.com> Reviewers: Jagadish <jagadish@apache.org> Closes apache#502 from shanthoosh/local_application_runner_set_exception_in_finish
HDDS-846 provides a new metric endpoint which publishes the available Hadoop metrics in prometheus friendly format with a new servlet.
Unfortunately it's enabled only on the scm/om side. It would be great to enable it in the Ozone/HDDS datanodes on the web server of the HDDS Rest endpoint.
See: https://issues.apache.org/jira/browse/HDDS-919