-
Notifications
You must be signed in to change notification settings - Fork 325
Provide a way to configure the tracer using properties file #862
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.
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
Changes from all commits
6fc69ba
1d57a58
4dd6708
076c2b1
bfd9680
da05772
12dc7c4
f578c9c
d0b3214
a3be0cb
d865a1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| package datadog.trace.api; | ||
|
|
||
| import java.io.File; | ||
| import java.io.FileNotFoundException; | ||
| import java.io.FileReader; | ||
| import java.io.IOException; | ||
| import java.net.InetAddress; | ||
| import java.net.UnknownHostException; | ||
| import java.util.Arrays; | ||
|
|
@@ -19,8 +23,8 @@ | |
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| /** | ||
| * Config gives priority to system properties and falls back to environment variables. It also | ||
| * includes default values to ensure a valid config. | ||
| * Config reads values with the following priority: 1) system properties, 2) environment variables, | ||
| * 3) optional configuration file. It also includes default values to ensure a valid config. | ||
| * | ||
| * <p> | ||
| * | ||
|
|
@@ -35,6 +39,7 @@ public class Config { | |
|
|
||
| private static final Pattern ENV_REPLACEMENT = Pattern.compile("[^a-zA-Z0-9_]"); | ||
|
|
||
| public static final String CONFIGURATION_FILE = "trace.config"; | ||
| public static final String SERVICE_NAME = "service.name"; | ||
| public static final String SERVICE = "service"; | ||
| public static final String TRACE_ENABLED = "trace.enabled"; | ||
|
|
@@ -190,9 +195,14 @@ public enum PropagationStyle { | |
|
|
||
| @Getter private final boolean traceAnalyticsEnabled; | ||
|
|
||
| // Read order: System Properties -> Env Variables, [-> default value] | ||
| // Values from an optionally provided properties file | ||
| private static Properties propertiesFromConfigFile; | ||
|
|
||
| // Read order: System Properties -> Env Variables, [-> properties file], [-> default value] | ||
| // Visible for testing | ||
| Config() { | ||
| propertiesFromConfigFile = loadConfigurationFile(); | ||
|
||
|
|
||
| runtimeId = UUID.randomUUID().toString(); | ||
|
|
||
| serviceName = getSettingFromEnvironment(SERVICE_NAME, DEFAULT_SERVICE_NAME); | ||
|
|
@@ -566,25 +576,43 @@ public static boolean traceAnalyticsIntegrationEnabled( | |
| /** | ||
| * Helper method that takes the name, adds a "dd." prefix then checks for System Properties of | ||
| * that name. If none found, the name is converted to an Environment Variable and used to check | ||
| * the env. If setting not configured in either location, defaultValue is returned. | ||
| * the env. If none of the above returns a value, then an optional properties file if checked. If | ||
| * setting is not configured in either location, <code>defaultValue</code> is returned. | ||
| * | ||
| * @param name | ||
| * @param defaultValue | ||
| * @return | ||
| * @deprecated This method should only be used internally. Use the explicit getter instead. | ||
| */ | ||
| public static String getSettingFromEnvironment(final String name, final String defaultValue) { | ||
| final String completeName = PREFIX + name; | ||
| final String value = | ||
| System.getProperties() | ||
| .getProperty(completeName, System.getenv(propertyToEnvironmentName(completeName))); | ||
| return value == null ? defaultValue : value; | ||
| String value; | ||
|
|
||
| // System properties and properties provided from command line have the highest precedence | ||
| value = System.getProperties().getProperty(propertyNameToSystemPropertyName(name)); | ||
| if (null != value) { | ||
labbati marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return value; | ||
| } | ||
|
|
||
| // If value not provided from system properties, looking at env variables | ||
| value = System.getenv(propertyNameToEnvironmentVariableName(name)); | ||
| if (null != value) { | ||
| return value; | ||
| } | ||
|
|
||
| // If value is not defined yet, we look at properties optionally defined in a properties file | ||
| value = propertiesFromConfigFile.getProperty(propertyNameToSystemPropertyName(name)); | ||
| if (null != value) { | ||
| return value; | ||
| } | ||
|
|
||
| return defaultValue; | ||
|
||
| } | ||
|
|
||
| /** @deprecated This method should only be used internally. Use the explicit getter instead. */ | ||
| private static Map<String, String> getMapSettingFromEnvironment( | ||
| final String name, final String defaultValue) { | ||
| return parseMap(getSettingFromEnvironment(name, defaultValue), PREFIX + name); | ||
| return parseMap( | ||
| getSettingFromEnvironment(name, defaultValue), propertyNameToSystemPropertyName(name)); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -674,8 +702,28 @@ private Set<Integer> getIntegerRangeSettingFromEnvironment( | |
| } | ||
| } | ||
|
|
||
| private static String propertyToEnvironmentName(final String name) { | ||
| return ENV_REPLACEMENT.matcher(name.toUpperCase()).replaceAll("_"); | ||
| /** | ||
| * Converts the property name, e.g. 'service.name' into a public environment variable name, e.g. | ||
| * `DD_SERVICE_NAME`. | ||
| * | ||
| * @param setting The setting name, e.g. `service.name` | ||
| * @return The public facing environment variable name | ||
| */ | ||
| private static String propertyNameToEnvironmentVariableName(final String setting) { | ||
| return ENV_REPLACEMENT | ||
| .matcher(propertyNameToSystemPropertyName(setting).toUpperCase()) | ||
| .replaceAll("_"); | ||
| } | ||
|
|
||
| /** | ||
| * Converts the property name, e.g. 'service.name' into a public system property name, e.g. | ||
| * `dd.service.name`. | ||
| * | ||
| * @param setting The setting name, e.g. `service.name` | ||
| * @return The public facing system property name | ||
| */ | ||
| private static String propertyNameToSystemPropertyName(String setting) { | ||
| return PREFIX + setting; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| } | ||
|
|
||
| private static Map<String, String> getPropertyMapValue( | ||
|
|
@@ -830,6 +878,50 @@ private static <V extends Enum<V>> Set<V> convertStringSetToEnumSet( | |
| return Collections.unmodifiableSet(result); | ||
| } | ||
|
|
||
| /** | ||
| * Loads the optional configuration properties file into the global {@link Properties} object. | ||
| * | ||
| * @return The {@link Properties} object. the returned instance might be empty of file does not | ||
| * exist or if it is in a wrong format. | ||
| */ | ||
| private static Properties loadConfigurationFile() { | ||
| Properties properties = new Properties(); | ||
|
|
||
| // Reading from system property first and from env after | ||
| String configurationFilePath = | ||
| System.getProperty(propertyNameToSystemPropertyName(CONFIGURATION_FILE)); | ||
| if (null == configurationFilePath) { | ||
| configurationFilePath = | ||
| System.getenv(propertyNameToEnvironmentVariableName(CONFIGURATION_FILE)); | ||
| } | ||
| if (null == configurationFilePath) { | ||
| return properties; | ||
| } | ||
|
|
||
| // Normalizing tilde (~) paths for unix systems | ||
| configurationFilePath = | ||
| configurationFilePath.replaceFirst("^~", System.getProperty("user.home")); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised... I would think java would handle this automatically. |
||
|
|
||
| // Configuration properties file is optional | ||
| File configurationFile = new File(configurationFilePath); | ||
| if (!configurationFile.exists()) { | ||
| log.error("Configuration file '{}' not found.", configurationFilePath); | ||
| return properties; | ||
| } | ||
|
|
||
| try { | ||
| FileReader fileReader = new FileReader(configurationFile); | ||
| properties.load(fileReader); | ||
| } catch (FileNotFoundException fnf) { | ||
| log.error("Configuration file '{}' not found.", configurationFilePath); | ||
| } catch (IOException ioe) { | ||
| log.error( | ||
| "Configuration file '{}' cannot be accessed or correctly parsed.", configurationFilePath); | ||
| } | ||
|
|
||
| return properties; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the detected hostname. This operation is time consuming so if the usage changes and | ||
| * this method will be called several times then we should implement some sort of caching. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import spock.lang.Specification | |
| import static datadog.trace.api.Config.AGENT_HOST | ||
| import static datadog.trace.api.Config.AGENT_PORT_LEGACY | ||
| import static datadog.trace.api.Config.AGENT_UNIX_DOMAIN_SOCKET | ||
| import static datadog.trace.api.Config.CONFIGURATION_FILE | ||
| import static datadog.trace.api.Config.DB_CLIENT_HOST_SPLIT_BY_INSTANCE | ||
| import static datadog.trace.api.Config.DEFAULT_JMX_FETCH_STATSD_PORT | ||
| import static datadog.trace.api.Config.GLOBAL_TAGS | ||
|
|
@@ -725,4 +726,65 @@ class ConfigTest extends Specification { | |
| then: | ||
| config.localRootSpanTags.get('_dd.hostname') == InetAddress.localHost.hostName | ||
| } | ||
|
|
||
| def "verify fallback to properties file"() { | ||
| setup: | ||
| System.setProperty(PREFIX + CONFIGURATION_FILE, "src/test/resources/dd-java-tracer.properties") | ||
|
|
||
| when: | ||
| def config = new Config() | ||
|
|
||
| then: | ||
| config.serviceName == "set-in-properties" | ||
|
|
||
| cleanup: | ||
| System.clearProperty(PREFIX + CONFIGURATION_FILE) | ||
| } | ||
|
|
||
| def "verify fallback to properties file has lower priority than system property"() { | ||
| setup: | ||
| System.setProperty(PREFIX + CONFIGURATION_FILE, "src/test/resources/dd-java-tracer.properties") | ||
| System.setProperty(PREFIX + SERVICE_NAME, "set-in-system") | ||
|
|
||
| when: | ||
| def config = new Config() | ||
|
|
||
| then: | ||
| config.serviceName == "set-in-system" | ||
|
|
||
| cleanup: | ||
| System.clearProperty(PREFIX + CONFIGURATION_FILE) | ||
| System.clearProperty(PREFIX + SERVICE_NAME) | ||
| } | ||
|
|
||
| def "verify fallback to properties file has lower priority than env var"() { | ||
| setup: | ||
| System.setProperty(PREFIX + CONFIGURATION_FILE, "src/test/resources/dd-java-tracer.properties") | ||
| environmentVariables.set("DD_SERVICE_NAME", "set-in-env") | ||
|
|
||
| when: | ||
| def config = new Config() | ||
|
|
||
| then: | ||
| config.serviceName == "set-in-env" | ||
|
|
||
| cleanup: | ||
| System.clearProperty(PREFIX + CONFIGURATION_FILE) | ||
| System.clearProperty(PREFIX + SERVICE_NAME) | ||
| environmentVariables.clear("DD_SERVICE_NAME") | ||
| } | ||
|
|
||
| def "verify fallback to properties file that does not exist does not crash app"() { | ||
| setup: | ||
| System.setProperty(PREFIX + CONFIGURATION_FILE, "src/test/resources/do-not-exist.properties") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may want to clear this property in cleanup. |
||
|
|
||
| when: | ||
| def config = new Config() | ||
|
|
||
| then: | ||
| config.serviceName == 'unnamed-java-app' | ||
|
|
||
| cleanup: | ||
| System.clearProperty(PREFIX + CONFIGURATION_FILE) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| dd.service.name=set-in-properties |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I commented about updating comment I meant this one :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opps :)