Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix instrumentation propagation of PrefixedViewConfigs #721

Merged
merged 2 commits into from
Apr 15, 2024
Merged

Fix instrumentation propagation of PrefixedViewConfigs #721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfb4a64
Update handling of nested PrefixedViewConfigs
akang31 Apr 12, 2024
a723036
Undo the extra propagation
akang31 Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion archaius2-core/src/main/java/com/netflix/archaius/config/AbstractDependentConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public boolean isEmpty() {
@Override
public void recordUsage(PropertyDetails propertyDetails) {
if (getState().getInstrumentedKeys().containsKey(propertyDetails.getKey())) {
getState().getInstrumentedKeys().get(propertyDetails.getKey()).recordUsage(propertyDetails);
getState().getInstrumentedKeys().get(propertyDetails.getKey()).recordUsage(createPropertyDetails(propertyDetails.getKey(), propertyDetails.getValue()));
}
}

Expand Down
55 changes: 41 additions & 14 deletions archaius2-core/src/test/java/com/netflix/archaius/config/CompositeConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.common.collect.Sets;
import com.netflix.archaius.api.Config;
import com.netflix.archaius.api.config.SettableConfig;
import com.netflix.archaius.api.config.CompositeConfig;
import com.netflix.archaius.config.polling.ManualPollingStrategy;
import com.netflix.archaius.config.polling.PollingResponse;
import com.netflix.archaius.instrumentation.AccessMonitorUtil;
Expand Down Expand Up @@ -56,10 +57,10 @@ public void basicTest() throws ConfigException {
Properties props = new Properties();
props.setProperty("env", "prod");

com.netflix.archaius.api.config.CompositeConfig libraries = new DefaultCompositeConfig();
com.netflix.archaius.api.config.CompositeConfig application = new DefaultCompositeConfig();
CompositeConfig libraries = new DefaultCompositeConfig();
CompositeConfig application = new DefaultCompositeConfig();

com.netflix.archaius.api.config.CompositeConfig config = DefaultCompositeConfig.builder()
CompositeConfig config = DefaultCompositeConfig.builder()
.withConfig("lib", libraries)
.withConfig("app", application)
.withConfig("set", MapConfig.from(props))
Expand Down Expand Up @@ -100,10 +101,10 @@ public void basicReversedTest() throws ConfigException {
Properties props = new Properties();
props.setProperty("env", "prod");

com.netflix.archaius.api.config.CompositeConfig libraries = new DefaultCompositeConfig(true);
com.netflix.archaius.api.config.CompositeConfig application = new DefaultCompositeConfig();
CompositeConfig libraries = new DefaultCompositeConfig(true);
CompositeConfig application = new DefaultCompositeConfig();

com.netflix.archaius.api.config.CompositeConfig config = DefaultCompositeConfig.builder()
CompositeConfig config = DefaultCompositeConfig.builder()
.withConfig("lib", libraries)
.withConfig("app", application)
.withConfig("set", MapConfig.from(props))
Expand Down Expand Up @@ -142,7 +143,7 @@ public void basicReversedTest() throws ConfigException {
@SuppressWarnings("deprecation")
@Test
public void getKeysTest() throws ConfigException {
com.netflix.archaius.api.config.CompositeConfig composite = new DefaultCompositeConfig();
CompositeConfig composite = new DefaultCompositeConfig();
composite.addConfig("a", EmptyConfig.INSTANCE);

Iterator<String> iter = composite.getKeys();
Expand All @@ -167,7 +168,7 @@ public void getKeysTest() throws ConfigException {

@Test
public void testGetKeysIteratorRemoveThrows() throws ConfigException {
com.netflix.archaius.api.config.CompositeConfig composite = new DefaultCompositeConfig();
CompositeConfig composite = new DefaultCompositeConfig();


composite.addConfig("d", MapConfig.builder().put("d1", "A").put("d2", "B").build());
Expand All @@ -182,7 +183,7 @@ public void testGetKeysIteratorRemoveThrows() throws ConfigException {

@Test
public void testKeysIterable() throws ConfigException {
com.netflix.archaius.api.config.CompositeConfig composite = new DefaultCompositeConfig();
CompositeConfig composite = new DefaultCompositeConfig();

composite.addConfig("d", MapConfig.builder().put("d1", "A").put("d2", "B").build());
composite.addConfig("e", MapConfig.builder().put("e1", "A").put("e2", "B").build());
Expand All @@ -195,7 +196,7 @@ public void testKeysIterable() throws ConfigException {

@Test
public void testKeysIterableModificationThrows() throws ConfigException {
com.netflix.archaius.api.config.CompositeConfig composite = new DefaultCompositeConfig();
CompositeConfig composite = new DefaultCompositeConfig();

composite.addConfig("d", MapConfig.builder().put("d1", "A").put("d2", "B").build());
composite.addConfig("e", MapConfig.builder().put("e1", "A").put("e2", "B").build());
Expand All @@ -207,7 +208,7 @@ public void testKeysIterableModificationThrows() throws ConfigException {
@Test
public void unusedCompositeConfigIsGarbageCollected() throws ConfigException {
SettableConfig sourceConfig = new DefaultSettableConfig();
com.netflix.archaius.api.config.CompositeConfig config = DefaultCompositeConfig.builder()
CompositeConfig config = DefaultCompositeConfig.builder()
.withConfig("settable", sourceConfig)
.build();
Reference<Config> weakReference = new WeakReference<>(config);
Expand All @@ -220,7 +221,7 @@ public void unusedCompositeConfigIsGarbageCollected() throws ConfigException {

@Test
public void instrumentationNotEnabled() throws Exception {
com.netflix.archaius.api.config.CompositeConfig composite = new DefaultCompositeConfig();
CompositeConfig composite = new DefaultCompositeConfig();

composite.addConfig("polling", createPollingDynamicConfig("a1", "1", "b1", "2", null));

Expand All @@ -231,13 +232,13 @@ public void instrumentationNotEnabled() throws Exception {

@Test
public void instrumentationPropagation() throws Exception {
com.netflix.archaius.api.config.CompositeConfig composite = new DefaultCompositeConfig();
CompositeConfig composite = new DefaultCompositeConfig();
AccessMonitorUtil accessMonitorUtil = spy(AccessMonitorUtil.builder().build());

PollingDynamicConfig outerPollingDynamicConfig = createPollingDynamicConfig("a1", "1", "b1", "2", accessMonitorUtil);
composite.addConfig("outer", outerPollingDynamicConfig);

com.netflix.archaius.api.config.CompositeConfig innerComposite = new DefaultCompositeConfig();
CompositeConfig innerComposite = new DefaultCompositeConfig();
PollingDynamicConfig nestedPollingDynamicConfig = createPollingDynamicConfig("b1", "1", "c1", "3", accessMonitorUtil);
innerComposite.addConfig("polling", nestedPollingDynamicConfig);
composite.addConfig("innerComposite", innerComposite);
Expand Down Expand Up @@ -283,6 +284,32 @@ public void instrumentationPropagation() throws Exception {
verify(accessMonitorUtil, times(6)).registerUsage((any()));
}

@Test
public void instrumentationNestedPrefixedViewConfig() throws Exception {
/*
CompositeConfig (config)
|
PrefixedViewConfig (prefixedConfig)
|
PollingDynamicConfig (instrumentedConfig)
*/
AccessMonitorUtil accessMonitorUtil = spy(AccessMonitorUtil.builder().build());
PollingDynamicConfig instrumentedConfig =
createPollingDynamicConfig("prefix.a1", "1", "b1", "2", accessMonitorUtil);
Config prefixedConfig = instrumentedConfig.getPrefixedView("prefix");
CompositeConfig config = new DefaultCompositeConfig();
config.addConfig("prefixedConfig", prefixedConfig);

assertEquals("1", config.getRawProperty("a1"));
verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("prefix.a1", "prefix.a1", "1")));

assertEquals("1", config.getRawPropertyUninstrumented("a1"));
verify(accessMonitorUtil, times(1)).registerUsage(any());

assertNull(config.getRawProperty("b1"));
verify(accessMonitorUtil, times(1)).registerUsage(any());
}

private PollingDynamicConfig createPollingDynamicConfig(
String key1, String value1, String key2, String value2, AccessMonitorUtil accessMonitorUtil) throws Exception {
ManualPollingStrategy strategy = new ManualPollingStrategy();
Expand Down
Loading