Skip to content

Commit 1154bec

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 1154bec

8 files changed

Lines changed: 48 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: 16 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,15 @@ 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+
136141
// instance variables
137142
/**
138143
* A scheduler to be used for the following 3 tasks:
@@ -564,6 +569,7 @@ public ApplicationInfoManager getApplicationInfoManager() {
564569
*/
565570
@Override
566571
public Application getApplication(String appName) {
572+
LOOKUP_GET_APPLICATION.increment();
567573
return getApplications().getRegisteredApplications(appName);
568574
}
569575

@@ -574,11 +580,13 @@ public Application getApplication(String appName) {
574580
*/
575581
@Override
576582
public Applications getApplications() {
583+
LOOKUP_GET_APPLICATIONS.increment();
577584
return localRegionApps.get();
578585
}
579586

580587
@Override
581588
public Applications getApplicationsForARegion(@Nullable String region) {
589+
LOOKUP_GET_APPLICATIONS_FOR_A_REGION.increment();
582590
if (instanceRegionChecker.isLocalRegion(region)) {
583591
return localRegionApps.get();
584592
} else {
@@ -604,6 +612,7 @@ public Set<String> getAllKnownRegions() {
604612
*/
605613
@Override
606614
public List<InstanceInfo> getInstancesById(String id) {
615+
LOOKUP_GET_INSTANCES_BY_ID.increment();
607616
List<InstanceInfo> instancesList = new ArrayList<>();
608617
for (Application app : this.getApplications()
609618
.getRegisteredApplications()) {
@@ -688,6 +697,7 @@ public List<InstanceInfo> getInstancesByVipAddress(String vipAddress, boolean se
688697
@Override
689698
public List<InstanceInfo> getInstancesByVipAddress(String vipAddress, boolean secure,
690699
@Nullable String region) {
700+
LOOKUP_GET_INSTANCES_BY_VIP.increment();
691701
if (vipAddress == null) {
692702
throw new IllegalArgumentException(
693703
"Supplied VIP Address cannot be null");
@@ -700,30 +710,17 @@ public List<InstanceInfo> getInstancesByVipAddress(String vipAddress, boolean se
700710
if (null == applications) {
701711
logger.debug("No applications are defined for region {}, so returning an empty instance list for vip "
702712
+ "address {}.", region, vipAddress);
703-
recordVipAddressLookup(vipAddress, secure);
704713
return Collections.emptyList();
705714
}
706715
}
707716

708-
recordVipAddressLookup(vipAddress, secure);
709717
if (!secure) {
710718
return applications.getInstancesByVirtualHostName(vipAddress);
711719
} else {
712720
return applications.getInstancesBySecureVirtualHostName(vipAddress);
713721
}
714722
}
715723

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-
727724
/**
728725
* Gets the list of instances matching the given VIP Address and the given
729726
* application name if both of them are not null. If one of them is null,
@@ -740,6 +737,7 @@ private void recordVipAddressLookup(String vipAddress, boolean secure) {
740737
@Override
741738
public List<InstanceInfo> getInstancesByVipAddressAndAppName(
742739
String vipAddress, String appName, boolean secure) {
740+
LOOKUP_GET_INSTANCES_BY_VIP_AND_APP.increment();
743741

744742
List<InstanceInfo> result = new ArrayList<>();
745743
if (vipAddress == null && appName == null) {
@@ -794,6 +792,7 @@ public List<InstanceInfo> getInstancesByVipAddressAndAppName(
794792
*/
795793
@Override
796794
public InstanceInfo getNextServerFromEureka(String virtualHostname, boolean secure) {
795+
LOOKUP_GET_NEXT_SERVER.increment();
797796
List<InstanceInfo> instanceInfoList = this.getInstancesByVipAddress(
798797
virtualHostname, secure);
799798
if (instanceInfoList == null || instanceInfoList.isEmpty()) {

‎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)