Skip to content

HDDS-7648. Add a servername tag in UGI metrics. #4094

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

Merged
adoroszlai merged 6 commits into from
Jan 3, 2023
Merged

HDDS-7648. Add a servername tag in UGI metrics. #4094

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2c18e38
Add servername tag to Prometheus metrics
mladjan-gadzic Dec 15, 2022
8e4f3b7
Refactor for readability and move utils to util class
mladjan-gadzic Dec 15, 2022
ada071a
Remove dependecy from hdds to ozone and refactor to increase readability
mladjan-gadzic Dec 19, 2022
2fde42b
Refactor class name to be inline with naming conventions and add javadoc
mladjan-gadzic Dec 20, 2022
e7ce28d
Merge branch 'master' into HDDS-7648
mladjan-gadzic Dec 22, 2022
c368c45
Remove uneccessary test and set servername in constructor
mladjan-gadzic Dec 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .../org/apache/hadoop/ozone/container/common/volume/TestVolumeIOStatsWithPrometheusSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ private String publishMetricsAndGetOutput() throws IOException {
ByteArrayOutputStream stream = new ByteArrayOutputStream();
OutputStreamWriter writer = new OutputStreamWriter(stream, UTF_8);

sink.writeMetrics(writer);
final int serverPort = 0;
sink.writeMetrics(writer, serverPort);
writer.flush();

return stream.toString(UTF_8.name());
Expand Down
9 changes: 9 additions & 0 deletions hadoop-hdds/framework/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,16 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<name>Apache Ozone HDDS Server Framework</name>
<packaging>jar</packaging>

<properties>
<ozone-common.version>1.3.0-SNAPSHOT</ozone-common.version>
</properties>

<dependencies>
<dependency>
<groupId>org.apache.ozone</groupId>
<artifactId>ozone-common</artifactId>
<version>${ozone-common.version}</version>
</dependency>

@adoroszlai adoroszlai Dec 15, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please do not introduce backwards dependency (hdds -> ozone). If we want to access some code currently in ozone from both, we should move it to hdds.

@mladjan-gadzic mladjan-gadzic Dec 16, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is a need for org.apache.hadoop.ozone.om.OMConfigKeys. Where exactly do you propose it should be moved to make the most sense out of this?

@mladjan-gadzic mladjan-gadzic Dec 16, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

org.apache.hadoop.ozone.om package (om part yet to be created but at same level where org.apache.hadoop.hdds.scm.ScmConfigKeys resides) from common module from hadoop-hdds (hdds-common artifact id) sounds reasonable enough, but there are quite a few moving parts together with org.apache.hadoop.ozone.om.OMConfigKeys.

@xBis7 xBis7 Dec 16, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@adoroszlai Thanks for looking into this. I don't think there is a package where we can access both OMConfigKeys and ScmConfigKeys but we might be able to simplify the switch case that requires both of them. We present prometheus metrics only for SCM and OM. We can only check the ScmConfigKeys and if it doesn't match then we can assume the server is OM without checking OMConfigKeys.

This might not be the best approach. We have the hostname available. There might be a way to use that to get the server.

@mladjan-gadzic mladjan-gadzic Dec 16, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@adoroszlai @xBis7 Thanks for the suggestions! Check for ScmConfigKeys is enough right now - it might not be the best future proof solution but it is still better than moving tons of classes just in order to use OMConfigKeys.

@adoroszlai adoroszlai Dec 16, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think there's a simple, more generic solution: PrometheusMetricsSink is created in BaseHttpServer, which has service-specific subclasses. So define a method in BaseHttpServer that returns the info you want to pass to PrometheusMetricsSink, then override in OzoneManagerHttpServer, etc.

@mladjan-gadzic mladjan-gadzic Dec 16, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@adoroszlai This might be exactly what I was looking for. Thanks for suggestion, will try to expand on it!

<dependency>
<groupId>org.apache.ozone</groupId>
<artifactId>hdds-interface-server</artifactId>
Expand Down
82 changes: 25 additions & 57 deletions ...dds/framework/src/main/java/org/apache/hadoop/hdds/server/http/PrometheusMetricsSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,15 @@
*/
package org.apache.hadoop.hdds.server.http;

import static org.apache.hadoop.hdds.utils.RocksDBStoreMBean.ROCKSDB_CONTEXT_PREFIX;

import java.io.IOException;
import java.io.Writer;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.List;
import java.util.regex.Pattern;
import org.apache.commons.configuration2.SubsetConfiguration;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.hdds.utils.DecayRpcSchedulerUtil;
import org.apache.hadoop.hdds.utils.PrometheusMetricsSinkUtil;
import org.apache.hadoop.metrics2.AbstractMetric;
import org.apache.hadoop.metrics2.MetricType;
import org.apache.hadoop.metrics2.MetricsRecord;
Expand All @@ -48,12 +45,6 @@ public class PrometheusMetricsSink implements MetricsSink {
private final Map<String, Map<String, String>> metricLines =
Collections.synchronizedSortedMap(new TreeMap<>());

private static final Pattern SPLIT_PATTERN =
Pattern.compile("(?<!(^|[A-Z_]))(?=[A-Z])|(?<!^)(?=[A-Z][a-z])");

private static final Pattern REPLACE_PATTERN =
Pattern.compile("[^a-zA-Z0-9]+");

public PrometheusMetricsSink() {
}

Expand All @@ -69,7 +60,7 @@ public void putMetrics(MetricsRecord metricsRecord) {
String username = DecayRpcSchedulerUtil
.checkMetricNameForUsername(metricsRecord.name(), metrics.name());

String key = prometheusName(
String key = PrometheusMetricsSinkUtil.prometheusName(
metricsRecord.name(), metricName);

String prometheusMetricKeyAsString =
Expand All @@ -94,56 +85,30 @@ private String getPrometheusMetricKeyAsString(MetricsRecord metricsRecord,
.append("{");
String sep = "";

// tagListWithUsernameIfNeeded() checks if username is null.
// If it's not then it returns a list with the existing
// metric tags and a username tag.
List<MetricsTag> metricTagList = DecayRpcSchedulerUtil
.tagListWithUsernameIfNeeded(metricsRecord, username);
List<MetricsTag> metricTags =
PrometheusMetricsSinkUtil.addTags(key, username, metricsRecord.tags());

//add tags
for (MetricsTag tag : metricTagList) {
for (MetricsTag tag : metricTags) {
String tagName = tag.name().toLowerCase();

//ignore specific tag which includes sub-hierarchy
if (!tagName.equals("numopenconnectionsperuser")) {
prometheusMetricKey.append(sep)
.append(tagName)
.append("=\"")
.append(tag.value())
.append("\"");
sep = ",";
if (tagName.equals("numopenconnectionsperuser")) {
continue;
}

prometheusMetricKey.append(sep)
.append(tagName)
.append("=\"")
.append(tag.value())
.append("\"");
sep = ",";
}
prometheusMetricKey.append("}");

return prometheusMetricKey.toString();
}

/**
* Convert CamelCase based names to lower-case names where the separator
* is the underscore, to follow prometheus naming conventions.
*/
public String prometheusName(String recordName,
String metricName) {

// RocksDB metric names already have underscores as delimiters,
// but record name is from DB file name and '.' (as in 'om.db') is invalid
if (StringUtils.isNotEmpty(recordName) &&
recordName.startsWith(ROCKSDB_CONTEXT_PREFIX)) {
return normalizeName(recordName) + "_" + metricName.toLowerCase();
}

String baseName = StringUtils.capitalize(recordName)
+ StringUtils.capitalize(metricName);
return normalizeName(baseName);
}

public static String normalizeName(String baseName) {
String[] parts = SPLIT_PATTERN.split(baseName);
String result = String.join("_", parts).toLowerCase();
return REPLACE_PATTERN.matcher(result).replaceAll("_");
}

@Override
public void flush() {

Expand All @@ -154,14 +119,17 @@ public void init(SubsetConfiguration subsetConfiguration) {

}

public void writeMetrics(Writer writer) throws IOException {
for (Map.Entry<String, Map<String, String>> metricsEntry
public void writeMetrics(Writer writer, int serverPort) throws IOException {
for (Map.Entry<String, Map<String, String>> metricEntry
: metricLines.entrySet()) {
writer.write(metricsEntry.getKey() + "\n");

for (Map.Entry<String, String> metrics
: metricsEntry.getValue().entrySet()) {
writer.write(metrics.getKey() + " " + metrics.getValue() + "\n");
writer.write(metricEntry.getKey() + "\n");

for (Map.Entry<String, String> metric
: metricEntry.getValue().entrySet()) {
String metricKey =
PrometheusMetricsSinkUtil.replaceServerNameTagValue(
metricEntry.getKey(), metric.getKey(), serverPort);
writer.write(metricKey + " " + metric.getValue() + "\n");
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...op-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/PrometheusServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
}
DefaultMetricsSystem.instance().publishMetricsNow();
PrintWriter writer = resp.getWriter();
getPrometheusSink().writeMetrics(writer);
getPrometheusSink().writeMetrics(writer, req.getServerPort());
writer.write("\n\n#Dropwizard metrics\n\n");
//print out dropwizard metrics used by ratis.
TextFormat.write004(writer,
Expand Down
36 changes: 0 additions & 36 deletions hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/DecayRpcSchedulerUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@

package org.apache.hadoop.hdds.utils;

import com.google.common.base.Strings;
import org.apache.hadoop.metrics2.MetricsInfo;
import org.apache.hadoop.metrics2.MetricsRecord;
import org.apache.hadoop.metrics2.MetricsTag;

import java.util.ArrayList;
import java.util.List;

/**
* Helper functions for DecayRpcScheduler
* metrics for Prometheus.
Expand All @@ -34,18 +26,6 @@ public final class DecayRpcSchedulerUtil {
private DecayRpcSchedulerUtil() {
}

private static final MetricsInfo USERNAME_INFO = new MetricsInfo() {
@Override
public String name() {
return "username";
}

@Override
public String description() {
return "caller username";
}
};

/**
* For Decay_Rpc_Scheduler, the metric name is in format
* "Caller(<callers_username>).Volume"
Expand Down Expand Up @@ -102,20 +82,4 @@ public static String checkMetricNameForUsername(String recordName,
return null;
}

/**
* MetricRecord.tags() is an unmodifiable collection of tags.
* Store it in a list, to modify it and add a username tag.
* @param metricsRecord
* @return the new list with the metric tags and the username tag
*/
public static List<MetricsTag> tagListWithUsernameIfNeeded(
MetricsRecord metricsRecord, String username) {
List<MetricsTag> list = new ArrayList<>(metricsRecord.tags());

if (!Strings.isNullOrEmpty(username)) {
MetricsTag tag = new MetricsTag(USERNAME_INFO, username);
list.add(tag);
}
return list;
}
}
Loading