From 7a43c50184c137a0873b34b4839fe8dfedfb1138 Mon Sep 17 00:00:00 2001 From: adiraj_linkedin Date: Wed, 19 Nov 2025 10:30:31 +0530 Subject: [PATCH 1/8] added the otel changes --- .../linkedin/d2/balancer/D2ClientConfig.java | 137 ++++++++++++++++++ .../linkedin/d2/jmx/D2ClientJmxManager.java | 26 ++++ .../jmx/NoOpXdsClientOtelMetricsProvider.java | 63 ++++++++ .../com/linkedin/d2/jmx/XdsClientJmx.java | 38 ++++- .../d2/jmx/XdsClientOtelMetricsProvider.java | 29 ++++ .../com/linkedin/d2/xds/XdsClientImpl.java | 89 ++++++++++-- .../XdsLoadBalancerWithFacilitiesFactory.java | 1 + .../linkedin/d2/xds/TestXdsClientImpl.java | 2 +- 8 files changed, 371 insertions(+), 14 deletions(-) create mode 100644 d2/src/main/java/com/linkedin/d2/jmx/NoOpXdsClientOtelMetricsProvider.java create mode 100644 d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java diff --git a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java index d0ffa9e826..44ea58566c 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java @@ -44,6 +44,8 @@ import com.linkedin.d2.jmx.JmxManager; import com.linkedin.d2.jmx.NoOpXdsServerMetricsProvider; import com.linkedin.d2.jmx.NoOpJmxManager; +import com.linkedin.d2.jmx.XdsClientOtelMetricsProvider; +import com.linkedin.d2.jmx.NoOpXdsClientOtelMetricsProvider; import com.linkedin.r2.transport.common.TransportClientFactory; import io.grpc.netty.shaded.io.netty.handler.ssl.SslContext; import java.time.Duration; @@ -181,6 +183,7 @@ public class D2ClientConfig public boolean subscribeToUriGlobCollection = false; public XdsServerMetricsProvider _xdsServerMetricsProvider = new NoOpXdsServerMetricsProvider(); + public XdsClientOtelMetricsProvider _otelMetricsProvider = new NoOpXdsClientOtelMetricsProvider(); public boolean loadBalanceStreamException = false; public boolean xdsInitialResourceVersionsEnabled = false; public Integer xdsStreamMaxRetryBackoffSeconds = null; @@ -293,6 +296,139 @@ public D2ClientConfig() D2CalleeInfoRecorder d2CalleeInfoRecorder, Boolean enableIndisDownstreamServicesFetcher, Duration indisDownstreamServicesFetchTimeout) + { + this(zkHosts, xdsServer, hostName, zkSessionTimeoutInMs, zkStartupTimeoutInMs, lbWaitTimeout, lbWaitUnit, + flagFile, basePath, fsBasePath, indisFsBasePath, componentFactory, clientFactories, lbWithFacilitiesFactory, + sslContext, grpcSslContext, sslParameters, isSSLEnabled, shutdownAsynchronously, isSymlinkAware, + clientServicesConfig, d2ServicePath, useNewEphemeralStoreWatcher, healthCheckOperations, executorService, + retry, restRetryEnabled, streamRetryEnabled, retryLimit, retryUpdateIntervalMs, retryAggregatedIntervalNum, + warmUp, warmUpTimeoutSeconds, indisWarmUpTimeoutSeconds, warmUpConcurrentRequests, + indisWarmUpConcurrentRequests, downstreamServicesFetcher, indisDownstreamServicesFetcher, + backupRequestsEnabled, backupRequestsStrategyStatsConsumer, + backupRequestsLatencyNotificationInterval, + backupRequestsLatencyNotificationIntervalUnit, + enableBackupRequestsClientAsync, + backupRequestsExecutorService, + emitter, + partitionAccessorRegistry, + zooKeeperDecorator, + enableSaveUriDataOnDisk, + loadBalancerStrategyFactories, + requestTimeoutHandlerEnabled, + sslSessionValidatorFactory, + zkConnection, + startUpExecutorService, + indisStartUpExecutorService, + jmxManager, + d2JmxManagerPrefix, + zookeeperReadWindowMs, + enableRelativeLoadBalancer, + deterministicSubsettingMetadataProvider, + canaryDistributionProvider, + enableClusterFailout, + failoutConfigProviderFactory, + failoutRedirectStrategy, + serviceDiscoveryEventEmitter, + dualReadStateManager, + xdsExecutorService, + xdsStreamReadyTimeout, + dualReadNewLbExecutor, + xdsChannelLoadBalancingPolicy, + xdsChannelLoadBalancingPolicyConfig, + subscribeToUriGlobCollection, + xdsServerMetricsProvider, + new NoOpXdsClientOtelMetricsProvider(), + loadBalanceStreamException, + xdsInitialResourceVersionsEnabled, + disableDetectLiRawD2Client, + isLiRawD2Client, + xdsStreamMaxRetryBackoffSeconds, + xdsChannelKeepAliveTimeMins, + xdsMinimumJavaVersion, + actionOnPrecheckFailure); + } + + D2ClientConfig(String zkHosts, + String xdsServer, + String hostName, + long zkSessionTimeoutInMs, + long zkStartupTimeoutInMs, + long lbWaitTimeout, + TimeUnit lbWaitUnit, + String flagFile, + String basePath, + String fsBasePath, + String indisFsBasePath, + ComponentFactory componentFactory, + Map clientFactories, + LoadBalancerWithFacilitiesFactory lbWithFacilitiesFactory, + SSLContext sslContext, + SslContext grpcSslContext, + SSLParameters sslParameters, + boolean isSSLEnabled, + boolean shutdownAsynchronously, + boolean isSymlinkAware, + Map> clientServicesConfig, + String d2ServicePath, + boolean useNewEphemeralStoreWatcher, + HealthCheckOperations healthCheckOperations, + ScheduledExecutorService executorService, + boolean retry, + boolean restRetryEnabled, + boolean streamRetryEnabled, + int retryLimit, + long retryUpdateIntervalMs, + int retryAggregatedIntervalNum, + boolean warmUp, + int warmUpTimeoutSeconds, + int indisWarmUpTimeoutSeconds, + int warmUpConcurrentRequests, + int indisWarmUpConcurrentRequests, + DownstreamServicesFetcher downstreamServicesFetcher, + DownstreamServicesFetcher indisDownstreamServicesFetcher, + boolean backupRequestsEnabled, + BackupRequestsStrategyStatsConsumer backupRequestsStrategyStatsConsumer, + long backupRequestsLatencyNotificationInterval, + TimeUnit backupRequestsLatencyNotificationIntervalUnit, + boolean enableBackupRequestsClientAsync, + ScheduledExecutorService backupRequestsExecutorService, + EventEmitter emitter, + PartitionAccessorRegistry partitionAccessorRegistry, + Function zooKeeperDecorator, + boolean enableSaveUriDataOnDisk, + Map> loadBalancerStrategyFactories, + boolean requestTimeoutHandlerEnabled, + SslSessionValidatorFactory sslSessionValidatorFactory, + ZKPersistentConnection zkConnection, + ScheduledExecutorService startUpExecutorService, + ScheduledExecutorService indisStartUpExecutorService, + JmxManager jmxManager, + String d2JmxManagerPrefix, + int zookeeperReadWindowMs, + boolean enableRelativeLoadBalancer, + DeterministicSubsettingMetadataProvider deterministicSubsettingMetadataProvider, + CanaryDistributionProvider canaryDistributionProvider, + boolean enableClusterFailout, + FailoutConfigProviderFactory failoutConfigProviderFactory, + FailoutRedirectStrategy failoutRedirectStrategy, + ServiceDiscoveryEventEmitter serviceDiscoveryEventEmitter, + DualReadStateManager dualReadStateManager, + ScheduledExecutorService xdsExecutorService, + Long xdsStreamReadyTimeout, + ExecutorService dualReadNewLbExecutor, + String xdsChannelLoadBalancingPolicy, + Map xdsChannelLoadBalancingPolicyConfig, + boolean subscribeToUriGlobCollection, + XdsServerMetricsProvider xdsServerMetricsProvider, + XdsClientOtelMetricsProvider otelMetricsProvider, + boolean loadBalanceStreamException, + boolean xdsInitialResourceVersionsEnabled, + boolean disableDetectLiRawD2Client, + boolean isLiRawD2Client, + Integer xdsStreamMaxRetryBackoffSeconds, + Long xdsChannelKeepAliveTimeMins, + String xdsMinimumJavaVersion, + XdsClientValidator.ActionOnPrecheckFailure actionOnPrecheckFailure) { this.zkHosts = zkHosts; this.xdsServer = xdsServer; @@ -367,6 +503,7 @@ public D2ClientConfig() this.xdsChannelKeepAliveTimeMins = xdsChannelKeepAliveTimeMins; this.subscribeToUriGlobCollection = subscribeToUriGlobCollection; this._xdsServerMetricsProvider = xdsServerMetricsProvider; + this._otelMetricsProvider = otelMetricsProvider; this.loadBalanceStreamException = loadBalanceStreamException; this.xdsInitialResourceVersionsEnabled = xdsInitialResourceVersionsEnabled; this.disableDetectLiRawD2Client = disableDetectLiRawD2Client; diff --git a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java index 7795d8c80f..90710230cb 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java @@ -306,9 +306,35 @@ public void registerXdsClientJmx(XdsClientJmx xdsClientJmx) _log.warn("Setting XdsClientJmx for Non-XDS source type: {}", _discoverySourceType); } final String jmxName = String.format("%s-XdsClientJmx", getGlobalPrefix(null)); + + // Extract the actual client name from the JMX name pattern + String clientName = extractClientNameFromGlobalPrefix(getGlobalPrefix(null)); + xdsClientJmx.setClientName(clientName); + _jmxManager.registerXdsClientJmxBean(jmxName, xdsClientJmx); } + /** + * Extracts the client name from the JMX name. + * @param jmxName The full JMX name ending with "-XdsClientJmx" + * @return The extracted client name + */ + private String extractClientNameFromGlobalPrefix(String prefix) + { + if(prefix == null){ + return "-"; + } + int lastDash = prefix.lastIndexOf('-'); + if (lastDash == -1 || lastDash == prefix.length() - 1) { + return "-"; + } + String clientName = prefix.substring(lastDash + 1); + if (!clientName.endsWith("Client")) { + return "-"; + } + return clientName; + } + private void doRegisterLoadBalancer(SimpleLoadBalancer balancer, @Nullable DualReadModeProvider.DualReadMode mode) { final String jmxName = String.format("%s-LoadBalancer", getGlobalPrefix(mode)); diff --git a/d2/src/main/java/com/linkedin/d2/jmx/NoOpXdsClientOtelMetricsProvider.java b/d2/src/main/java/com/linkedin/d2/jmx/NoOpXdsClientOtelMetricsProvider.java new file mode 100644 index 0000000000..bc5933dc28 --- /dev/null +++ b/d2/src/main/java/com/linkedin/d2/jmx/NoOpXdsClientOtelMetricsProvider.java @@ -0,0 +1,63 @@ +package com.linkedin.d2.jmx; + +/** + * No-Op implementation of XdsClientOtelMetricsProvider. + * Used when OpenTelemetry metrics are disabled. + */ +public class NoOpXdsClientOtelMetricsProvider implements XdsClientOtelMetricsProvider { + + @Override + public void recordConnectionLost(String clientName) { + // No-op + } + + @Override + public void recordConnectionClosed(String clientName) { + // No-op + } + + @Override + public void recordReconnection(String clientName) { + // No-op + } + + @Override + public void recordRequestSent(String clientName) { + // No-op + } + + @Override + public void recordResponseReceived(String clientName) { + // No-op + } + + @Override + public void recordInitialResourceVersionSent(String clientName, int count) { + // No-op + } + + @Override + public void recordResourceNotFound(String clientName) { + // No-op + } + + @Override + public void recordResourceInvalid(String clientName) { + // No-op + } + + @Override + public void recordServerLatency(String clientName, long latencyMs) { + // No-op + } + + @Override + public void updateConnectionState(String clientName, boolean isConnected) { + // No-op + } + + @Override + public void updateActiveInitialWaitTime(String clientName, long waitTimeMs) { + // No-op + } +} \ No newline at end of file diff --git a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java index 0b0f643293..bffa6203ae 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java @@ -37,18 +37,39 @@ public class XdsClientJmx implements XdsClientJmxMBean private final AtomicInteger _resourceNotFoundCount = new AtomicInteger(); private final AtomicInteger _resourceInvalidCount = new AtomicInteger(); private final XdsServerMetricsProvider _xdsServerMetricsProvider; + private final XdsClientOtelMetricsProvider _otelMetricsProvider; + + private String _clientName = "-"; + @Nullable private XdsClientImpl _xdsClient = null; @Deprecated public XdsClientJmx() { - this(new NoOpXdsServerMetricsProvider()); + this(new NoOpXdsServerMetricsProvider(), new NoOpXdsClientOtelMetricsProvider()); } public XdsClientJmx(XdsServerMetricsProvider xdsServerMetricsProvider) + { + this(xdsServerMetricsProvider, new NoOpXdsClientOtelMetricsProvider()); + } + + public XdsClientJmx(XdsServerMetricsProvider xdsServerMetricsProvider, + XdsClientOtelMetricsProvider otelMetricsProvider) { _xdsServerMetricsProvider = xdsServerMetricsProvider == null ? new NoOpXdsServerMetricsProvider() : xdsServerMetricsProvider; + _otelMetricsProvider = otelMetricsProvider == null ? + new NoOpXdsClientOtelMetricsProvider() : otelMetricsProvider; + } + + // Method to set client name (called from D2ClientJmxManager) + public void setClientName(String clientName) { + _clientName = clientName != "-" ? clientName : "-"; + } + + public String getClientName() { + return _clientName; } public void setXdsClient(XdsClientImpl xdsClient) @@ -146,55 +167,66 @@ public int isDisconnected() @Override public long getActiveInitialWaitTimeMillis() { + long waitTime = -1; if (_xdsClient != null) { - return _xdsClient.getActiveInitialWaitTimeMillis(); + waitTime = _xdsClient.getActiveInitialWaitTimeMillis(); + _otelMetricsProvider.updateActiveInitialWaitTime(_clientName, waitTime); } - return -1; + return waitTime; } public void incrementConnectionLostCount() { _connectionLostCount.incrementAndGet(); + _otelMetricsProvider.recordConnectionLost(_clientName); } public void incrementConnectionClosedCount() { _connectionClosedCount.incrementAndGet(); + _otelMetricsProvider.recordConnectionClosed(_clientName); } public void incrementReconnectionCount() { _reconnectionCount.incrementAndGet(); + _otelMetricsProvider.recordReconnection(_clientName); } public void incrementRequestSentCount() { _resquestSentCount.incrementAndGet(); + _otelMetricsProvider.recordRequestSent(_clientName); } public void addToIrvSentCount(int delta) { _irvSentCount.addAndGet(delta); + _otelMetricsProvider.recordInitialResourceVersionSent(_clientName, delta); } public void incrementResponseReceivedCount() { _responseReceivedCount.incrementAndGet(); + _otelMetricsProvider.recordResponseReceived(_clientName); } public void setIsConnected(boolean connected) { _isConnected.getAndSet(connected); + _otelMetricsProvider.updateConnectionState(_clientName, connected); } public void incrementResourceNotFoundCount() { _resourceNotFoundCount.incrementAndGet(); + _otelMetricsProvider.recordResourceNotFound(_clientName); } public void incrementResourceInvalidCount() { _resourceInvalidCount.incrementAndGet(); + _otelMetricsProvider.recordResourceInvalid(_clientName); } } diff --git a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java new file mode 100644 index 0000000000..a6bd706676 --- /dev/null +++ b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java @@ -0,0 +1,29 @@ +package com.linkedin.d2.jmx; + +/** + * Interface for OpenTelemetry metrics collection for XDS Client. + * Provides raw event-based metrics that will be processed by OpenTelemetry in the container. + */ +public interface XdsClientOtelMetricsProvider { + + // Connection state changes (event-driven counters) + void recordConnectionLost(String clientName); + void recordConnectionClosed(String clientName); + void recordReconnection(String clientName); + + // Request/Response counters (event-driven) + void recordRequestSent(String clientName); + void recordResponseReceived(String clientName); + void recordInitialResourceVersionSent(String clientName, int count); + + // Error counters (event-driven) + void recordResourceNotFound(String clientName); + void recordResourceInvalid(String clientName); + + // Server latency histogram (event-driven) + void recordServerLatency(String clientName, long latencyMs); + + // Gauge updates (on-demand) + void updateConnectionState(String clientName, boolean isConnected); + void updateActiveInitialWaitTime(String clientName, long waitTimeMs); +} \ No newline at end of file diff --git a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java index e4faa58326..210625f6fe 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java +++ b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java @@ -23,8 +23,10 @@ import com.google.common.collect.Maps; import com.google.protobuf.util.Timestamps; import com.google.rpc.Code; +import com.linkedin.d2.jmx.NoOpXdsClientOtelMetricsProvider; import com.linkedin.d2.jmx.NoOpXdsServerMetricsProvider; import com.linkedin.d2.jmx.XdsClientJmx; +import com.linkedin.d2.jmx.XdsClientOtelMetricsProvider; import com.linkedin.d2.jmx.XdsServerMetricsProvider; import com.linkedin.d2.xds.GlobCollectionUtils.D2UriIdentifier; import com.linkedin.util.RateLimitedLogger; @@ -115,6 +117,7 @@ public class XdsClientImpl extends XdsClient private final XdsClientJmx _xdsClientJmx; private final XdsServerMetricsProvider _serverMetricsProvider; + private final XdsClientOtelMetricsProvider _otelMetricsProvider; private final boolean _initialResourceVersionsEnabled; private final String _minimumJavaVersion; private final XdsClientValidator.ActionOnPrecheckFailure _actionOnPrecheckFailure; @@ -200,6 +203,22 @@ public XdsClientImpl(Node node, Integer maxRetryBackoffSeconds, String minimumJavaVersion, XdsClientValidator.ActionOnPrecheckFailure actionOnPrecheckFailure) + { + this(node, managedChannel, executorService, readyTimeoutMillis, subscribeToUriGlobCollection, + serverMetricsProvider, new NoOpXdsClientOtelMetricsProvider(), irvSupport, maxRetryBackoffSeconds, XdsClientValidator.DEFAULT_MINIMUM_JAVA_VERSION, XdsClientValidator.DEFAULT_ACTION_ON_PRECHECK_FAILURE); + } + + public XdsClientImpl(Node node, + ManagedChannel managedChannel, + ScheduledExecutorService executorService, + long readyTimeoutMillis, + boolean subscribeToUriGlobCollection, + XdsServerMetricsProvider serverMetricsProvider, + XdsClientOtelMetricsProvider otelMetricsProvider, + boolean irvSupport, + Integer maxRetryBackoffSeconds, + String minimumJavaVersion, + XdsClientValidator.ActionOnPrecheckFailure actionOnPrecheckFailure) { _readyTimeoutMillis = readyTimeoutMillis; _node = node; @@ -212,8 +231,9 @@ public XdsClientImpl(Node node, _log.info("Glob collection support enabled"); } - _xdsClientJmx = new XdsClientJmx(serverMetricsProvider); + _xdsClientJmx = new XdsClientJmx(serverMetricsProvider, otelMetricsProvider); _serverMetricsProvider = serverMetricsProvider == null ? new NoOpXdsServerMetricsProvider() : serverMetricsProvider; + _otelMetricsProvider = otelMetricsProvider == null ? new NoOpXdsClientOtelMetricsProvider() : otelMetricsProvider; _initialResourceVersionsEnabled = irvSupport; if (_initialResourceVersionsEnabled) { @@ -799,7 +819,9 @@ private void handleResourceUpdate(Map updates, if (wildcardSubscriber != null) { - wildcardSubscriber.onData(entry.getKey(), entry.getValue(), _serverMetricsProvider); + // wildcardSubscriber.onData(entry.getKey(), entry.getValue(), _serverMetricsProvider); + String clientName = _xdsClientJmx.getClientName(); + wildcardSubscriber.onData(entry.getKey(), entry.getValue(), _serverMetricsProvider, _otelMetricsProvider, clientName); } } } @@ -996,6 +1018,12 @@ void addWatcher(ResourceWatcher watcher) @VisibleForTesting void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider) + { + onData(data, metricsProvider, null); + } + + @VisibleForTesting + void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider, XdsClientOtelMetricsProvider otelMetricsProvider) { SubscriberFetchState prev = _fetchState.getAndSet(FETCHED); if (!FETCHED.equals(prev)) @@ -1009,7 +1037,11 @@ void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider) { // Even though the data is the same, the subscriber is waiting for init data after either startup // or a reconnection, so we need to track latency. - trackServerLatency(data, _data, metricsProvider, _subscribedAt.get(), _isIrvEnabled, prev); + // trackServerLatency(data, _data, metricsProvider, _subscribedAt.get(), _isIrvEnabled, prev); + // Get client name and OpenTelemetry provider from XdsClientJmx + String clientName = _xdsClientJmx.getClientName(); + // data updated, track xds server latency + trackServerLatency(data, _data, metricsProvider, _subscribedAt.get(), _isIrvEnabled, prev, otelMetricsProvider, clientName); } _log.debug("Received resource update data equal to the current data. Will not perform any update."); return; @@ -1191,6 +1223,13 @@ void addWatcher(WildcardResourceWatcher watcher) @VisibleForTesting void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider metricsProvider) + { + onData(resourceName, data, metricsProvider, null, "-"); + } + + @VisibleForTesting + void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider metricsProvider, + XdsClientOtelMetricsProvider otelMetricsProvider, String clientName) { if (Objects.equals(_data.get(resourceName), data)) { @@ -1198,7 +1237,9 @@ void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider m { // Even though the data is the same, the subscriber is waiting for init data after either startup // or a reconnection, so we need to track latency. - trackServerLatency(data, _data.get(resourceName), metricsProvider, _subscribedAt.get(), _isIrvEnabled, _fetchState.get()); + // trackServerLatency(data, _data.get(resourceName), metricsProvider, _subscribedAt.get(), _isIrvEnabled, _fetchState.get()); + // Now we can pass OpenTelemetry provider and client name for wildcard subscribers too + trackServerLatency(data, _data.get(resourceName), metricsProvider, _subscribedAt.get(), _isIrvEnabled, _fetchState.get(), otelMetricsProvider, clientName); } _log.debug("Received resource update data equal to the current data. Will not perform the update."); return; @@ -1345,6 +1386,13 @@ private boolean shouldSubscribeUriGlobCollection(ResourceType type) private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUpdate currentData, XdsServerMetricsProvider metricsProvider, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) + { + trackServerLatency(resourceUpdate, currentData, metricsProvider, subscribedAt, isIrvEnabled, fetchState, null, "-"); + } + + private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUpdate currentData, + XdsServerMetricsProvider metricsProvider, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState, + XdsClientOtelMetricsProvider otelMetricsProvider, String clientName) { long now = SystemClock.instance().currentTimeMillis(); if (resourceUpdate instanceof NodeUpdate) @@ -1355,7 +1403,7 @@ private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUp return; } trackServerLatencyHelper(metricsProvider, now, nodeData.getStat().getMtime(), subscribedAt, - isIrvEnabled, fetchState); + isIrvEnabled, fetchState, otelMetricsProvider, clientName); } else if (resourceUpdate instanceof D2URIMapUpdate) { @@ -1370,9 +1418,9 @@ else if (resourceUpdate instanceof D2URIMapUpdate) Map.Entry::getKey, e -> e.getValue().leftValue()) // new data of updated uris ); - trackServerLatencyForUris(updatedUris, update, metricsProvider, now, subscribedAt, isIrvEnabled, fetchState); + trackServerLatencyForUris(updatedUris, update, metricsProvider, now, subscribedAt, isIrvEnabled, fetchState, otelMetricsProvider, clientName); trackServerLatencyForUris(rawDiff.entriesOnlyOnLeft(), update, metricsProvider, now, subscribedAt, - isIrvEnabled, fetchState); // newly added uris + isIrvEnabled, fetchState, otelMetricsProvider, clientName); // newly added uris } else if (resourceUpdate instanceof D2URIUpdate) { @@ -1382,7 +1430,7 @@ else if (resourceUpdate instanceof D2URIUpdate) { update.setIsStaleModifiedTime( trackServerLatencyHelper(metricsProvider, now, Timestamps.toMillis(uri.getModifiedTime()), subscribedAt, - isIrvEnabled, fetchState) + isIrvEnabled, fetchState, otelMetricsProvider, clientName) ); } } @@ -1391,10 +1439,17 @@ else if (resourceUpdate instanceof D2URIUpdate) private static void trackServerLatencyForUris(Map uriMap, D2URIMapUpdate update, XdsServerMetricsProvider metricsProvider, long end, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) + { + trackServerLatencyForUris(uriMap, update, metricsProvider, end, subscribedAt, isIrvEnabled, fetchState, null, "-"); + } + + private static void trackServerLatencyForUris(Map uriMap, D2URIMapUpdate update, + XdsServerMetricsProvider metricsProvider, long end, long subscribedAt, boolean isIrvEnabled, + SubscriberFetchState fetchState, XdsClientOtelMetricsProvider otelMetricsProvider, String clientName) { uriMap.forEach((k, v) -> { boolean isStaleModifiedTime = trackServerLatencyHelper(metricsProvider, end, Timestamps.toMillis(v.getModifiedTime()), subscribedAt, - isIrvEnabled, fetchState); + isIrvEnabled, fetchState, otelMetricsProvider, clientName); update.setIsStaleModifiedTime(k, isStaleModifiedTime); } ); @@ -1410,6 +1465,13 @@ private static void trackServerLatencyForUris(Map uriMap, D // on the resource modified time. private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metricsProvider, long end, long modifiedAt, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) + { + return trackServerLatencyHelper(metricsProvider, end, modifiedAt, subscribedAt, isIrvEnabled, fetchState, null, "-"); + } + + private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metricsProvider, + long end, long modifiedAt, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState, + XdsClientOtelMetricsProvider otelMetricsProvider, String clientName) { long start; boolean isStaleModifiedAt; @@ -1423,7 +1485,14 @@ private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metrics start = Math.max(modifiedAt, subscribedAt); isStaleModifiedAt = modifiedAt < subscribedAt; } - metricsProvider.trackLatency(end - start); + long latency = end - start; + metricsProvider.trackLatency(latency); + + // Record OpenTelemetry latency if provider is available + if (otelMetricsProvider != null) + { + otelMetricsProvider.recordServerLatency(clientName, latency); + } return isStaleModifiedAt; } diff --git a/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java b/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java index 4c77c81433..2c7ba2f101 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java +++ b/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java @@ -69,6 +69,7 @@ public LoadBalancerWithFacilities create(D2ClientConfig config) xdsStreamReadyTimeout, config.subscribeToUriGlobCollection, config._xdsServerMetricsProvider, + config._otelMetricsProvider, config.xdsInitialResourceVersionsEnabled, config.xdsStreamMaxRetryBackoffSeconds, config.xdsMinimumJavaVersion, diff --git a/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java b/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java index 57c27f34aa..d826a1ec2e 100644 --- a/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java +++ b/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java @@ -542,7 +542,7 @@ public void testHandleD2URIMapUpdateWithEmptyResponse() { fixture.verifyAckSent(2); // onData is called only once. Empty response does not trigger onData calls. verify(fixture._clusterSubscriber).onData(any(), any()); - verify(fixture._uriMapWildcardSubscriber).onData(any(), any(), any()); + verify(fixture._uriMapWildcardSubscriber).onData(any(), any(), any(), any(), any()); } @Test(dataProvider = "providerWatcherFlags") From 54a840160654a5bfb31004feff485deae171dc75 Mon Sep 17 00:00:00 2001 From: adiraj_linkedin Date: Sun, 23 Nov 2025 10:51:58 +0530 Subject: [PATCH 2/8] Added more logic for the XdsClient Otel metrics --- .../linkedin/d2/balancer/D2ClientBuilder.java | 7 ++ .../linkedin/d2/balancer/D2ClientConfig.java | 16 +++-- .../linkedin/d2/jmx/D2ClientJmxManager.java | 25 +------ .../com/linkedin/d2/jmx/XdsClientJmx.java | 34 ++++----- .../d2/jmx/XdsClientOtelMetricsProvider.java | 70 +++++++++++++++++-- .../com/linkedin/d2/xds/XdsClientImpl.java | 68 +++++++++--------- .../XdsLoadBalancerWithFacilitiesFactory.java | 2 +- .../linkedin/d2/xds/TestXdsClientImpl.java | 8 ++- 8 files changed, 141 insertions(+), 89 deletions(-) diff --git a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java index 88dba60f21..c330297a20 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java @@ -52,6 +52,7 @@ import com.linkedin.d2.discovery.stores.zk.ZKPersistentConnection; import com.linkedin.d2.discovery.stores.zk.ZooKeeper; import com.linkedin.d2.jmx.XdsServerMetricsProvider; +import com.linkedin.d2.jmx.XdsClientOtelMetricsProvider; import com.linkedin.d2.jmx.JmxManager; import com.linkedin.d2.xds.XdsClientValidator; import com.linkedin.d2.jmx.NoOpJmxManager; @@ -237,6 +238,7 @@ public D2Client build() _config.xdsChannelLoadBalancingPolicyConfig, _config.subscribeToUriGlobCollection, _config._xdsServerMetricsProvider, + _config._xdsClientOtelMetricsProvider, _config.loadBalanceStreamException, _config.xdsInitialResourceVersionsEnabled, _config.disableDetectLiRawD2Client, @@ -856,6 +858,11 @@ public D2ClientBuilder setXdsServerMetricsProvider(XdsServerMetricsProvider xdsS return this; } + public D2ClientBuilder setXdsClientOtelMetricsProvider(XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider) { + _config._xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider; + return this; + } + public D2ClientBuilder setLoadBalanceStreamException(boolean loadBalanceStreamException) { _config.loadBalanceStreamException = loadBalanceStreamException; return this; diff --git a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java index 44ea58566c..cad73b3c45 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java @@ -183,7 +183,7 @@ public class D2ClientConfig public boolean subscribeToUriGlobCollection = false; public XdsServerMetricsProvider _xdsServerMetricsProvider = new NoOpXdsServerMetricsProvider(); - public XdsClientOtelMetricsProvider _otelMetricsProvider = new NoOpXdsClientOtelMetricsProvider(); + public XdsClientOtelMetricsProvider _xdsClientOtelMetricsProvider = new NoOpXdsClientOtelMetricsProvider(); public boolean loadBalanceStreamException = false; public boolean xdsInitialResourceVersionsEnabled = false; public Integer xdsStreamMaxRetryBackoffSeconds = null; @@ -345,7 +345,10 @@ public D2ClientConfig() xdsStreamMaxRetryBackoffSeconds, xdsChannelKeepAliveTimeMins, xdsMinimumJavaVersion, - actionOnPrecheckFailure); + actionOnPrecheckFailure, + d2CalleeInfoRecorder, + enableIndisDownstreamServicesFetcher, + indisDownstreamServicesFetchTimeout); } D2ClientConfig(String zkHosts, @@ -420,7 +423,7 @@ public D2ClientConfig() Map xdsChannelLoadBalancingPolicyConfig, boolean subscribeToUriGlobCollection, XdsServerMetricsProvider xdsServerMetricsProvider, - XdsClientOtelMetricsProvider otelMetricsProvider, + XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, boolean loadBalanceStreamException, boolean xdsInitialResourceVersionsEnabled, boolean disableDetectLiRawD2Client, @@ -428,7 +431,10 @@ public D2ClientConfig() Integer xdsStreamMaxRetryBackoffSeconds, Long xdsChannelKeepAliveTimeMins, String xdsMinimumJavaVersion, - XdsClientValidator.ActionOnPrecheckFailure actionOnPrecheckFailure) + XdsClientValidator.ActionOnPrecheckFailure actionOnPrecheckFailure, + D2CalleeInfoRecorder d2CalleeInfoRecorder, + Boolean enableIndisDownstreamServicesFetcher, + Duration indisDownstreamServicesFetchTimeout) { this.zkHosts = zkHosts; this.xdsServer = xdsServer; @@ -503,7 +509,7 @@ public D2ClientConfig() this.xdsChannelKeepAliveTimeMins = xdsChannelKeepAliveTimeMins; this.subscribeToUriGlobCollection = subscribeToUriGlobCollection; this._xdsServerMetricsProvider = xdsServerMetricsProvider; - this._otelMetricsProvider = otelMetricsProvider; + this._xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider; this.loadBalanceStreamException = loadBalanceStreamException; this.xdsInitialResourceVersionsEnabled = xdsInitialResourceVersionsEnabled; this.disableDetectLiRawD2Client = disableDetectLiRawD2Client; diff --git a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java index 90710230cb..f640f5bd9a 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java @@ -307,34 +307,13 @@ public void registerXdsClientJmx(XdsClientJmx xdsClientJmx) } final String jmxName = String.format("%s-XdsClientJmx", getGlobalPrefix(null)); - // Extract the actual client name from the JMX name pattern - String clientName = extractClientNameFromGlobalPrefix(getGlobalPrefix(null)); + // Get the client name from global prefix + String clientName = getGlobalPrefix(null); xdsClientJmx.setClientName(clientName); _jmxManager.registerXdsClientJmxBean(jmxName, xdsClientJmx); } - /** - * Extracts the client name from the JMX name. - * @param jmxName The full JMX name ending with "-XdsClientJmx" - * @return The extracted client name - */ - private String extractClientNameFromGlobalPrefix(String prefix) - { - if(prefix == null){ - return "-"; - } - int lastDash = prefix.lastIndexOf('-'); - if (lastDash == -1 || lastDash == prefix.length() - 1) { - return "-"; - } - String clientName = prefix.substring(lastDash + 1); - if (!clientName.endsWith("Client")) { - return "-"; - } - return clientName; - } - private void doRegisterLoadBalancer(SimpleLoadBalancer balancer, @Nullable DualReadModeProvider.DualReadMode mode) { final String jmxName = String.format("%s-LoadBalancer", getGlobalPrefix(mode)); diff --git a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java index bffa6203ae..c406e1b152 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java @@ -37,7 +37,7 @@ public class XdsClientJmx implements XdsClientJmxMBean private final AtomicInteger _resourceNotFoundCount = new AtomicInteger(); private final AtomicInteger _resourceInvalidCount = new AtomicInteger(); private final XdsServerMetricsProvider _xdsServerMetricsProvider; - private final XdsClientOtelMetricsProvider _otelMetricsProvider; + private final XdsClientOtelMetricsProvider _xdsClientOtelMetricsProvider; private String _clientName = "-"; @@ -46,26 +46,26 @@ public class XdsClientJmx implements XdsClientJmxMBean @Deprecated public XdsClientJmx() { - this(new NoOpXdsServerMetricsProvider(), new NoOpXdsClientOtelMetricsProvider()); + this(new NoOpXdsServerMetricsProvider(), null); } public XdsClientJmx(XdsServerMetricsProvider xdsServerMetricsProvider) { - this(xdsServerMetricsProvider, new NoOpXdsClientOtelMetricsProvider()); + this(xdsServerMetricsProvider, null); } public XdsClientJmx(XdsServerMetricsProvider xdsServerMetricsProvider, - XdsClientOtelMetricsProvider otelMetricsProvider) + XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider) { _xdsServerMetricsProvider = xdsServerMetricsProvider == null ? new NoOpXdsServerMetricsProvider() : xdsServerMetricsProvider; - _otelMetricsProvider = otelMetricsProvider == null ? - new NoOpXdsClientOtelMetricsProvider() : otelMetricsProvider; + _xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider == null ? + new NoOpXdsClientOtelMetricsProvider() : xdsClientOtelMetricsProvider; } // Method to set client name (called from D2ClientJmxManager) public void setClientName(String clientName) { - _clientName = clientName != "-" ? clientName : "-"; + _clientName = clientName; } public String getClientName() { @@ -171,7 +171,7 @@ public long getActiveInitialWaitTimeMillis() if (_xdsClient != null) { waitTime = _xdsClient.getActiveInitialWaitTimeMillis(); - _otelMetricsProvider.updateActiveInitialWaitTime(_clientName, waitTime); + _xdsClientOtelMetricsProvider.updateActiveInitialWaitTime(_clientName, waitTime); } return waitTime; } @@ -179,54 +179,54 @@ public long getActiveInitialWaitTimeMillis() public void incrementConnectionLostCount() { _connectionLostCount.incrementAndGet(); - _otelMetricsProvider.recordConnectionLost(_clientName); + _xdsClientOtelMetricsProvider.recordConnectionLost(_clientName); } public void incrementConnectionClosedCount() { _connectionClosedCount.incrementAndGet(); - _otelMetricsProvider.recordConnectionClosed(_clientName); + _xdsClientOtelMetricsProvider.recordConnectionClosed(_clientName); } public void incrementReconnectionCount() { _reconnectionCount.incrementAndGet(); - _otelMetricsProvider.recordReconnection(_clientName); + _xdsClientOtelMetricsProvider.recordReconnection(_clientName); } public void incrementRequestSentCount() { _resquestSentCount.incrementAndGet(); - _otelMetricsProvider.recordRequestSent(_clientName); + _xdsClientOtelMetricsProvider.recordRequestSent(_clientName); } public void addToIrvSentCount(int delta) { _irvSentCount.addAndGet(delta); - _otelMetricsProvider.recordInitialResourceVersionSent(_clientName, delta); + _xdsClientOtelMetricsProvider.recordInitialResourceVersionSent(_clientName, delta); } public void incrementResponseReceivedCount() { _responseReceivedCount.incrementAndGet(); - _otelMetricsProvider.recordResponseReceived(_clientName); + _xdsClientOtelMetricsProvider.recordResponseReceived(_clientName); } public void setIsConnected(boolean connected) { _isConnected.getAndSet(connected); - _otelMetricsProvider.updateConnectionState(_clientName, connected); + _xdsClientOtelMetricsProvider.updateConnectionState(_clientName, connected); } public void incrementResourceNotFoundCount() { _resourceNotFoundCount.incrementAndGet(); - _otelMetricsProvider.recordResourceNotFound(_clientName); + _xdsClientOtelMetricsProvider.recordResourceNotFound(_clientName); } public void incrementResourceInvalidCount() { _resourceInvalidCount.incrementAndGet(); - _otelMetricsProvider.recordResourceInvalid(_clientName); + _xdsClientOtelMetricsProvider.recordResourceInvalid(_clientName); } } diff --git a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java index a6bd706676..9ed59b0194 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java @@ -6,24 +6,84 @@ */ public interface XdsClientOtelMetricsProvider { - // Connection state changes (event-driven counters) + /** + * Records a connection lost event in the OpenTelemetry counter. + * + * @param clientName the name of the XDS client + */ void recordConnectionLost(String clientName); + + /** + * Records a connection closed event in the OpenTelemetry counter. + * + * @param clientName the name of the XDS client + */ void recordConnectionClosed(String clientName); + + /** + * Records a reconnection event in the OpenTelemetry counter. + * + * @param clientName the name of the XDS client + */ void recordReconnection(String clientName); - // Request/Response counters (event-driven) + /** + * Records a request sent event in the OpenTelemetry counter. + * + * @param clientName the name of the XDS client + */ void recordRequestSent(String clientName); + + /** + * Records a response received event in the OpenTelemetry counter. + * + * @param clientName the name of the XDS client + */ void recordResponseReceived(String clientName); + + /** + * Records initial resource version sent count in the OpenTelemetry counter. + * + * @param clientName the name of the XDS client + * @param count the count to add + */ void recordInitialResourceVersionSent(String clientName, int count); - // Error counters (event-driven) + /** + * Records a resource not found error in the OpenTelemetry counter. + * + * @param clientName the name of the XDS client + */ void recordResourceNotFound(String clientName); + + /** + * Records a resource invalid error in the OpenTelemetry counter. + * + * @param clientName the name of the XDS client + */ void recordResourceInvalid(String clientName); - // Server latency histogram (event-driven) + /** + * Records server latency in the OpenTelemetry histogram. + * + * @param clientName the name of the XDS client + * @param latencyMs the latency in milliseconds + */ void recordServerLatency(String clientName, long latencyMs); - // Gauge updates (on-demand) + /** + * Updates the connection state for a client. + * + * @param clientName the name of the XDS client + * @param isConnected whether the client is connected + */ void updateConnectionState(String clientName, boolean isConnected); + + /** + * Updates the active initial wait time for a client. + * + * @param clientName the name of the XDS client + * @param waitTimeMs the wait time in milliseconds + */ void updateActiveInitialWaitTime(String clientName, long waitTimeMs); } \ No newline at end of file diff --git a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java index 210625f6fe..8b4d56aa68 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java +++ b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java @@ -84,6 +84,7 @@ public class XdsClientImpl extends XdsClient new RateLimitedLogger(_log, TimeUnit.MINUTES.toMillis(1), SystemClock.instance()); public static final long DEFAULT_READY_TIMEOUT_MILLIS = 2000L; public static final Integer DEFAULT_MAX_RETRY_BACKOFF_SECS = 30; // default value for max retry backoff seconds + private static final String NO_VALUE = "-"; /** * The resource subscribers maps the resource type to its subscribers. Note that the {@link ResourceType#D2_URI} @@ -117,7 +118,7 @@ public class XdsClientImpl extends XdsClient private final XdsClientJmx _xdsClientJmx; private final XdsServerMetricsProvider _serverMetricsProvider; - private final XdsClientOtelMetricsProvider _otelMetricsProvider; + private final XdsClientOtelMetricsProvider _xdsClientOtelMetricsProvider; private final boolean _initialResourceVersionsEnabled; private final String _minimumJavaVersion; private final XdsClientValidator.ActionOnPrecheckFailure _actionOnPrecheckFailure; @@ -205,7 +206,7 @@ public XdsClientImpl(Node node, XdsClientValidator.ActionOnPrecheckFailure actionOnPrecheckFailure) { this(node, managedChannel, executorService, readyTimeoutMillis, subscribeToUriGlobCollection, - serverMetricsProvider, new NoOpXdsClientOtelMetricsProvider(), irvSupport, maxRetryBackoffSeconds, XdsClientValidator.DEFAULT_MINIMUM_JAVA_VERSION, XdsClientValidator.DEFAULT_ACTION_ON_PRECHECK_FAILURE); + serverMetricsProvider, null, irvSupport, maxRetryBackoffSeconds, XdsClientValidator.DEFAULT_MINIMUM_JAVA_VERSION, XdsClientValidator.DEFAULT_ACTION_ON_PRECHECK_FAILURE); } public XdsClientImpl(Node node, @@ -214,7 +215,7 @@ public XdsClientImpl(Node node, long readyTimeoutMillis, boolean subscribeToUriGlobCollection, XdsServerMetricsProvider serverMetricsProvider, - XdsClientOtelMetricsProvider otelMetricsProvider, + XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, boolean irvSupport, Integer maxRetryBackoffSeconds, String minimumJavaVersion, @@ -231,9 +232,9 @@ public XdsClientImpl(Node node, _log.info("Glob collection support enabled"); } - _xdsClientJmx = new XdsClientJmx(serverMetricsProvider, otelMetricsProvider); + _xdsClientJmx = new XdsClientJmx(serverMetricsProvider, xdsClientOtelMetricsProvider); _serverMetricsProvider = serverMetricsProvider == null ? new NoOpXdsServerMetricsProvider() : serverMetricsProvider; - _otelMetricsProvider = otelMetricsProvider == null ? new NoOpXdsClientOtelMetricsProvider() : otelMetricsProvider; + _xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider == null ? new NoOpXdsClientOtelMetricsProvider() : xdsClientOtelMetricsProvider; _initialResourceVersionsEnabled = irvSupport; if (_initialResourceVersionsEnabled) { @@ -698,7 +699,7 @@ private void handleD2URICollectionResponse(DiscoveryResponseData data) || uriSubscriber.getData() == null // The URI was corrupted and there was no previous version of this URI ) { - uriSubscriber.onData(new D2URIUpdate(uri), _serverMetricsProvider); + uriSubscriber.onData(new D2URIUpdate(uri), _serverMetricsProvider, _xdsClientOtelMetricsProvider, _xdsClientJmx.getClientName()); } } @@ -806,6 +807,7 @@ private void processResourceChanges(ResourceType type, Map updates, ResourceType type) { + String clientName = _xdsClientJmx.getClientName(); Map subscribers = getResourceSubscriberMap(type); WildcardResourceSubscriber wildcardSubscriber = getWildcardResourceSubscriber(type); @@ -814,14 +816,12 @@ private void handleResourceUpdate(Map updates, ResourceSubscriber subscriber = subscribers.get(entry.getKey()); if (subscriber != null) { - subscriber.onData(entry.getValue(), _serverMetricsProvider); + subscriber.onData(entry.getValue(), _serverMetricsProvider, _xdsClientOtelMetricsProvider, clientName); } if (wildcardSubscriber != null) { - // wildcardSubscriber.onData(entry.getKey(), entry.getValue(), _serverMetricsProvider); - String clientName = _xdsClientJmx.getClientName(); - wildcardSubscriber.onData(entry.getKey(), entry.getValue(), _serverMetricsProvider, _otelMetricsProvider, clientName); + wildcardSubscriber.onData(entry.getKey(), entry.getValue(), _serverMetricsProvider, _xdsClientOtelMetricsProvider, clientName); } } } @@ -1019,11 +1019,11 @@ void addWatcher(ResourceWatcher watcher) @VisibleForTesting void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider) { - onData(data, metricsProvider, null); + onData(data, metricsProvider, null, NO_VALUE); } @VisibleForTesting - void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider, XdsClientOtelMetricsProvider otelMetricsProvider) + void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider, XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) { SubscriberFetchState prev = _fetchState.getAndSet(FETCHED); if (!FETCHED.equals(prev)) @@ -1037,11 +1037,8 @@ void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider, XdsCl { // Even though the data is the same, the subscriber is waiting for init data after either startup // or a reconnection, so we need to track latency. - // trackServerLatency(data, _data, metricsProvider, _subscribedAt.get(), _isIrvEnabled, prev); - // Get client name and OpenTelemetry provider from XdsClientJmx - String clientName = _xdsClientJmx.getClientName(); // data updated, track xds server latency - trackServerLatency(data, _data, metricsProvider, _subscribedAt.get(), _isIrvEnabled, prev, otelMetricsProvider, clientName); + trackServerLatency(data, _data, metricsProvider, _subscribedAt.get(), _isIrvEnabled, prev, xdsClientOtelMetricsProvider, clientName); } _log.debug("Received resource update data equal to the current data. Will not perform any update."); return; @@ -1050,7 +1047,7 @@ void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider, XdsCl // null value guard to avoid overwriting the property with null if (data != null && data.isValid()) { - trackServerLatency(data, _data, metricsProvider, _subscribedAt.get(), _isIrvEnabled, prev); + trackServerLatency(data, _data, metricsProvider, _subscribedAt.get(), _isIrvEnabled, prev, xdsClientOtelMetricsProvider, clientName); _data = data; } else @@ -1224,12 +1221,12 @@ void addWatcher(WildcardResourceWatcher watcher) @VisibleForTesting void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider metricsProvider) { - onData(resourceName, data, metricsProvider, null, "-"); + onData(resourceName, data, metricsProvider, null, NO_VALUE); } @VisibleForTesting void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider metricsProvider, - XdsClientOtelMetricsProvider otelMetricsProvider, String clientName) + XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) { if (Objects.equals(_data.get(resourceName), data)) { @@ -1237,9 +1234,8 @@ void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider m { // Even though the data is the same, the subscriber is waiting for init data after either startup // or a reconnection, so we need to track latency. - // trackServerLatency(data, _data.get(resourceName), metricsProvider, _subscribedAt.get(), _isIrvEnabled, _fetchState.get()); - // Now we can pass OpenTelemetry provider and client name for wildcard subscribers too - trackServerLatency(data, _data.get(resourceName), metricsProvider, _subscribedAt.get(), _isIrvEnabled, _fetchState.get(), otelMetricsProvider, clientName); + // we are passing OpenTelemetry provider for wildcard subscribers too + trackServerLatency(data, _data.get(resourceName), metricsProvider, _subscribedAt.get(), _isIrvEnabled, _fetchState.get(), xdsClientOtelMetricsProvider, clientName); } _log.debug("Received resource update data equal to the current data. Will not perform the update."); return; @@ -1247,7 +1243,7 @@ void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider m // null value guard to avoid overwriting the property with null if (data != null && data.isValid()) { - trackServerLatency(data, _data.get(resourceName), metricsProvider, _subscribedAt.get(), _isIrvEnabled, _fetchState.get()); + trackServerLatency(data, _data.get(resourceName), metricsProvider, _subscribedAt.get(), _isIrvEnabled, _fetchState.get(), xdsClientOtelMetricsProvider, clientName); _data.put(resourceName, data); } else @@ -1387,12 +1383,12 @@ private boolean shouldSubscribeUriGlobCollection(ResourceType type) private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUpdate currentData, XdsServerMetricsProvider metricsProvider, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) { - trackServerLatency(resourceUpdate, currentData, metricsProvider, subscribedAt, isIrvEnabled, fetchState, null, "-"); + trackServerLatency(resourceUpdate, currentData, metricsProvider, subscribedAt, isIrvEnabled, fetchState, null, NO_VALUE); } private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUpdate currentData, XdsServerMetricsProvider metricsProvider, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState, - XdsClientOtelMetricsProvider otelMetricsProvider, String clientName) + XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) { long now = SystemClock.instance().currentTimeMillis(); if (resourceUpdate instanceof NodeUpdate) @@ -1403,7 +1399,7 @@ private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUp return; } trackServerLatencyHelper(metricsProvider, now, nodeData.getStat().getMtime(), subscribedAt, - isIrvEnabled, fetchState, otelMetricsProvider, clientName); + isIrvEnabled, fetchState, xdsClientOtelMetricsProvider, clientName); } else if (resourceUpdate instanceof D2URIMapUpdate) { @@ -1418,9 +1414,9 @@ else if (resourceUpdate instanceof D2URIMapUpdate) Map.Entry::getKey, e -> e.getValue().leftValue()) // new data of updated uris ); - trackServerLatencyForUris(updatedUris, update, metricsProvider, now, subscribedAt, isIrvEnabled, fetchState, otelMetricsProvider, clientName); + trackServerLatencyForUris(updatedUris, update, metricsProvider, now, subscribedAt, isIrvEnabled, fetchState, xdsClientOtelMetricsProvider, clientName); trackServerLatencyForUris(rawDiff.entriesOnlyOnLeft(), update, metricsProvider, now, subscribedAt, - isIrvEnabled, fetchState, otelMetricsProvider, clientName); // newly added uris + isIrvEnabled, fetchState, xdsClientOtelMetricsProvider, clientName); // newly added uris } else if (resourceUpdate instanceof D2URIUpdate) { @@ -1430,7 +1426,7 @@ else if (resourceUpdate instanceof D2URIUpdate) { update.setIsStaleModifiedTime( trackServerLatencyHelper(metricsProvider, now, Timestamps.toMillis(uri.getModifiedTime()), subscribedAt, - isIrvEnabled, fetchState, otelMetricsProvider, clientName) + isIrvEnabled, fetchState, xdsClientOtelMetricsProvider, clientName) ); } } @@ -1440,16 +1436,16 @@ private static void trackServerLatencyForUris(Map uriMap, D XdsServerMetricsProvider metricsProvider, long end, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) { - trackServerLatencyForUris(uriMap, update, metricsProvider, end, subscribedAt, isIrvEnabled, fetchState, null, "-"); + trackServerLatencyForUris(uriMap, update, metricsProvider, end, subscribedAt, isIrvEnabled, fetchState, null, NO_VALUE); } private static void trackServerLatencyForUris(Map uriMap, D2URIMapUpdate update, XdsServerMetricsProvider metricsProvider, long end, long subscribedAt, boolean isIrvEnabled, - SubscriberFetchState fetchState, XdsClientOtelMetricsProvider otelMetricsProvider, String clientName) + SubscriberFetchState fetchState, XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) { uriMap.forEach((k, v) -> { boolean isStaleModifiedTime = trackServerLatencyHelper(metricsProvider, end, Timestamps.toMillis(v.getModifiedTime()), subscribedAt, - isIrvEnabled, fetchState, otelMetricsProvider, clientName); + isIrvEnabled, fetchState, xdsClientOtelMetricsProvider, clientName); update.setIsStaleModifiedTime(k, isStaleModifiedTime); } ); @@ -1466,12 +1462,12 @@ private static void trackServerLatencyForUris(Map uriMap, D private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metricsProvider, long end, long modifiedAt, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) { - return trackServerLatencyHelper(metricsProvider, end, modifiedAt, subscribedAt, isIrvEnabled, fetchState, null, "-"); + return trackServerLatencyHelper(metricsProvider, end, modifiedAt, subscribedAt, isIrvEnabled, fetchState, null, NO_VALUE); } private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metricsProvider, long end, long modifiedAt, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState, - XdsClientOtelMetricsProvider otelMetricsProvider, String clientName) + XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) { long start; boolean isStaleModifiedAt; @@ -1489,9 +1485,9 @@ private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metrics metricsProvider.trackLatency(latency); // Record OpenTelemetry latency if provider is available - if (otelMetricsProvider != null) + if (xdsClientOtelMetricsProvider != null) { - otelMetricsProvider.recordServerLatency(clientName, latency); + xdsClientOtelMetricsProvider.recordServerLatency(clientName, latency); } return isStaleModifiedAt; } diff --git a/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java b/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java index 2c7ba2f101..2d014fede4 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java +++ b/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java @@ -69,7 +69,7 @@ public LoadBalancerWithFacilities create(D2ClientConfig config) xdsStreamReadyTimeout, config.subscribeToUriGlobCollection, config._xdsServerMetricsProvider, - config._otelMetricsProvider, + config._xdsClientOtelMetricsProvider, config.xdsInitialResourceVersionsEnabled, config.xdsStreamMaxRetryBackoffSeconds, config.xdsMinimumJavaVersion, diff --git a/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java b/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java index d826a1ec2e..56e2aab7ca 100644 --- a/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java +++ b/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java @@ -8,6 +8,7 @@ import com.google.protobuf.ByteString; import com.google.protobuf.util.Timestamps; import com.linkedin.d2.jmx.XdsClientJmx; +import com.linkedin.d2.jmx.XdsClientOtelMetricsProvider; import com.linkedin.d2.jmx.XdsServerMetricsProvider; import com.linkedin.d2.xds.XdsClient.D2URIMapUpdate; import com.linkedin.d2.xds.XdsClient.ResourceType; @@ -541,7 +542,7 @@ public void testHandleD2URIMapUpdateWithEmptyResponse() { fixture._xdsClientImpl.handleResponse(DISCOVERY_RESPONSE_WITH_EMPTY_URI_MAP_RESPONSE); fixture.verifyAckSent(2); // onData is called only once. Empty response does not trigger onData calls. - verify(fixture._clusterSubscriber).onData(any(), any()); + verify(fixture._clusterSubscriber).onData(any(), any(), any(), any()); verify(fixture._uriMapWildcardSubscriber).onData(any(), any(), any(), any(), any()); } @@ -1262,6 +1263,8 @@ private static final class XdsClientImplFixture { @Mock XdsServerMetricsProvider _serverMetricsProvider; @Mock + XdsClientOtelMetricsProvider _xdsClientOtelMetricsProvider; + @Mock Clock _clock; @Captor @@ -1295,6 +1298,7 @@ private static final class XdsClientImplFixture { doNothing().when(_resourceWatcher).onChanged(any()); doNothing().when(_wildcardResourceWatcher).onChanged(any(), any()); doNothing().when(_serverMetricsProvider).trackLatency(anyLong()); + doNothing().when(_xdsClientOtelMetricsProvider).recordServerLatency(anyString(), anyLong()); for (ResourceSubscriber subscriber : Lists.newArrayList(_nodeSubscriber, _clusterSubscriber, _d2UriSubscriber, _calleesSubscriber)) { @@ -1311,7 +1315,7 @@ private static final class XdsClientImplFixture { _executorService = spy(Executors.newScheduledThreadPool(1)); _xdsClientImpl = spy(new XdsClientImpl(null, mock(ManagedChannel.class), _executorService, 0, useGlobCollections, - _serverMetricsProvider, useIRV)); + _serverMetricsProvider, _xdsClientOtelMetricsProvider, useIRV, null, null, null)); _xdsClientImpl._adsStream = _adsStream; doNothing().when(_xdsClientImpl).startRpcStreamLocal(); From 0df69f4cb23b0ecd94eaa13c7235313df9c8895c Mon Sep 17 00:00:00 2001 From: adiraj_linkedin Date: Mon, 24 Nov 2025 11:59:40 +0530 Subject: [PATCH 3/8] Use NoOpXdsClientOtelMetricsProvider as default to eliminate null handling in XdsClientImpl --- .../com/linkedin/d2/xds/XdsClientImpl.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java index 8b4d56aa68..b05dee51ed 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java +++ b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java @@ -1019,7 +1019,7 @@ void addWatcher(ResourceWatcher watcher) @VisibleForTesting void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider) { - onData(data, metricsProvider, null, NO_VALUE); + onData(data, metricsProvider, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); } @VisibleForTesting @@ -1221,7 +1221,7 @@ void addWatcher(WildcardResourceWatcher watcher) @VisibleForTesting void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider metricsProvider) { - onData(resourceName, data, metricsProvider, null, NO_VALUE); + onData(resourceName, data, metricsProvider, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); } @VisibleForTesting @@ -1383,7 +1383,7 @@ private boolean shouldSubscribeUriGlobCollection(ResourceType type) private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUpdate currentData, XdsServerMetricsProvider metricsProvider, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) { - trackServerLatency(resourceUpdate, currentData, metricsProvider, subscribedAt, isIrvEnabled, fetchState, null, NO_VALUE); + trackServerLatency(resourceUpdate, currentData, metricsProvider, subscribedAt, isIrvEnabled, fetchState, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); } private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUpdate currentData, @@ -1436,7 +1436,7 @@ private static void trackServerLatencyForUris(Map uriMap, D XdsServerMetricsProvider metricsProvider, long end, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) { - trackServerLatencyForUris(uriMap, update, metricsProvider, end, subscribedAt, isIrvEnabled, fetchState, null, NO_VALUE); + trackServerLatencyForUris(uriMap, update, metricsProvider, end, subscribedAt, isIrvEnabled, fetchState, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); } private static void trackServerLatencyForUris(Map uriMap, D2URIMapUpdate update, @@ -1462,7 +1462,7 @@ private static void trackServerLatencyForUris(Map uriMap, D private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metricsProvider, long end, long modifiedAt, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) { - return trackServerLatencyHelper(metricsProvider, end, modifiedAt, subscribedAt, isIrvEnabled, fetchState, null, NO_VALUE); + return trackServerLatencyHelper(metricsProvider, end, modifiedAt, subscribedAt, isIrvEnabled, fetchState, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); } private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metricsProvider, @@ -1484,11 +1484,9 @@ private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metrics long latency = end - start; metricsProvider.trackLatency(latency); - // Record OpenTelemetry latency if provider is available - if (xdsClientOtelMetricsProvider != null) - { - xdsClientOtelMetricsProvider.recordServerLatency(clientName, latency); - } + // Record OpenTelemetry latency + xdsClientOtelMetricsProvider.recordServerLatency(clientName, latency); + return isStaleModifiedAt; } From ea5faa45de350bf626d90fd3beda6bab223c3264 Mon Sep 17 00:00:00 2001 From: Aditya Raj Date: Wed, 26 Nov 2025 15:11:56 +0530 Subject: [PATCH 4/8] Update comments in XdsClientOtelMetricsProvider.java --- .../java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java index 9ed59b0194..5e57d5f5ef 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProvider.java @@ -2,7 +2,6 @@ /** * Interface for OpenTelemetry metrics collection for XDS Client. - * Provides raw event-based metrics that will be processed by OpenTelemetry in the container. */ public interface XdsClientOtelMetricsProvider { @@ -86,4 +85,4 @@ public interface XdsClientOtelMetricsProvider { * @param waitTimeMs the wait time in milliseconds */ void updateActiveInitialWaitTime(String clientName, long waitTimeMs); -} \ No newline at end of file +} From e9155b3cf3f96f47428e53ae43315f001de716b6 Mon Sep 17 00:00:00 2001 From: adiraj_linkedin Date: Fri, 5 Dec 2025 10:31:59 +0530 Subject: [PATCH 5/8] Added tests for XdsClientOtelMetricsProvider --- .../linkedin/d2/balancer/D2ClientConfig.java | 5 + .../linkedin/d2/jmx/D2ClientJmxManager.java | 6 +- .../jmx/NoOpXdsClientOtelMetricsProvider.java | 37 +- .../com/linkedin/d2/jmx/XdsClientJmx.java | 12 +- .../com/linkedin/d2/xds/XdsClientImpl.java | 39 +- .../jmx/XdsClientOtelMetricsProviderTest.java | 332 ++++++++++++++++++ .../linkedin/d2/xds/TestXdsClientImpl.java | 12 +- 7 files changed, 400 insertions(+), 43 deletions(-) create mode 100644 d2/src/test/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProviderTest.java diff --git a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java index cad73b3c45..57025f4608 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java @@ -183,6 +183,11 @@ public class D2ClientConfig public boolean subscribeToUriGlobCollection = false; public XdsServerMetricsProvider _xdsServerMetricsProvider = new NoOpXdsServerMetricsProvider(); + + /** + * Provider for OpenTelemetry metrics collection for XDS client operations. + * Defaults to no-op implementation; can be overridden to enable metric tracking. + */ public XdsClientOtelMetricsProvider _xdsClientOtelMetricsProvider = new NoOpXdsClientOtelMetricsProvider(); public boolean loadBalanceStreamException = false; public boolean xdsInitialResourceVersionsEnabled = false; diff --git a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java index f640f5bd9a..a849a0fb88 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java @@ -309,7 +309,11 @@ public void registerXdsClientJmx(XdsClientJmx xdsClientJmx) // Get the client name from global prefix String clientName = getGlobalPrefix(null); - xdsClientJmx.setClientName(clientName); + if(clientName != null && !clientName.isEmpty()) + { + xdsClientJmx.setClientName(clientName); + } + _jmxManager.registerXdsClientJmxBean(jmxName, xdsClientJmx); } diff --git a/d2/src/main/java/com/linkedin/d2/jmx/NoOpXdsClientOtelMetricsProvider.java b/d2/src/main/java/com/linkedin/d2/jmx/NoOpXdsClientOtelMetricsProvider.java index bc5933dc28..12182a76a4 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/NoOpXdsClientOtelMetricsProvider.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/NoOpXdsClientOtelMetricsProvider.java @@ -1,61 +1,94 @@ package com.linkedin.d2.jmx; /** - * No-Op implementation of XdsClientOtelMetricsProvider. + * No-Op implementation of {@link XdsClientOtelMetricsProvider}. * Used when OpenTelemetry metrics are disabled. */ public class NoOpXdsClientOtelMetricsProvider implements XdsClientOtelMetricsProvider { - + + /** + * {@inheritDoc} + */ @Override public void recordConnectionLost(String clientName) { // No-op } + /** + * {@inheritDoc} + */ @Override public void recordConnectionClosed(String clientName) { // No-op } + /** + * {@inheritDoc} + */ @Override public void recordReconnection(String clientName) { // No-op } + /** + * {@inheritDoc} + */ @Override public void recordRequestSent(String clientName) { // No-op } + /** + * {@inheritDoc} + */ @Override public void recordResponseReceived(String clientName) { // No-op } + /** + * {@inheritDoc} + */ @Override public void recordInitialResourceVersionSent(String clientName, int count) { // No-op } + /** + * {@inheritDoc} + */ @Override public void recordResourceNotFound(String clientName) { // No-op } + /** + * {@inheritDoc} + */ @Override public void recordResourceInvalid(String clientName) { // No-op } + /** + * {@inheritDoc} + */ @Override public void recordServerLatency(String clientName, long latencyMs) { // No-op } + /** + * {@inheritDoc} + */ @Override public void updateConnectionState(String clientName, boolean isConnected) { // No-op } + /** + * {@inheritDoc} + */ @Override public void updateActiveInitialWaitTime(String clientName, long waitTimeMs) { // No-op diff --git a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java index c406e1b152..59038622c1 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java @@ -63,11 +63,21 @@ public XdsClientJmx(XdsServerMetricsProvider xdsServerMetricsProvider, new NoOpXdsClientOtelMetricsProvider() : xdsClientOtelMetricsProvider; } - // Method to set client name (called from D2ClientJmxManager) + /** + * Sets the client name for this XDS client instance. + * Used for identifying metrics associated with this client. + * + * @param clientName the name to identify this XDS client + */ public void setClientName(String clientName) { _clientName = clientName; } + /** + * Gets the client name for this XDS client instance. + * + * @return the client name, or "-" if not set + */ public String getClientName() { return _clientName; } diff --git a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java index b05dee51ed..faa9eb74c9 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java +++ b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java @@ -84,7 +84,6 @@ public class XdsClientImpl extends XdsClient new RateLimitedLogger(_log, TimeUnit.MINUTES.toMillis(1), SystemClock.instance()); public static final long DEFAULT_READY_TIMEOUT_MILLIS = 2000L; public static final Integer DEFAULT_MAX_RETRY_BACKOFF_SECS = 30; // default value for max retry backoff seconds - private static final String NO_VALUE = "-"; /** * The resource subscribers maps the resource type to its subscribers. Note that the {@link ResourceType#D2_URI} @@ -208,7 +207,12 @@ public XdsClientImpl(Node node, this(node, managedChannel, executorService, readyTimeoutMillis, subscribeToUriGlobCollection, serverMetricsProvider, null, irvSupport, maxRetryBackoffSeconds, XdsClientValidator.DEFAULT_MINIMUM_JAVA_VERSION, XdsClientValidator.DEFAULT_ACTION_ON_PRECHECK_FAILURE); } - + + /** + * Constructor for XdsClientImpl with OpenTelemetry metrics support. + * + * @param xdsClientOtelMetricsProvider provider for OpenTelemetry metrics collection, or null for no-op + */ public XdsClientImpl(Node node, ManagedChannel managedChannel, ScheduledExecutorService executorService, @@ -1016,12 +1020,6 @@ void addWatcher(ResourceWatcher watcher) } } - @VisibleForTesting - void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider) - { - onData(data, metricsProvider, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); - } - @VisibleForTesting void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvider, XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) { @@ -1218,12 +1216,6 @@ void addWatcher(WildcardResourceWatcher watcher) } } - @VisibleForTesting - void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider metricsProvider) - { - onData(resourceName, data, metricsProvider, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); - } - @VisibleForTesting void onData(String resourceName, ResourceUpdate data, XdsServerMetricsProvider metricsProvider, XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) @@ -1380,12 +1372,6 @@ private boolean shouldSubscribeUriGlobCollection(ResourceType type) return _subscribeToUriGlobCollection && type == ResourceType.D2_URI_MAP; } - private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUpdate currentData, - XdsServerMetricsProvider metricsProvider, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) - { - trackServerLatency(resourceUpdate, currentData, metricsProvider, subscribedAt, isIrvEnabled, fetchState, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); - } - private static void trackServerLatency(ResourceUpdate resourceUpdate, ResourceUpdate currentData, XdsServerMetricsProvider metricsProvider, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState, XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) @@ -1432,13 +1418,6 @@ else if (resourceUpdate instanceof D2URIUpdate) } } - private static void trackServerLatencyForUris(Map uriMap, D2URIMapUpdate update, - XdsServerMetricsProvider metricsProvider, long end, long subscribedAt, boolean isIrvEnabled, - SubscriberFetchState fetchState) - { - trackServerLatencyForUris(uriMap, update, metricsProvider, end, subscribedAt, isIrvEnabled, fetchState, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); - } - private static void trackServerLatencyForUris(Map uriMap, D2URIMapUpdate update, XdsServerMetricsProvider metricsProvider, long end, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState, XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) @@ -1459,12 +1438,6 @@ private static void trackServerLatencyForUris(Map uriMap, D // -- When IRV is enabled, the caveat above will be fixed. Since the client will never receive resources that it already // received with IRV, except the first fetch, so after skipping the first fetch we can track latency always based // on the resource modified time. - private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metricsProvider, - long end, long modifiedAt, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState) - { - return trackServerLatencyHelper(metricsProvider, end, modifiedAt, subscribedAt, isIrvEnabled, fetchState, new NoOpXdsClientOtelMetricsProvider(), NO_VALUE); - } - private static boolean trackServerLatencyHelper(XdsServerMetricsProvider metricsProvider, long end, long modifiedAt, long subscribedAt, boolean isIrvEnabled, SubscriberFetchState fetchState, XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider, String clientName) diff --git a/d2/src/test/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProviderTest.java b/d2/src/test/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProviderTest.java new file mode 100644 index 0000000000..6b4b8d043e --- /dev/null +++ b/d2/src/test/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProviderTest.java @@ -0,0 +1,332 @@ +package com.linkedin.d2.jmx; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +/** + * Test suite for {@XdsClientOtelMetricsProvider} interface. + * Uses a test implementation to verify that all methods are called with expected parameters. + */ +public class XdsClientOtelMetricsProviderTest { + + private TestMetricsProvider _testProvider; + + @BeforeMethod + public void setUp() { + _testProvider = new TestMetricsProvider(); + } + + @Test + public void testRecordConnectionLost() { + String clientName = "test-client-1"; + _testProvider.recordConnectionLost(clientName); + + assertEquals(_testProvider.getCallCount("recordConnectionLost"), 1); + assertEquals(_testProvider.getLastClientName("recordConnectionLost"), clientName); + } + + @Test + public void testRecordConnectionClosed() { + String clientName = "test-client-2"; + _testProvider.recordConnectionClosed(clientName); + + assertEquals(_testProvider.getCallCount("recordConnectionClosed"), 1); + assertEquals(_testProvider.getLastClientName("recordConnectionClosed"), clientName); + } + + @Test + public void testRecordReconnection() { + String clientName = "test-client-3"; + _testProvider.recordReconnection(clientName); + + assertEquals(_testProvider.getCallCount("recordReconnection"), 1); + assertEquals(_testProvider.getLastClientName("recordReconnection"), clientName); + } + + @Test + public void testRecordRequestSent() { + String clientName = "test-client-4"; + _testProvider.recordRequestSent(clientName); + + assertEquals(_testProvider.getCallCount("recordRequestSent"), 1); + assertEquals(_testProvider.getLastClientName("recordRequestSent"), clientName); + } + + @Test + public void testRecordResponseReceived() { + String clientName = "test-client-5"; + _testProvider.recordResponseReceived(clientName); + + assertEquals(_testProvider.getCallCount("recordResponseReceived"), 1); + assertEquals(_testProvider.getLastClientName("recordResponseReceived"), clientName); + } + + @Test + public void testRecordInitialResourceVersionSent() { + String clientName = "test-client-6"; + int count = 42; + _testProvider.recordInitialResourceVersionSent(clientName, count); + + assertEquals(_testProvider.getCallCount("recordInitialResourceVersionSent"), 1); + assertEquals(_testProvider.getLastClientName("recordInitialResourceVersionSent"), clientName); + assertEquals(_testProvider.getLastCount("recordInitialResourceVersionSent").intValue(), count); + } + + @Test + public void testRecordResourceNotFound() { + String clientName = "test-client-7"; + _testProvider.recordResourceNotFound(clientName); + + assertEquals(_testProvider.getCallCount("recordResourceNotFound"), 1); + assertEquals(_testProvider.getLastClientName("recordResourceNotFound"), clientName); + } + + @Test + public void testRecordResourceInvalid() { + String clientName = "test-client-8"; + _testProvider.recordResourceInvalid(clientName); + + assertEquals(_testProvider.getCallCount("recordResourceInvalid"), 1); + assertEquals(_testProvider.getLastClientName("recordResourceInvalid"), clientName); + } + + @Test + public void testRecordServerLatency() { + String clientName = "test-client-9"; + long latencyMs = 150L; + _testProvider.recordServerLatency(clientName, latencyMs); + + assertEquals(_testProvider.getCallCount("recordServerLatency"), 1); + assertEquals(_testProvider.getLastClientName("recordServerLatency"), clientName); + assertEquals(_testProvider.getLastLatency("recordServerLatency").longValue(), latencyMs); + } + + @Test + public void testUpdateConnectionState() { + String clientName = "test-client-10"; + boolean isConnected = true; + _testProvider.updateConnectionState(clientName, isConnected); + + assertEquals(_testProvider.getCallCount("updateConnectionState"), 1); + assertEquals(_testProvider.getLastClientName("updateConnectionState"), clientName); + assertTrue(_testProvider.getLastConnectionState("updateConnectionState")); + } + + @Test + public void testUpdateActiveInitialWaitTime() { + String clientName = "test-client-11"; + long waitTimeMs = 5000L; + _testProvider.updateActiveInitialWaitTime(clientName, waitTimeMs); + + assertEquals(_testProvider.getCallCount("updateActiveInitialWaitTime"), 1); + assertEquals(_testProvider.getLastClientName("updateActiveInitialWaitTime"), clientName); + assertEquals(_testProvider.getLastWaitTime("updateActiveInitialWaitTime").longValue(), waitTimeMs); + } + + @Test + public void testMultipleCalls() { + String clientName = "test-client-multi"; + + _testProvider.recordConnectionLost(clientName); + _testProvider.recordConnectionLost(clientName); + _testProvider.recordConnectionLost(clientName); + + assertEquals(_testProvider.getCallCount("recordConnectionLost"), 3); + assertEquals(_testProvider.getLastClientName("recordConnectionLost"), clientName); + } + + @Test + public void testDifferentClientNames() { + _testProvider.recordRequestSent("client-A"); + _testProvider.recordRequestSent("client-B"); + _testProvider.recordRequestSent("client-C"); + + assertEquals(_testProvider.getCallCount("recordRequestSent"), 3); + assertEquals(_testProvider.getLastClientName("recordRequestSent"), "client-C"); + } + + @Test + public void testAllMethodsCalled() { + String clientName = "comprehensive-client"; + + _testProvider.recordConnectionLost(clientName); + _testProvider.recordConnectionClosed(clientName); + _testProvider.recordReconnection(clientName); + _testProvider.recordRequestSent(clientName); + _testProvider.recordResponseReceived(clientName); + _testProvider.recordInitialResourceVersionSent(clientName, 10); + _testProvider.recordResourceNotFound(clientName); + _testProvider.recordResourceInvalid(clientName); + _testProvider.recordServerLatency(clientName, 200L); + _testProvider.updateConnectionState(clientName, true); + _testProvider.updateActiveInitialWaitTime(clientName, 3000L); + + // Verify all methods were called exactly once + assertEquals(_testProvider.getCallCount("recordConnectionLost"), 1); + assertEquals(_testProvider.getCallCount("recordConnectionClosed"), 1); + assertEquals(_testProvider.getCallCount("recordReconnection"), 1); + assertEquals(_testProvider.getCallCount("recordRequestSent"), 1); + assertEquals(_testProvider.getCallCount("recordResponseReceived"), 1); + assertEquals(_testProvider.getCallCount("recordInitialResourceVersionSent"), 1); + assertEquals(_testProvider.getCallCount("recordResourceNotFound"), 1); + assertEquals(_testProvider.getCallCount("recordResourceInvalid"), 1); + assertEquals(_testProvider.getCallCount("recordServerLatency"), 1); + assertEquals(_testProvider.getCallCount("updateConnectionState"), 1); + assertEquals(_testProvider.getCallCount("updateActiveInitialWaitTime"), 1); + } + + /** + * Test implementation of XdsClientOtelMetricsProvider that tracks method calls + * and their parameters for verification purposes. + */ + private static class TestMetricsProvider implements XdsClientOtelMetricsProvider { + + private final List _calls = new ArrayList<>(); + + @Override + public void recordConnectionLost(String clientName) { + _calls.add(new MetricsInvocation("recordConnectionLost", clientName)); + } + + @Override + public void recordConnectionClosed(String clientName) { + _calls.add(new MetricsInvocation("recordConnectionClosed", clientName)); + } + + @Override + public void recordReconnection(String clientName) { + _calls.add(new MetricsInvocation("recordReconnection", clientName)); + } + + @Override + public void recordRequestSent(String clientName) { + _calls.add(new MetricsInvocation("recordRequestSent", clientName)); + } + + @Override + public void recordResponseReceived(String clientName) { + _calls.add(new MetricsInvocation("recordResponseReceived", clientName)); + } + + @Override + public void recordInitialResourceVersionSent(String clientName, int count) { + _calls.add(new MetricsInvocation("recordInitialResourceVersionSent", clientName, count)); + } + + @Override + public void recordResourceNotFound(String clientName) { + _calls.add(new MetricsInvocation("recordResourceNotFound", clientName)); + } + + @Override + public void recordResourceInvalid(String clientName) { + _calls.add(new MetricsInvocation("recordResourceInvalid", clientName)); + } + + @Override + public void recordServerLatency(String clientName, long latencyMs) { + _calls.add(new MetricsInvocation("recordServerLatency", clientName, latencyMs)); + } + + @Override + public void updateConnectionState(String clientName, boolean isConnected) { + _calls.add(new MetricsInvocation("updateConnectionState", clientName, isConnected)); + } + + @Override + public void updateActiveInitialWaitTime(String clientName, long waitTimeMs) { + _calls.add(new MetricsInvocation("updateActiveInitialWaitTime", clientName, waitTimeMs)); + } + + // Helper methods for verification + + public int getCallCount(String methodName) { + return (int) _calls.stream() + .filter(call -> call.methodName.equals(methodName)) + .count(); + } + + public String getLastClientName(String methodName) { + return _calls.stream() + .filter(call -> call.methodName.equals(methodName)) + .reduce((first, second) -> second) + .map(call -> call.clientName) + .orElse(null); + } + + public Integer getLastCount(String methodName) { + return _calls.stream() + .filter(call -> call.methodName.equals(methodName)) + .reduce((first, second) -> second) + .map(call -> call.count) + .orElse(null); + } + + public Long getLastLatency(String methodName) { + return _calls.stream() + .filter(call -> call.methodName.equals(methodName)) + .reduce((first, second) -> second) + .map(call -> call.latencyMs) + .orElse(null); + } + + public Boolean getLastConnectionState(String methodName) { + return _calls.stream() + .filter(call -> call.methodName.equals(methodName)) + .reduce((first, second) -> second) + .map(call -> call.isConnected) + .orElse(null); + } + + public Long getLastWaitTime(String methodName) { + return _calls.stream() + .filter(call -> call.methodName.equals(methodName)) + .reduce((first, second) -> second) + .map(call -> call.waitTimeMs) + .orElse(null); + } + + /** + * Inner class to represent a metrics invocation with its parameters + */ + private static class MetricsInvocation { + String methodName; + String clientName; + Integer count; + Long latencyMs; + Boolean isConnected; + Long waitTimeMs; + + MetricsInvocation(String methodName, String clientName) { + this.methodName = methodName; + this.clientName = clientName; + } + + MetricsInvocation(String methodName, String clientName, int count) { + this(methodName, clientName); + this.count = count; + } + + MetricsInvocation(String methodName, String clientName, long value) { + this(methodName, clientName); + // Determine if it's latency or wait time based on method name + if (methodName.equals("recordServerLatency")) { + this.latencyMs = value; + } else if (methodName.equals("updateActiveInitialWaitTime")) { + this.waitTimeMs = value; + } + } + + MetricsInvocation(String methodName, String clientName, boolean isConnected) { + this(methodName, clientName); + this.isConnected = isConnected; + } + } + } +} diff --git a/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java b/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java index 56e2aab7ca..5c0276f4c2 100644 --- a/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java +++ b/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java @@ -337,9 +337,9 @@ public void testHandleD2NodeUpdateWithEmptyResponse() { fixture.watchAllResourceAndWatcherTypes(); fixture._xdsClientImpl.handleResponse(DISCOVERY_RESPONSE_WITH_EMPTY_NODE_RESPONSE); fixture.verifyAckSent(1); - verify(fixture._d2UriSubscriber, times(0)).onData(any(), any()); - verify(fixture._clusterSubscriber, times(0)).onData(any(), any()); - verify(fixture._uriMapWildcardSubscriber, times(0)).onData(any(), any(), any()); + verify(fixture._d2UriSubscriber, times(0)).onData(any(), any(), any(), any()); + verify(fixture._clusterSubscriber, times(0)).onData(any(), any(), any(), any()); + verify(fixture._uriMapWildcardSubscriber, times(0)).onData(any(), any(), any(), any(), any()); } @DataProvider(name = "badNodeUpdateTestCases") @@ -441,7 +441,7 @@ public void testHandleD2ClusterOrServiceNameEmptyResponse() { fixture.watchAllResourceAndWatcherTypes(); fixture._xdsClientImpl.handleResponse(RESPONSE_WITH_EMPTY_NAMES); fixture.verifyAckSent(1); - verify(fixture._nameWildcardSubscriber, times(0)).onData(any(), any(), any()); + verify(fixture._nameWildcardSubscriber, times(0)).onData(any(), any(), any(), any(), any()); } @Test @@ -1122,8 +1122,8 @@ public void testHandleD2CalleesUpdateWithEmptyResponse() { fixture.watchAllResourceAndWatcherTypes(); fixture._xdsClientImpl.handleResponse(DISCOVERY_RESPONSE_WITH_EMPTY_CALLEES_RESPONSE); fixture.verifyAckSent(1); - verify(fixture._calleesSubscriber, times(0)).onData(any(), any()); - verify(fixture._calleesWildcardSubscriber, times(0)).onData(any(), any(), any()); + verify(fixture._calleesSubscriber, times(0)).onData(any(), any(), any(), any()); + verify(fixture._calleesWildcardSubscriber, times(0)).onData(any(), any(), any(), any(), any()); } @Test From 5cd8559aa375c6625ff7421b312fe6ed7cb38e82 Mon Sep 17 00:00:00 2001 From: adiraj_linkedin Date: Tue, 9 Dec 2025 18:47:50 +0530 Subject: [PATCH 6/8] Resolved PR comments --- .../linkedin/d2/balancer/D2ClientBuilder.java | 4 +- .../linkedin/d2/balancer/D2ClientConfig.java | 4 +- .../linkedin/d2/jmx/D2ClientJmxManager.java | 5 +- .../XdsLoadBalancerWithFacilitiesFactory.java | 2 +- .../jmx/XdsClientOtelMetricsProviderTest.java | 102 ++++++++---------- ...tXdsLoadBalancerWithFacilitiesFactory.java | 83 ++++++++++++++ 6 files changed, 135 insertions(+), 65 deletions(-) create mode 100644 d2/src/test/java/com/linkedin/d2/xds/balancer/TestXdsLoadBalancerWithFacilitiesFactory.java diff --git a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java index c330297a20..39dbf3ef5b 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientBuilder.java @@ -238,7 +238,7 @@ public D2Client build() _config.xdsChannelLoadBalancingPolicyConfig, _config.subscribeToUriGlobCollection, _config._xdsServerMetricsProvider, - _config._xdsClientOtelMetricsProvider, + _config.xdsClientOtelMetricsProvider, _config.loadBalanceStreamException, _config.xdsInitialResourceVersionsEnabled, _config.disableDetectLiRawD2Client, @@ -859,7 +859,7 @@ public D2ClientBuilder setXdsServerMetricsProvider(XdsServerMetricsProvider xdsS } public D2ClientBuilder setXdsClientOtelMetricsProvider(XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider) { - _config._xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider; + _config.xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider; return this; } diff --git a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java index 57025f4608..5072b6dd9e 100644 --- a/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java +++ b/d2/src/main/java/com/linkedin/d2/balancer/D2ClientConfig.java @@ -188,7 +188,7 @@ public class D2ClientConfig * Provider for OpenTelemetry metrics collection for XDS client operations. * Defaults to no-op implementation; can be overridden to enable metric tracking. */ - public XdsClientOtelMetricsProvider _xdsClientOtelMetricsProvider = new NoOpXdsClientOtelMetricsProvider(); + public XdsClientOtelMetricsProvider xdsClientOtelMetricsProvider = new NoOpXdsClientOtelMetricsProvider(); public boolean loadBalanceStreamException = false; public boolean xdsInitialResourceVersionsEnabled = false; public Integer xdsStreamMaxRetryBackoffSeconds = null; @@ -514,7 +514,7 @@ public D2ClientConfig() this.xdsChannelKeepAliveTimeMins = xdsChannelKeepAliveTimeMins; this.subscribeToUriGlobCollection = subscribeToUriGlobCollection; this._xdsServerMetricsProvider = xdsServerMetricsProvider; - this._xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider; + this.xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider; this.loadBalanceStreamException = loadBalanceStreamException; this.xdsInitialResourceVersionsEnabled = xdsInitialResourceVersionsEnabled; this.disableDetectLiRawD2Client = disableDetectLiRawD2Client; diff --git a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java index a849a0fb88..a4d2e563d0 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java @@ -305,10 +305,11 @@ public void registerXdsClientJmx(XdsClientJmx xdsClientJmx) { _log.warn("Setting XdsClientJmx for Non-XDS source type: {}", _discoverySourceType); } - final String jmxName = String.format("%s-XdsClientJmx", getGlobalPrefix(null)); - // Get the client name from global prefix String clientName = getGlobalPrefix(null); + + final String jmxName = String.format("%s-XdsClientJmx", clientName); + if(clientName != null && !clientName.isEmpty()) { xdsClientJmx.setClientName(clientName); diff --git a/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java b/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java index 2d014fede4..cbe2c2d2a8 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java +++ b/d2/src/main/java/com/linkedin/d2/xds/balancer/XdsLoadBalancerWithFacilitiesFactory.java @@ -69,7 +69,7 @@ public LoadBalancerWithFacilities create(D2ClientConfig config) xdsStreamReadyTimeout, config.subscribeToUriGlobCollection, config._xdsServerMetricsProvider, - config._xdsClientOtelMetricsProvider, + config.xdsClientOtelMetricsProvider, config.xdsInitialResourceVersionsEnabled, config.xdsStreamMaxRetryBackoffSeconds, config.xdsMinimumJavaVersion, diff --git a/d2/src/test/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProviderTest.java b/d2/src/test/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProviderTest.java index 6b4b8d043e..4f7cfbaf19 100644 --- a/d2/src/test/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProviderTest.java +++ b/d2/src/test/java/com/linkedin/d2/jmx/XdsClientOtelMetricsProviderTest.java @@ -1,6 +1,7 @@ package com.linkedin.d2.jmx; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.util.ArrayList; @@ -22,49 +23,52 @@ public void setUp() { _testProvider = new TestMetricsProvider(); } - @Test - public void testRecordConnectionLost() { - String clientName = "test-client-1"; - _testProvider.recordConnectionLost(clientName); - - assertEquals(_testProvider.getCallCount("recordConnectionLost"), 1); - assertEquals(_testProvider.getLastClientName("recordConnectionLost"), clientName); - } - - @Test - public void testRecordConnectionClosed() { - String clientName = "test-client-2"; - _testProvider.recordConnectionClosed(clientName); - - assertEquals(_testProvider.getCallCount("recordConnectionClosed"), 1); - assertEquals(_testProvider.getLastClientName("recordConnectionClosed"), clientName); - } - - @Test - public void testRecordReconnection() { - String clientName = "test-client-3"; - _testProvider.recordReconnection(clientName); - - assertEquals(_testProvider.getCallCount("recordReconnection"), 1); - assertEquals(_testProvider.getLastClientName("recordReconnection"), clientName); - } - - @Test - public void testRecordRequestSent() { - String clientName = "test-client-4"; - _testProvider.recordRequestSent(clientName); - - assertEquals(_testProvider.getCallCount("recordRequestSent"), 1); - assertEquals(_testProvider.getLastClientName("recordRequestSent"), clientName); + @DataProvider(name = "simpleMethodProvider") + public Object[][] simpleMethodProvider() { + return new Object[][] { + {"recordConnectionLost"}, + {"recordConnectionClosed"}, + {"recordReconnection"}, + {"recordRequestSent"}, + {"recordResponseReceived"}, + {"recordResourceNotFound"}, + {"recordResourceInvalid"} + }; } - @Test - public void testRecordResponseReceived() { - String clientName = "test-client-5"; - _testProvider.recordResponseReceived(clientName); + @Test(dataProvider = "simpleMethodProvider") + public void testSimpleRecordMethods(String methodName) { + String clientName = "test-client-" + methodName; + + // Call the appropriate method based on methodName + switch (methodName) { + case "recordConnectionLost": + _testProvider.recordConnectionLost(clientName); + break; + case "recordConnectionClosed": + _testProvider.recordConnectionClosed(clientName); + break; + case "recordReconnection": + _testProvider.recordReconnection(clientName); + break; + case "recordRequestSent": + _testProvider.recordRequestSent(clientName); + break; + case "recordResponseReceived": + _testProvider.recordResponseReceived(clientName); + break; + case "recordResourceNotFound": + _testProvider.recordResourceNotFound(clientName); + break; + case "recordResourceInvalid": + _testProvider.recordResourceInvalid(clientName); + break; + default: + throw new IllegalArgumentException("Unknown method: " + methodName); + } - assertEquals(_testProvider.getCallCount("recordResponseReceived"), 1); - assertEquals(_testProvider.getLastClientName("recordResponseReceived"), clientName); + assertEquals(_testProvider.getCallCount(methodName), 1); + assertEquals(_testProvider.getLastClientName(methodName), clientName); } @Test @@ -78,24 +82,6 @@ public void testRecordInitialResourceVersionSent() { assertEquals(_testProvider.getLastCount("recordInitialResourceVersionSent").intValue(), count); } - @Test - public void testRecordResourceNotFound() { - String clientName = "test-client-7"; - _testProvider.recordResourceNotFound(clientName); - - assertEquals(_testProvider.getCallCount("recordResourceNotFound"), 1); - assertEquals(_testProvider.getLastClientName("recordResourceNotFound"), clientName); - } - - @Test - public void testRecordResourceInvalid() { - String clientName = "test-client-8"; - _testProvider.recordResourceInvalid(clientName); - - assertEquals(_testProvider.getCallCount("recordResourceInvalid"), 1); - assertEquals(_testProvider.getLastClientName("recordResourceInvalid"), clientName); - } - @Test public void testRecordServerLatency() { String clientName = "test-client-9"; diff --git a/d2/src/test/java/com/linkedin/d2/xds/balancer/TestXdsLoadBalancerWithFacilitiesFactory.java b/d2/src/test/java/com/linkedin/d2/xds/balancer/TestXdsLoadBalancerWithFacilitiesFactory.java new file mode 100644 index 0000000000..c3e327f59a --- /dev/null +++ b/d2/src/test/java/com/linkedin/d2/xds/balancer/TestXdsLoadBalancerWithFacilitiesFactory.java @@ -0,0 +1,83 @@ +/* + Copyright (c) 2023 LinkedIn Corp. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package com.linkedin.d2.xds.balancer; + +import com.linkedin.d2.balancer.D2ClientConfig; +import com.linkedin.d2.jmx.NoOpXdsClientOtelMetricsProvider; +import com.linkedin.d2.jmx.XdsClientOtelMetricsProvider; +import org.testng.annotations.Test; + +import static org.testng.Assert.*; + +/** + * Minimal test for {@link XdsLoadBalancerWithFacilitiesFactory}. + * Verifies that config.xdsClientOtelMetricsProvider can be configured. + */ +public class TestXdsLoadBalancerWithFacilitiesFactory { + + @Test + public void testFactoryIsIndisOnly() { + XdsLoadBalancerWithFacilitiesFactory factory = new XdsLoadBalancerWithFacilitiesFactory(); + assertTrue(factory.isIndisOnly()); + } + + @Test + public void testConfigWithDefaultMetricsProvider() { + // Verify the config field that the factory uses exists and has a default + D2ClientConfig config = new D2ClientConfig(); + assertNotNull(config.xdsClientOtelMetricsProvider); + assertTrue(config.xdsClientOtelMetricsProvider instanceof NoOpXdsClientOtelMetricsProvider); + } + + @Test + public void testConfigWithCustomMetricsProvider() { + // Verify the config field that the factory uses can be set to a custom provider + D2ClientConfig config = new D2ClientConfig(); + XdsClientOtelMetricsProvider customProvider = new TestMetricsProvider(); + config.xdsClientOtelMetricsProvider = customProvider; + + assertSame(config.xdsClientOtelMetricsProvider, customProvider); + } + + /** + * Minimal test metrics provider implementation. + */ + private static class TestMetricsProvider implements XdsClientOtelMetricsProvider { + @Override + public void recordConnectionLost(String clientName) {} + @Override + public void recordConnectionClosed(String clientName) {} + @Override + public void recordReconnection(String clientName) {} + @Override + public void recordRequestSent(String clientName) {} + @Override + public void recordResponseReceived(String clientName) {} + @Override + public void recordInitialResourceVersionSent(String clientName, int count) {} + @Override + public void recordResourceNotFound(String clientName) {} + @Override + public void recordResourceInvalid(String clientName) {} + @Override + public void recordServerLatency(String clientName, long latencyMs) {} + @Override + public void updateConnectionState(String clientName, boolean isConnected) {} + @Override + public void updateActiveInitialWaitTime(String clientName, long waitTimeMs) {} + } +} \ No newline at end of file From f710f7e5a833f0955771f81031fe161463a16081 Mon Sep 17 00:00:00 2001 From: Aditya Raj Date: Wed, 17 Dec 2025 16:37:14 +0530 Subject: [PATCH 7/8] Resolved PR comments --- .../linkedin/d2/jmx/D2ClientJmxManager.java | 6 +--- .../com/linkedin/d2/jmx/D2JmxConstants.java | 34 +++++++++++++++++++ .../com/linkedin/d2/jmx/XdsClientJmx.java | 31 +++++++++-------- .../com/linkedin/d2/xds/XdsClientImpl.java | 4 +-- .../linkedin/d2/xds/TestXdsClientImpl.java | 4 +++ ...tXdsLoadBalancerWithFacilitiesFactory.java | 16 --------- 6 files changed, 57 insertions(+), 38 deletions(-) create mode 100644 d2/src/main/java/com/linkedin/d2/jmx/D2JmxConstants.java diff --git a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java index a4d2e563d0..1b4a678d48 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/D2ClientJmxManager.java @@ -307,15 +307,11 @@ public void registerXdsClientJmx(XdsClientJmx xdsClientJmx) } // Get the client name from global prefix String clientName = getGlobalPrefix(null); - - final String jmxName = String.format("%s-XdsClientJmx", clientName); - if(clientName != null && !clientName.isEmpty()) { xdsClientJmx.setClientName(clientName); } - - + final String jmxName = String.format("%s-XdsClientJmx", clientName); _jmxManager.registerXdsClientJmxBean(jmxName, xdsClientJmx); } diff --git a/d2/src/main/java/com/linkedin/d2/jmx/D2JmxConstants.java b/d2/src/main/java/com/linkedin/d2/jmx/D2JmxConstants.java new file mode 100644 index 0000000000..c761cf36be --- /dev/null +++ b/d2/src/main/java/com/linkedin/d2/jmx/D2JmxConstants.java @@ -0,0 +1,34 @@ +/* + Copyright (c) 2025 LinkedIn Corp. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package com.linkedin.d2.jmx; + +/** + * Common constants used across D2 JMX and metrics components. + */ +public final class D2JmxConstants +{ + /** + * Default client name used when a specific client name is not set. + * Used for identifying metrics associated with unnamed clients. + */ + public static final String NO_VALUE = "-"; + + private D2JmxConstants() + { + // Utility class, prevent instantiation + } +} diff --git a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java index 59038622c1..5de17965f6 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java @@ -20,8 +20,11 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; +import static com.linkedin.d2.jmx.D2JmxConstants.NO_VALUE; + public class XdsClientJmx implements XdsClientJmxMBean { @@ -38,9 +41,7 @@ public class XdsClientJmx implements XdsClientJmxMBean private final AtomicInteger _resourceInvalidCount = new AtomicInteger(); private final XdsServerMetricsProvider _xdsServerMetricsProvider; private final XdsClientOtelMetricsProvider _xdsClientOtelMetricsProvider; - - private String _clientName = "-"; - + private final AtomicReference _clientName = new AtomicReference<>(NO_VALUE); @Nullable private XdsClientImpl _xdsClient = null; @Deprecated @@ -70,7 +71,7 @@ public XdsClientJmx(XdsServerMetricsProvider xdsServerMetricsProvider, * @param clientName the name to identify this XDS client */ public void setClientName(String clientName) { - _clientName = clientName; + _clientName.compareAndSet(NO_VALUE, clientName); } /** @@ -79,7 +80,7 @@ public void setClientName(String clientName) { * @return the client name, or "-" if not set */ public String getClientName() { - return _clientName; + return _clientName.get(); } public void setXdsClient(XdsClientImpl xdsClient) @@ -181,7 +182,7 @@ public long getActiveInitialWaitTimeMillis() if (_xdsClient != null) { waitTime = _xdsClient.getActiveInitialWaitTimeMillis(); - _xdsClientOtelMetricsProvider.updateActiveInitialWaitTime(_clientName, waitTime); + _xdsClientOtelMetricsProvider.updateActiveInitialWaitTime(getClientName(), waitTime); } return waitTime; } @@ -189,54 +190,54 @@ public long getActiveInitialWaitTimeMillis() public void incrementConnectionLostCount() { _connectionLostCount.incrementAndGet(); - _xdsClientOtelMetricsProvider.recordConnectionLost(_clientName); + _xdsClientOtelMetricsProvider.recordConnectionLost(getClientName()); } public void incrementConnectionClosedCount() { _connectionClosedCount.incrementAndGet(); - _xdsClientOtelMetricsProvider.recordConnectionClosed(_clientName); + _xdsClientOtelMetricsProvider.recordConnectionClosed(getClientName()); } public void incrementReconnectionCount() { _reconnectionCount.incrementAndGet(); - _xdsClientOtelMetricsProvider.recordReconnection(_clientName); + _xdsClientOtelMetricsProvider.recordReconnection(getClientName()); } public void incrementRequestSentCount() { _resquestSentCount.incrementAndGet(); - _xdsClientOtelMetricsProvider.recordRequestSent(_clientName); + _xdsClientOtelMetricsProvider.recordRequestSent(getClientName()); } public void addToIrvSentCount(int delta) { _irvSentCount.addAndGet(delta); - _xdsClientOtelMetricsProvider.recordInitialResourceVersionSent(_clientName, delta); + _xdsClientOtelMetricsProvider.recordInitialResourceVersionSent(getClientName(), delta); } public void incrementResponseReceivedCount() { _responseReceivedCount.incrementAndGet(); - _xdsClientOtelMetricsProvider.recordResponseReceived(_clientName); + _xdsClientOtelMetricsProvider.recordResponseReceived(getClientName()); } public void setIsConnected(boolean connected) { _isConnected.getAndSet(connected); - _xdsClientOtelMetricsProvider.updateConnectionState(_clientName, connected); + _xdsClientOtelMetricsProvider.updateConnectionState(getClientName(), connected); } public void incrementResourceNotFoundCount() { _resourceNotFoundCount.incrementAndGet(); - _xdsClientOtelMetricsProvider.recordResourceNotFound(_clientName); + _xdsClientOtelMetricsProvider.recordResourceNotFound(getClientName()); } public void incrementResourceInvalidCount() { _resourceInvalidCount.incrementAndGet(); - _xdsClientOtelMetricsProvider.recordResourceInvalid(_clientName); + _xdsClientOtelMetricsProvider.recordResourceInvalid(getClientName()); } } diff --git a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java index faa9eb74c9..795f744c11 100644 --- a/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java +++ b/d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java @@ -236,9 +236,9 @@ public XdsClientImpl(Node node, _log.info("Glob collection support enabled"); } - _xdsClientJmx = new XdsClientJmx(serverMetricsProvider, xdsClientOtelMetricsProvider); - _serverMetricsProvider = serverMetricsProvider == null ? new NoOpXdsServerMetricsProvider() : serverMetricsProvider; _xdsClientOtelMetricsProvider = xdsClientOtelMetricsProvider == null ? new NoOpXdsClientOtelMetricsProvider() : xdsClientOtelMetricsProvider; + _serverMetricsProvider = serverMetricsProvider == null ? new NoOpXdsServerMetricsProvider() : serverMetricsProvider; + _xdsClientJmx = new XdsClientJmx(serverMetricsProvider, _xdsClientOtelMetricsProvider); _initialResourceVersionsEnabled = irvSupport; if (_initialResourceVersionsEnabled) { diff --git a/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java b/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java index 5c0276f4c2..3c9c0673ab 100644 --- a/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java +++ b/d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java @@ -433,6 +433,7 @@ public void testHandleD2ClusterOrServiceNameResponse() { Assert.assertEquals(fixture._nameWildcardSubscriber.getData(SERVICE_RESOURCE_NAME), SERVICE_NAME_DATA_UPDATE); Assert.assertEquals(fixture._nameWildcardSubscriber.getData(SERVICE_RESOURCE_NAME_2), SERVICE_NAME_DATA_UPDATE_2); verifyZeroInteractions(fixture._serverMetricsProvider); // D2ClusterOrServiceName response does not track latency + verifyZeroInteractions(fixture._xdsClientOtelMetricsProvider); } @Test @@ -575,6 +576,7 @@ public void testHandleD2URIMapUpdateWithBadData(boolean toWatchIndividual, boole // bad data will not overwrite the original valid data Assert.assertEquals(fixture._clusterSubscriber.getData(), D2_URI_MAP_UPDATE_WITH_DATA1); verifyZeroInteractions(fixture._serverMetricsProvider); + verifyZeroInteractions(fixture._xdsClientOtelMetricsProvider); D2URIMapUpdate actualData = (D2URIMapUpdate) fixture._clusterSubscriber.getData(); Assert.assertFalse(actualData.isGlobCollectionEnabled()); Assert.assertTrue(actualData.getUpdatedUrisName().isEmpty()); @@ -686,6 +688,7 @@ public void testHandleD2URICollectionUpdateWithBadData(boolean toWatchIndividual verify(fixture._wildcardResourceWatcher, times(toWatchWildcard ? 1 : 0)).onChanged(any(), eq(D2_URI_MAP.emptyData())); verifyZeroInteractions(fixture._serverMetricsProvider); + verifyZeroInteractions(fixture._xdsClientOtelMetricsProvider); // current data is not null, bad data will not overwrite the original valid data and watchers won't be notified. fixture._clusterSubscriber.setData(D2_URI_MAP_GLOB_COLLECTION_UPDATE_WITH_DATA1); @@ -696,6 +699,7 @@ public void testHandleD2URICollectionUpdateWithBadData(boolean toWatchIndividual verify(fixture._wildcardResourceWatcher, times(0)).onChanged(any(), eq(D2_URI_MAP_GLOB_COLLECTION_UPDATE_WITH_DATA1)); verifyZeroInteractions(fixture._serverMetricsProvider); + verifyZeroInteractions(fixture._xdsClientOtelMetricsProvider); Assert.assertEquals(fixture._clusterSubscriber.getData(), D2_URI_MAP_GLOB_COLLECTION_UPDATE_WITH_DATA1); // Verify that bad data doesn't affect the updated and removed URIs D2URIMapUpdate actualData = (D2URIMapUpdate) fixture._clusterSubscriber.getData(); diff --git a/d2/src/test/java/com/linkedin/d2/xds/balancer/TestXdsLoadBalancerWithFacilitiesFactory.java b/d2/src/test/java/com/linkedin/d2/xds/balancer/TestXdsLoadBalancerWithFacilitiesFactory.java index c3e327f59a..d583704571 100644 --- a/d2/src/test/java/com/linkedin/d2/xds/balancer/TestXdsLoadBalancerWithFacilitiesFactory.java +++ b/d2/src/test/java/com/linkedin/d2/xds/balancer/TestXdsLoadBalancerWithFacilitiesFactory.java @@ -1,19 +1,3 @@ -/* - Copyright (c) 2023 LinkedIn Corp. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - package com.linkedin.d2.xds.balancer; import com.linkedin.d2.balancer.D2ClientConfig; From 4effe1ec4553d4e87b3a6cb0a7023a92b51faeb5 Mon Sep 17 00:00:00 2001 From: Aditya Raj Date: Thu, 18 Dec 2025 20:07:40 +0530 Subject: [PATCH 8/8] revert d2jmxconstant changes. --- .../com/linkedin/d2/jmx/D2JmxConstants.java | 34 ------------------- .../com/linkedin/d2/jmx/XdsClientJmx.java | 4 +-- 2 files changed, 1 insertion(+), 37 deletions(-) delete mode 100644 d2/src/main/java/com/linkedin/d2/jmx/D2JmxConstants.java diff --git a/d2/src/main/java/com/linkedin/d2/jmx/D2JmxConstants.java b/d2/src/main/java/com/linkedin/d2/jmx/D2JmxConstants.java deleted file mode 100644 index c761cf36be..0000000000 --- a/d2/src/main/java/com/linkedin/d2/jmx/D2JmxConstants.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - Copyright (c) 2025 LinkedIn Corp. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package com.linkedin.d2.jmx; - -/** - * Common constants used across D2 JMX and metrics components. - */ -public final class D2JmxConstants -{ - /** - * Default client name used when a specific client name is not set. - * Used for identifying metrics associated with unnamed clients. - */ - public static final String NO_VALUE = "-"; - - private D2JmxConstants() - { - // Utility class, prevent instantiation - } -} diff --git a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java index 5de17965f6..fece131dcb 100644 --- a/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java +++ b/d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java @@ -23,11 +23,9 @@ import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; -import static com.linkedin.d2.jmx.D2JmxConstants.NO_VALUE; - - public class XdsClientJmx implements XdsClientJmxMBean { + private final String NO_VALUE = "-"; private final AtomicInteger _connectionLostCount = new AtomicInteger(); private final AtomicInteger _connectionClosedCount = new AtomicInteger();