Skip to content

Commit e6f7270

Browse files
jasonk000jasonk000
committed
Simplify new VIP address lookup metrics
drop the VIP tag which allows reuse of existing metric code.
1 parent 845962d commit e6f7270

8 files changed

Lines changed: 50 additions & 196 deletions

File tree

‎eureka-client/src/main/java/com/netflix/discovery/DefaultEurekaClientConfig.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.util.List;
2424

2525
import com.netflix.appinfo.EurekaAccept;
26-
import com.netflix.config.DynamicBooleanProperty;
2726
import com.netflix.config.DynamicPropertyFactory;
2827
import com.netflix.config.DynamicStringProperty;
2928
import com.netflix.discovery.internal.util.Archaius1Utils;
@@ -70,9 +69,6 @@ public class DefaultEurekaClientConfig implements EurekaClientConfig {
7069
private final DynamicPropertyFactory configInstance;
7170
private final EurekaTransportConfig transportConfig;
7271

73-
// Cached for hot path performance - see EurekaServerConfig for pattern
74-
private final DynamicBooleanProperty enableVipAddressLookupMetrics;
75-
7672
public DefaultEurekaClientConfig() {
7773
this(CommonConstants.DEFAULT_CONFIG_NAMESPACE);
7874
}
@@ -84,9 +80,6 @@ public DefaultEurekaClientConfig(String namespace) {
8480

8581
this.configInstance = Archaius1Utils.initConfig(CommonConstants.CONFIG_FILE_NAME);
8682
this.transportConfig = new DefaultEurekaTransportConfig(namespace, configInstance);
87-
88-
this.enableVipAddressLookupMetrics = configInstance.getBooleanProperty(
89-
this.namespace + SHOULD_ENABLE_VIP_ADDRESS_LOOKUP_METRICS_KEY, false);
9083
}
9184

9285
/*
@@ -335,11 +328,6 @@ public boolean shouldLogDeltaDiff() {
335328
namespace + SHOULD_LOG_DELTA_DIFF_KEY, false).get();
336329
}
337330

338-
@Override
339-
public boolean shouldEnableVipAddressLookupMetrics() {
340-
return enableVipAddressLookupMetrics.get();
341-
}
342-
343331
/*
344332
* (non-Javadoc)
345333
*

‎eureka-client/src/main/java/com/netflix/discovery/DiscoveryClient.java‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,13 @@
1616

1717
package com.netflix.discovery;
1818

19-
import static com.netflix.discovery.EurekaClientNames.METRIC_LOOKUP_PREFIX;
2019
import static com.netflix.discovery.EurekaClientNames.METRIC_REGISTRATION_PREFIX;
2120
import static com.netflix.discovery.EurekaClientNames.METRIC_REGISTRY_PREFIX;
2221
import static com.netflix.discovery.util.SpectatorUtil.monitoredNumber;
2322
import static com.netflix.discovery.util.SpectatorUtil.monitoredValue;
2423

2524
import com.netflix.discovery.util.SpectatorUtil;
26-
import com.netflix.spectator.api.BasicTag;
2725
import com.netflix.spectator.api.Counter;
28-
import com.netflix.spectator.api.Spectator;
29-
import com.netflix.spectator.api.Tag;
3026
import com.netflix.spectator.api.Timer;
3127
import java.util.ArrayList;
3228
import java.util.Collection;
@@ -133,6 +129,16 @@ public class DiscoveryClient implements EurekaClient {
133129
private final Timer FETCH_REGISTRY_TIMER = SpectatorUtil.timer(PREFIX + "FetchRegistry", DiscoveryClient.class);
134130
private final Counter REREGISTER_COUNTER = SpectatorUtil.counter(PREFIX + "Reregister", DiscoveryClient.class);
135131

132+
// Lookup counters
133+
private final Counter LOOKUP_GET_APPLICATION = SpectatorUtil.counter(PREFIX + "Lookup", "getApplication", DiscoveryClient.class);
134+
private final Counter LOOKUP_GET_APPLICATIONS = SpectatorUtil.counter(PREFIX + "Lookup", "getApplications", DiscoveryClient.class);
135+
private final Counter LOOKUP_GET_APPLICATIONS_FOR_A_REGION = SpectatorUtil.counter(PREFIX + "Lookup", "getApplicationsForARegion", DiscoveryClient.class);
136+
private final Counter LOOKUP_GET_INSTANCES_BY_ID = SpectatorUtil.counter(PREFIX + "Lookup", "getInstancesById", DiscoveryClient.class);
137+
private final Counter LOOKUP_GET_INSTANCES_BY_VIP = SpectatorUtil.counter(PREFIX + "Lookup", "getInstancesByVipAddress", DiscoveryClient.class);
138+
private final Counter LOOKUP_GET_INSTANCES_BY_VIP_AND_APP = SpectatorUtil.counter(PREFIX + "Lookup", "getInstancesByVipAddressAndAppName", DiscoveryClient.class);
139+
private final Counter LOOKUP_GET_NEXT_SERVER = SpectatorUtil.counter(PREFIX + "Lookup", "getNextServerFromEureka", DiscoveryClient.class);
140+
private final Counter LOOKUP_GET_APPLICATIONS_SERVICE_URL = SpectatorUtil.counter(PREFIX + "Lookup", "getApplicationsServiceUrl", DiscoveryClient.class);
141+
136142
// instance variables
137143
/**
138144
* A scheduler to be used for the following 3 tasks:
@@ -564,6 +570,7 @@ public ApplicationInfoManager getApplicationInfoManager() {
564570
*/
565571
@Override
566572
public Application getApplication(String appName) {
573+
LOOKUP_GET_APPLICATION.increment();
567574
return getApplications().getRegisteredApplications(appName);
568575
}
569576

@@ -574,11 +581,13 @@ public Application getApplication(String appName) {
574581
*/
575582
@Override
576583
public Applications getApplications() {
584+
LOOKUP_GET_APPLICATIONS.increment();
577585
return localRegionApps.get();
578586
}
579587

580588
@Override
581589
public Applications getApplicationsForARegion(@Nullable String region) {
590+
LOOKUP_GET_APPLICATIONS_FOR_A_REGION.increment();
582591
if (instanceRegionChecker.isLocalRegion(region)) {
583592
return localRegionApps.get();
584593
} else {
@@ -604,6 +613,7 @@ public Set<String> getAllKnownRegions() {
604613
*/
605614
@Override
606615
public List<InstanceInfo> getInstancesById(String id) {
616+
LOOKUP_GET_INSTANCES_BY_ID.increment();
607617
List<InstanceInfo> instancesList = new ArrayList<>();
608618
for (Application app : this.getApplications()
609619
.getRegisteredApplications()) {
@@ -688,6 +698,7 @@ public List<InstanceInfo> getInstancesByVipAddress(String vipAddress, boolean se
688698
@Override
689699
public List<InstanceInfo> getInstancesByVipAddress(String vipAddress, boolean secure,
690700
@Nullable String region) {
701+
LOOKUP_GET_INSTANCES_BY_VIP.increment();
691702
if (vipAddress == null) {
692703
throw new IllegalArgumentException(
693704
"Supplied VIP Address cannot be null");
@@ -700,30 +711,17 @@ public List<InstanceInfo> getInstancesByVipAddress(String vipAddress, boolean se
700711
if (null == applications) {
701712
logger.debug("No applications are defined for region {}, so returning an empty instance list for vip "
702713
+ "address {}.", region, vipAddress);
703-
recordVipAddressLookup(vipAddress, secure);
704714
return Collections.emptyList();
705715
}
706716
}
707717

708-
recordVipAddressLookup(vipAddress, secure);
709718
if (!secure) {
710719
return applications.getInstancesByVirtualHostName(vipAddress);
711720
} else {
712721
return applications.getInstancesBySecureVirtualHostName(vipAddress);
713722
}
714723
}
715724

716-
private void recordVipAddressLookup(String vipAddress, boolean secure) {
717-
if (clientConfig.shouldEnableVipAddressLookupMetrics()) {
718-
SpectatorUtil.counter(
719-
METRIC_LOOKUP_PREFIX + "vipAddress",
720-
"vip", vipAddress,
721-
"secure", String.valueOf(secure),
722-
"class", "DiscoveryClient"
723-
).increment();
724-
}
725-
}
726-
727725
/**
728726
* Gets the list of instances matching the given VIP Address and the given
729727
* application name if both of them are not null. If one of them is null,
@@ -740,6 +738,7 @@ private void recordVipAddressLookup(String vipAddress, boolean secure) {
740738
@Override
741739
public List<InstanceInfo> getInstancesByVipAddressAndAppName(
742740
String vipAddress, String appName, boolean secure) {
741+
LOOKUP_GET_INSTANCES_BY_VIP_AND_APP.increment();
743742

744743
List<InstanceInfo> result = new ArrayList<>();
745744
if (vipAddress == null && appName == null) {
@@ -794,6 +793,7 @@ public List<InstanceInfo> getInstancesByVipAddressAndAppName(
794793
*/
795794
@Override
796795
public InstanceInfo getNextServerFromEureka(String virtualHostname, boolean secure) {
796+
LOOKUP_GET_NEXT_SERVER.increment();
797797
List<InstanceInfo> instanceInfoList = this.getInstancesByVipAddress(
798798
virtualHostname, secure);
799799
if (instanceInfoList == null || instanceInfoList.isEmpty()) {
@@ -815,6 +815,7 @@ public InstanceInfo getNextServerFromEureka(String virtualHostname, boolean secu
815815
*/
816816
@Override
817817
public Applications getApplications(String serviceUrl) {
818+
LOOKUP_GET_APPLICATIONS_SERVICE_URL.increment();
818819
try {
819820
EurekaHttpResponse<Applications> response = clientConfig.getRegistryRefreshSingleVipAddress() == null
820821
? eurekaTransport.queryClient.getApplications()

‎eureka-client/src/main/java/com/netflix/discovery/EurekaClientConfig.java‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -347,21 +347,6 @@ default boolean shouldUnregisterOnShutdown() {
347347
*/
348348
boolean shouldLogDeltaDiff();
349349

350-
/**
351-
* Indicates whether the eureka client should record metrics for VIP address lookups.
352-
*
353-
* <p>
354-
* When enabled, a counter metric {@code eurekaClient.lookup.vipAddress} is incremented
355-
* each time {@link com.netflix.discovery.shared.LookupService#getInstancesByVipAddress}
356-
* is called, with tags for the VIP address and secure flag.
357-
* </p>
358-
*
359-
* @return true if VIP address lookup metrics should be recorded, false otherwise.
360-
*/
361-
default boolean shouldEnableVipAddressLookupMetrics() {
362-
return false;
363-
}
364-
365350
/**
366351
* Indicates whether the eureka client should disable fetching of delta and
367352
* should rather resort to getting the full registry information.

‎eureka-client/src/main/java/com/netflix/discovery/EurekaClientNames.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ public final class EurekaClientNames {
3939

4040
public static final String METRIC_TRANSPORT_PREFIX = METRIC_PREFIX + "transport.";
4141

42-
public static final String METRIC_LOOKUP_PREFIX = METRIC_PREFIX + "lookup.";
43-
4442
public static final String RESOLVER = "resolver";
4543
public static final String BOOTSTRAP = "bootstrap";
4644
public static final String QUERY = "query";

‎eureka-client/src/main/java/com/netflix/discovery/PropertyBasedClientConfigConstants.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ final class PropertyBasedClientConfigConstants {
6262
static final String EUREKA_SERVER_CONNECTION_IDLE_TIMEOUT_KEY = "eurekaserver.connectionIdleTimeoutInSeconds";
6363

6464
static final String SHOULD_LOG_DELTA_DIFF_KEY = "printDeltaFullDiff";
65-
static final String SHOULD_ENABLE_VIP_ADDRESS_LOOKUP_METRICS_KEY = "vipAddressLookup.metricsEnabled";
6665

6766
static final String CONFIG_DOLLAR_REPLACEMENT_KEY = "dollarReplacement";
6867
static final String CONFIG_ESCAPE_CHAR_REPLACEMENT_KEY = "escapeCharReplacement";

‎eureka-client/src/main/java/com/netflix/discovery/util/SpectatorUtil.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,6 @@ public static Counter counter(@Nonnull String name, @Nullable String id,
100100
return Spectator.globalRegistry().counter(name, tags(id, clazz, extraTags));
101101
}
102102

103-
/**
104-
* Creates a counter with varargs string tags (key-value pairs).
105-
* Example: counter("myMetric", "key1", "value1", "key2", "value2")
106-
*/
107-
public static Counter counter(@Nonnull String name, String... tags) {
108-
return Spectator.globalRegistry().counter(name, tags);
109-
}
110-
111103
public static Timer timer(@Nonnull String name, @Nonnull Class<?> clazz) {
112104
return Spectator.globalRegistry().timer(name, tags(null, clazz));
113105
}

‎eureka-client/src/test/java/com/netflix/discovery/DiscoveryClientRegistryTest.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616
import com.netflix.discovery.shared.transport.EurekaHttpResponse;
1717
import com.netflix.discovery.shared.transport.SimpleEurekaHttpServer;
1818
import com.netflix.discovery.util.InstanceInfoGenerator;
19+
import com.netflix.spectator.api.Counter;
20+
import com.netflix.spectator.api.DefaultRegistry;
21+
import com.netflix.spectator.api.Registry;
22+
import com.netflix.spectator.api.Spectator;
1923
import org.junit.AfterClass;
2024
import org.junit.Before;
2125
import org.junit.BeforeClass;
@@ -52,6 +56,7 @@ public class DiscoveryClientRegistryTest {
5256

5357
private static final EurekaHttpClient requestHandler = mock(EurekaHttpClient.class);
5458
private static SimpleEurekaHttpServer eurekaHttpServer;
59+
private static DefaultRegistry testRegistry;
5560

5661
@Rule
5762
public DiscoveryClientResource discoveryClientResource = DiscoveryClientResource.newBuilder()
@@ -67,13 +72,18 @@ public class DiscoveryClientRegistryTest {
6772
@BeforeClass
6873
public static void setUpClass() throws IOException {
6974
eurekaHttpServer = new SimpleEurekaHttpServer(requestHandler);
75+
testRegistry = new DefaultRegistry();
76+
Spectator.globalRegistry().add(testRegistry);
7077
}
7178

7279
@AfterClass
7380
public static void tearDownClass() throws Exception {
7481
if (eurekaHttpServer != null) {
7582
eurekaHttpServer.shutdown();
7683
}
84+
if (testRegistry != null) {
85+
Spectator.globalRegistry().remove(testRegistry);
86+
}
7787
}
7888

7989
@Before
@@ -314,6 +324,28 @@ public void testEurekaClientPeriodicCacheRefreshForDeleteAndNoApplication() thro
314324
assertEquals(client.getApplications().getRegisteredApplications(), new ArrayList<>());
315325
}
316326

327+
@Test
328+
public void testLookupMetricsIncremented() throws Exception {
329+
Applications applications = InstanceInfoGenerator.newBuilder(2, "app1").build().toApplications();
330+
InstanceInfo instance = applications.getRegisteredApplications("app1").getInstances().get(0);
331+
String vipAddress = instance.getVIPAddress();
332+
333+
when(requestHandler.getApplications(TEST_REMOTE_REGION)).thenReturn(
334+
anEurekaHttpResponse(200, applications).type(MediaType.APPLICATION_JSON_TYPE).build()
335+
);
336+
337+
Registry registry = Spectator.globalRegistry();
338+
Counter vipCounter = registry.counter(
339+
registry.createId("DiscoveryClient_Lookup")
340+
.withTag("id", "getInstancesByVipAddress")
341+
.withTag("class", "DiscoveryClient"));
342+
long initialCount = vipCounter.count();
343+
344+
discoveryClientResource.getClient().getInstancesByVipAddress(vipAddress, false);
345+
346+
assertThat(vipCounter.count(), is(equalTo(initialCount + 1)));
347+
}
348+
317349
/**
318350
* There is a bug, because of which remote registry data structures are not initialized during full registry fetch, only during delta.
319351
*/

0 commit comments

Comments
 (0)