Skip to content

[metric] Add cluster_id tag for client metric group #1655

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[metric] Add cluster_id tag for client metric group #1655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions fluss-rpc/src/main/java/org/apache/fluss/rpc/metrics/ClientMetricGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import org.apache.fluss.metrics.groups.AbstractMetricGroup;
import org.apache.fluss.metrics.registry.MetricRegistry;

import javax.annotation.Nullable;

import java.util.Map;

/** The metric group for clients. */
Expand All @@ -29,10 +31,16 @@ public class ClientMetricGroup extends AbstractMetricGroup {
private static final String NAME = "client";

private final String clientId;
private final @Nullable String clusterId;

public ClientMetricGroup(MetricRegistry registry, String clientId) {
this(registry, null, clientId);
}

public ClientMetricGroup(MetricRegistry registry, @Nullable String clusterId, String clientId) {
super(registry, new String[] {NAME}, null);
this.clientId = clientId;
this.clusterId = clusterId;
}

@Override
Expand All @@ -43,6 +51,11 @@ protected String getGroupName(CharacterFilter filter) {
@Override
protected final void putVariables(Map<String, String> variables) {
variables.put("client_id", clientId);
if (clusterId != null) {
variables.put("cluster_id", clusterId);
} else {
variables.put("cluster_id", "");
}
}

public MetricRegistry getMetricRegistry() {
Expand Down
5 changes: 4 additions & 1 deletion fluss-server/src/main/java/org/apache/fluss/server/tablet/TabletServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,10 @@ protected void startServices() throws Exception {
// rpc client to sent request to the tablet server where the leader replica is located
// to fetch log.
this.clientMetricGroup =
new ClientMetricGroup(metricRegistry, SERVER_NAME + "-" + serverId);
new ClientMetricGroup(
metricRegistry,
ServerMetricUtils.validateAndGetClusterId(conf),
SERVER_NAME + "-" + serverId);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems more like server_id rather than cluster_id? Different server in same cluster will collect different viriable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems more like server_id rather than cluster_id? Different server in same cluster will collect different viriable.

server_id has already been added. Here I want to add cluster_id for client metric. The cluster_id is necessary when troubleshooting issues, for example, when examining metrics related to requests sent by the ReplicaFetcherThread. Currently, since the client metrics do not have a cluster_id label, we are unable to distinguish metrics from different clusters.

this.rpcClient = RpcClient.create(conf, clientMetricGroup, true);

CoordinatorGateway coordinatorGateway =
Expand Down