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

🌶 Ignore sysprop formatted envvars and log the correct variable name #39

Merged
merged 2 commits into from
Sep 15, 2020
Merged

🌶 Ignore sysprop formatted envvars and log the correct variable name #39

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
79ee8ee
#38 ignore sysprop formatted envvars and log the correct variable name
tspring Aug 28, 2020
d69b987
test to ensure we dont add sysprop envvars to the config
tspring Aug 28, 2020
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
24 changes: 14 additions & 10 deletions newrelic-agent/src/main/java/com/newrelic/agent/config/SystemPropertyProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public class SystemPropertyProvider {
private static final String LOG_FILE_NAME = AgentConfigImpl.SYSTEM_PROPERTY_ROOT + AgentConfigImpl.LOG_FILE_NAME;
private static final String NEW_RELIC_SYSTEM_PROPERTY_ROOT = "newrelic.";


private final Map<String, String> envVars;
private final Map<String, String> envVarToSystemPropKeyMap;
private final Map<String, String> newRelicSystemProps;
Expand Down Expand Up @@ -85,7 +84,8 @@ private Map<String, Object> createNewRelicSystemPropertiesWithoutPrefix() {
addNewRelicSystemProperties(nrProps, systemProps.getAllSystemProperties());
return Collections.unmodifiableMap(nrProps);
}
private Map<String, Object> createNewRelicEnvVarsWithoutPrefix(){

private Map<String, Object> createNewRelicEnvVarsWithoutPrefix() {
Map<String, Object> nrEnv = new HashMap<>();
addNewRelicEnvProperties(nrEnv, environmentFacade.getAllEnvProperties());
return Collections.unmodifiableMap(nrEnv);
Expand All @@ -110,12 +110,15 @@ private void addNewRelicEnvProperties(Map<String, Object> nrProps, Map<String, S
} else {
addPropertyWithoutEnvPrefix(nrProps, envVar.toLowerCase(), entry.getValue());
}
} else if (envVar.startsWith(AgentConfigImpl.SYSTEM_PROPERTY_ROOT)){
//because some newrelic.config properties get passed as environment variables
addPropertyWithoutSystemPropRoot(nrProps, envVar, entry.getValue());
} else if (envVar.startsWith(AgentConfigImpl.SYSTEM_PROPERTY_ROOT)) {
Agent.LOG.log(Level.WARNING,
"The agent only supports environment variable configurations consisting of" +
" alphanumeric characters and underscores. Use {0} instead.",
formatNewRelicEnvVarPrefix(envVar));
}
}
}

private void addPropertyWithoutEnvPrefix(Map<String, Object> nrProps, String key, Object value) {
nrProps.put(key.substring(NEW_RELIC_PREFIX_ENV.length()), value);
}
Expand All @@ -139,21 +142,22 @@ private String getenv(String key) {
return val;
}
//check if current key needs to be converted from NR config prop to NR env var
String removeConfigKey = addNewRelicEnvVarPrefix(key.replace("newrelic.config", "new.relic"));
return environmentFacade.getenv(removeConfigKey);

return environmentFacade.getenv(formatNewRelicEnvVarPrefix(key));
}

private String addNewRelicEnvVarPrefix(String key) {
private String formatNewRelicEnvVarPrefix(String key) {
// Replace any dots and dashes with underscores to allow config to be set as environment variables. We are replacing dashes here because
// our instrumentation modules have dashes in their names and we want to be able to allow those to be disabled via environment variables.
return key.replaceAll("[.-]", "_").toUpperCase();
return key.replace("newrelic.config", "new.relic")
.replaceAll("[.-]", "_")
.toUpperCase();
}

public String getSystemProperty(String prop) {
return systemProps.getSystemProperty(prop);
}


/**
* Get a map of the New Relic system properties (any property starting with newrelic.)
*/
Expand Down
23 changes: 3 additions & 20 deletions newrelic-agent/src/test/java/com/newrelic/agent/config/SystemPropertyProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;

public class SystemPropertyProviderTest {
@Test
Expand Down Expand Up @@ -50,25 +51,6 @@ public void testSystemPropertyProviderGeneralEnvVars() {
assertEquals("logfile.log", provider.getNewRelicEnvVarsWithoutPrefix().get("log_file_name"));
}

@Test
public void testSystemPropertyProviderGeneralEnvProps() {
//to cover for a case where config properties get passed as environment variables.
Map<String, String> envs = new HashMap<>(System.getenv());
envs.put("newrelic.config.process_host.display_name", "hello");
envs.put("newrelic.config.app_name", "people");
envs.put("newrelic.config.log_file_name", "logfile.log");

SystemPropertyProvider provider = new SystemPropertyProvider(
new SaveSystemPropertyProviderRule.TestSystemProps(),
new SaveSystemPropertyProviderRule.TestEnvironmentFacade(envs)
);

assertNotNull("Properties can not be null", provider.getNewRelicEnvVarsWithoutPrefix().get("process_host.display_name"));
assertEquals("hello", provider.getNewRelicEnvVarsWithoutPrefix().get("process_host.display_name"));
assertEquals("people", provider.getNewRelicEnvVarsWithoutPrefix().get("app_name"));
assertEquals("logfile.log", provider.getNewRelicEnvVarsWithoutPrefix().get("log_file_name"));
}

@Test
public void testEnvironmentVariable() {
Map<String, String> envs = new HashMap<>();
Expand All @@ -89,7 +71,8 @@ public void testEnvironmentVariable() {
assertEquals("hello", provider.getEnvironmentVariable("NEW_RELIC_PROCESS_HOST_DISPLAY_NAME"));
assertEquals("people", provider.getEnvironmentVariable("NEW_RELIC_APP_NAME"));
assertEquals("10.96.0.1", provider.getEnvironmentVariable("KUBERNETES_SERVICE_HOST"));
assertEquals("true", provider.getEnvironmentVariable("newrelic.config.distributed_tracing.enabled"));
// Environment variables with system property style dot notation should not be read by the agent
assertNull(provider.getNewRelicEnvVarsWithoutPrefix().get("distributed_tracing.enabled"));
}

@Test
Expand Down