From bba49140b538b6770d8f96a673d021e536295a04 Mon Sep 17 00:00:00 2001 From: Pablo Collins Date: Fri, 11 Nov 2022 15:47:53 -0500 Subject: [PATCH] java zc: don't generate service name by default (#2250) This change adds support to the Java Linux auto instrumentation configuration file for a new, optional parameter called generate_svc_name. If left unset, it defaults to false, which means that the .so will not generate a service name and add it to the environment of the soon-to-be running Java process. This differs from prior behavior where the .so did generate the service name, but generating the service name in the .so is no longer optimal because the Java instrumentation library itself will generate the service name if it hasn't been set. --- instrumentation/Dockerfile | 1 + instrumentation/README.md | 8 +- instrumentation/src/config.c | 7 +- instrumentation/src/config.h | 3 +- instrumentation/src/splunk.c | 30 ++++--- instrumentation/src/test_main.c | 89 ++++++++++++++----- instrumentation/src/test_main.h | 8 +- instrumentation/src/test_utils.c | 6 ++ instrumentation/src/test_utils.h | 2 + .../testdata/instrumentation-svcname.conf | 5 ++ 10 files changed, 122 insertions(+), 37 deletions(-) create mode 100644 instrumentation/testdata/instrumentation-svcname.conf diff --git a/instrumentation/Dockerfile b/instrumentation/Dockerfile index 00e0ac6e68e..e5c3649ca70 100644 --- a/instrumentation/Dockerfile +++ b/instrumentation/Dockerfile @@ -7,5 +7,6 @@ WORKDIR /libsplunk COPY src /libsplunk/src COPY testdata/instrumentation.conf /libsplunk/testdata/instrumentation.conf +COPY testdata/instrumentation-svcname.conf /libsplunk/testdata/instrumentation-svcname.conf COPY install/instrumentation.conf /libsplunk/install/instrumentation.conf COPY Makefile /libsplunk/Makefile diff --git a/instrumentation/README.md b/instrumentation/README.md index 3cd96cdff61..8c1c4788e78 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -9,7 +9,7 @@ This directory contains functionality for building a Linux .so (shared object) f reference to that file in `/etc/ld.so.preload` (provided by an installer defined elsewhere) causes processes on the host to run this .so before the main executable runs. If the executable that's starting is not named `java`, the .so quits silently and the -executable starts normally. Otherwise, it attempts to set two environment variables that will cause the +executable starts normally. Otherwise, it attempts to set environment variables that will cause the [Splunk OTel Java JAR](https://github.com/signalfx/splunk-otel-java) (also provided by the installer) to instrument the soon-to-be running Java application. In this way, all Java applications on the host will be automatically instrumented by the Splunk OTel Java agent. @@ -35,6 +35,12 @@ The `java_agent_jar` parameter is set to the default location of the agent jar. The full path to the auto instrumentation JAR (provided by the installer). +#### `generate_service_name` (optional) + +Whether to disable service name generation by the .so. If set to "true", the .so will attempt to generate a service name +from the Java command arguments. If set to "false" (the default), it will not set the service name, leaving it to be +generated by the Java auto-instrumentation library. + #### `service_name` (optional) This is an optional override for the service name that would otherwise be generated by the shared object before Java diff --git a/instrumentation/src/config.c b/instrumentation/src/config.c index 2ab0ecb31da..771ba7a97f4 100644 --- a/instrumentation/src/config.c +++ b/instrumentation/src/config.c @@ -29,6 +29,9 @@ void load_config(logger log, struct config *cfg, char *file_name) { if (cfg->disable_telemetry == NULL) { log_debug(log, "disable_telemetry not specified in config"); } + if (cfg->generate_service_name == NULL) { + log_debug(log, "generate_service_name not specified in config"); + } } void read_config_file(logger log, struct config *cfg, char *file_name) { @@ -63,6 +66,8 @@ void read_lines(struct config *cfg, FILE *fp) { cfg->resource_attributes = strdup(pair.v); } else if (streq(pair.k, "disable_telemetry")) { cfg->disable_telemetry = strdup(pair.v); + } else if (streq(pair.k, "generate_service_name")) { + cfg->generate_service_name = strdup(pair.v); } } } @@ -72,7 +77,7 @@ void split_on_eq(char *string, struct kv *pair) { pair->v = string; } -bool eq_true(char *v) { +bool str_eq_true(char *v) { return v != NULL && !streq("false", v) && !streq("FALSE", v) && !streq("0", v); } diff --git a/instrumentation/src/config.h b/instrumentation/src/config.h index 5e4a521b475..13d14d800cc 100644 --- a/instrumentation/src/config.h +++ b/instrumentation/src/config.h @@ -10,11 +10,12 @@ struct config { char *service_name; char *resource_attributes; char *disable_telemetry; + char *generate_service_name; }; void load_config(logger log, struct config *cfg, char *file_name); -bool eq_true(char *v); +bool str_eq_true(char *v); void free_config(struct config *cfg); diff --git a/instrumentation/src/splunk.c b/instrumentation/src/splunk.c index a62cf750a2f..3effbd13000 100644 --- a/instrumentation/src/splunk.c +++ b/instrumentation/src/splunk.c @@ -33,6 +33,8 @@ void set_env_var(logger log, const char *var_name, const char *value); void set_env_var_from_attr(logger log, const char *attr_name, const char *env_var_name, const char *value); +void get_service_name(logger log, cmdline_reader cr, struct config *cfg, char *dest); + // The entry point for all executables prior to their execution. If the executable is named "java", we // set the env vars JAVA_TOOL_OPTIONS to the path of the java agent jar and OTEL_SERVICE_NAME to the // service name based on the arguments to the java command. @@ -85,24 +87,22 @@ void auto_instrument( } char service_name[MAX_CMDLINE_LEN] = ""; - if (cfg.service_name == NULL) { - get_service_name_from_cmdline(log, service_name, cr); + if (str_eq_true(cfg.generate_service_name)) { + get_service_name(log, cr, &cfg, service_name); + if (strlen(service_name) == 0) { + log_info(log, "service name empty, quitting"); + return; + } + set_env_var(log, otel_service_name_var, service_name); } else { - strncpy(service_name, cfg.service_name, MAX_CMDLINE_LEN); + log_debug(log, "service name generation disabled"); } - if (strlen(service_name) == 0) { - log_info(log, "service name empty, quitting"); - return; - } - - set_env_var(log, otel_service_name_var, service_name); - set_java_tool_options(log, &cfg); set_env_var_from_attr(log, "resource_attributes", resource_attributes_var, cfg.resource_attributes); - if (eq_true(cfg.disable_telemetry)) { + if (str_eq_true(cfg.disable_telemetry)) { log_info(log, "disabling telemetry as per config"); } else { send_otlp_metric_func(log, service_name); @@ -111,6 +111,14 @@ void auto_instrument( free_config(&cfg); } +void get_service_name(logger log, cmdline_reader cr, struct config *cfg, char *dest) { + if (cfg->service_name == NULL) { + get_service_name_from_cmdline(log, dest, cr); + } else { + strncpy(dest, (*cfg).service_name, MAX_CMDLINE_LEN); + } +} + void get_service_name_from_cmdline(logger log, char *dest, cmdline_reader cr) { char *args[MAX_ARGS]; int n = get_cmdline_args(args, cr, MAX_ARGS, MAX_CMDLINE_LEN, log); diff --git a/instrumentation/src/test_main.c b/instrumentation/src/test_main.c index c36110f58b5..26d7f9b3b5f 100644 --- a/instrumentation/src/test_main.c +++ b/instrumentation/src/test_main.c @@ -8,6 +8,7 @@ #include static char *const test_config_path = "testdata/instrumentation.conf"; +static char *const test_config_path_svcname = "testdata/instrumentation-svcname.conf"; int main(void) { run_tests(); @@ -18,6 +19,7 @@ int main(void) { void run_tests() { test_func_t *tests[] = { test_auto_instrument_svc_name_in_config, + test_auto_instrument_svc_name_explicitly_enabled, test_auto_instrument_no_svc_name_in_config, test_auto_instrument_not_java, test_auto_instrument_no_access, @@ -26,6 +28,7 @@ void run_tests() { test_auto_instrument_splunk_env_var_false_caps, test_auto_instrument_splunk_env_var_zero, test_read_config, + test_read_config_svcname, test_read_config_missing_file, test_read_args_simple, test_read_args_max_args_limit, @@ -72,20 +75,34 @@ void run_test(test_func_t run_test) { void test_auto_instrument_svc_name_in_config(logger l) { cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_true, "java", fake_load_config, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_true, "java", fake_load_config_svcname_unspecified, cr, fake_send_otlp_metric); char *logs[256]; int n = get_logs(l, logs); char *funcname = "test_auto_instrument_svc_name_in_config"; require_equal_ints(funcname, 4, n); - require_equal_strings(funcname, "setting OTEL_SERVICE_NAME='my.service'", logs[0]); + require_equal_strings(funcname, "service name generation disabled", logs[0]); require_equal_strings(funcname, "setting JAVA_TOOL_OPTIONS='-javaagent:/foo/asdf.jar'", logs[1]); require_equal_strings(funcname, "setting OTEL_RESOURCE_ATTRIBUTES='myattr=myval'", logs[2]); require_equal_strings(funcname, "sending metric", logs[3]); require_env(funcname, "-javaagent:/foo/asdf.jar", java_tool_options_var); - require_env(funcname, "my.service", otel_service_name_var); + require_unset_env(funcname, otel_service_name_var); cmdline_reader_close(cr); } +void test_auto_instrument_svc_name_explicitly_enabled(logger l) { + char *funcname = "test_auto_instrument_svc_name_explicitly_enabled"; + cmdline_reader cr = new_default_test_cmdline_reader(); + auto_instrument(l, access_check_true, "java", fake_load_config_generate_svcname_enabled, cr, fake_send_otlp_metric); + char *logs[256]; + int n = get_logs(l, logs); + require_equal_strings(funcname, "setting OTEL_SERVICE_NAME='my.service'", logs[0]); + require_equal_strings(funcname, "setting JAVA_TOOL_OPTIONS='-javaagent:/foo/asdf.jar'", logs[1]); + require_equal_strings(funcname, "setting OTEL_RESOURCE_ATTRIBUTES='myattr=myval'", logs[2]); + require_equal_strings(funcname, "sending metric", logs[3]); + require_equal_ints(funcname, 4, n); + require_env(funcname, "my.service", otel_service_name_var); +} + void test_auto_instrument_no_svc_name_in_config(logger l) { cmdline_reader cr = new_default_test_cmdline_reader(); auto_instrument(l, access_check_true, "java", fake_load_config_no_svcname, cr, fake_send_otlp_metric); @@ -93,17 +110,17 @@ void test_auto_instrument_no_svc_name_in_config(logger l) { int n = get_logs(l, logs); char *funcname = "test_auto_instrument_no_svc_name_in_config"; require_equal_ints(funcname, 3, n); - require_equal_strings(funcname, "setting OTEL_SERVICE_NAME='foo'", logs[0]); + require_equal_strings(funcname, "service name generation disabled", logs[0]); require_equal_strings(funcname, "setting JAVA_TOOL_OPTIONS='-javaagent:/foo/asdf.jar'", logs[1]); require_equal_strings(funcname, "sending metric", logs[2]); require_env(funcname, "-javaagent:/foo/asdf.jar", java_tool_options_var); - require_env(funcname, "foo", otel_service_name_var); + require_unset_env(funcname, otel_service_name_var); cmdline_reader_close(cr); } void test_auto_instrument_not_java(logger l) { cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_true, "foo", fake_load_config, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_true, "foo", fake_load_config_svcname_unspecified, cr, fake_send_otlp_metric); char *funcname = "test_auto_instrument_not_java"; require_unset_env(funcname, java_tool_options_var); char *logs[256]; @@ -114,7 +131,7 @@ void test_auto_instrument_not_java(logger l) { void test_auto_instrument_no_access(logger l) { cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_false, "java", fake_load_config, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_false, "java", fake_load_config_svcname_unspecified, cr, fake_send_otlp_metric); require_unset_env("test_auto_instrument_no_access", java_tool_options_var); char *logs[256]; char *funcname = "test_auto_instrument_no_access"; @@ -126,7 +143,7 @@ void test_auto_instrument_no_access(logger l) { void test_auto_instrument_splunk_env_var_true(logger l) { setenv(disable_env_var, "true", 0); cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_true, "java", fake_load_config, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_true, "java", fake_load_config_svcname_unspecified, cr, fake_send_otlp_metric); require_unset_env("test_auto_instrument_splunk_env_var_true", "JAVA_TOOL_OPTIONS"); cmdline_reader_close(cr); } @@ -134,7 +151,7 @@ void test_auto_instrument_splunk_env_var_true(logger l) { void test_auto_instrument_splunk_env_var_false(logger l) { setenv(disable_env_var, "false", 0); cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_true, "java", fake_load_config, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_true, "java", fake_load_config_svcname_unspecified, cr, fake_send_otlp_metric); require_env("test_auto_instrument_splunk_env_var_false", "-javaagent:/foo/asdf.jar", "JAVA_TOOL_OPTIONS"); cmdline_reader_close(cr); } @@ -142,7 +159,7 @@ void test_auto_instrument_splunk_env_var_false(logger l) { void test_auto_instrument_splunk_env_var_false_caps(logger l) { setenv(disable_env_var, "FALSE", 0); cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_true, "java", fake_load_config, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_true, "java", fake_load_config_svcname_unspecified, cr, fake_send_otlp_metric); require_env("test_auto_instrument_splunk_env_var_false_caps", "-javaagent:/foo/asdf.jar", "JAVA_TOOL_OPTIONS"); cmdline_reader_close(cr); } @@ -150,7 +167,7 @@ void test_auto_instrument_splunk_env_var_false_caps(logger l) { void test_auto_instrument_splunk_env_var_zero(logger l) { setenv(disable_env_var, "0", 0); cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_true, "java", fake_load_config, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_true, "java", fake_load_config_svcname_unspecified, cr, fake_send_otlp_metric); require_env("test_auto_instrument_splunk_env_var_zero", "-javaagent:/foo/asdf.jar", "JAVA_TOOL_OPTIONS"); cmdline_reader_close(cr); } @@ -161,12 +178,30 @@ void test_read_config(logger l) { char *logs[256]; int n = get_logs(l, logs); char *funcname = "test_read_config"; - require_equal_ints(funcname, 1, n); + require_equal_ints(funcname, 2, n); require_equal_strings(funcname, "reading config file: testdata/instrumentation.conf", logs[0]); + require_equal_strings(funcname, "generate_service_name not specified in config", logs[1]); require_equal_strings(funcname, "default.service", cfg.service_name); require_equal_strings(funcname, "/usr/lib/splunk-instrumentation/splunk-otel-javaagent.jar", cfg.java_agent_jar); require_equal_strings(funcname, "deployment.environment=test", cfg.resource_attributes); require_equal_strings(funcname, "true", cfg.disable_telemetry); + require_equal_strings(funcname, NULL, cfg.generate_service_name); + free_config(&cfg); +} + +void test_read_config_svcname(logger l) { + struct config cfg = {.java_agent_jar = NULL, .service_name = NULL}; + load_config(l, &cfg, test_config_path_svcname); + char *logs[256]; + int n = get_logs(l, logs); + char *funcname = "test_read_config_svcname"; + require_equal_ints(funcname, 1, n); + require_equal_strings(funcname, "reading config file: testdata/instrumentation-svcname.conf", logs[0]); + require_equal_strings(funcname, "default.service", cfg.service_name); + require_equal_strings(funcname, "/usr/lib/splunk-instrumentation/splunk-otel-javaagent.jar", cfg.java_agent_jar); + require_equal_strings(funcname, "deployment.environment=test", cfg.resource_attributes); + require_equal_strings(funcname, "true", cfg.disable_telemetry); + require_equal_strings(funcname, "true", cfg.generate_service_name); free_config(&cfg); } @@ -176,12 +211,13 @@ void test_read_config_missing_file(logger l) { char *logs[256]; int n = get_logs(l, logs); char *funcname = "test_read_config_missing_file"; - require_equal_ints(funcname, 5, n); + require_equal_ints(funcname, 6, n); require_equal_strings(funcname, "file not found: foo.txt", logs[0]); require_equal_strings(funcname, "service_name not specified in config", logs[1]); require_equal_strings(funcname, "java_agent_jar not specified in config", logs[2]); require_equal_strings(funcname, "resource_attributes not specified in config", logs[3]); require_equal_strings(funcname, "disable_telemetry not specified in config", logs[4]); + require_equal_strings(funcname, "generate_service_name not specified in config", logs[5]); require_equal_strings(funcname, NULL, cfg.service_name); require_equal_strings(funcname, NULL, cfg.java_agent_jar); free_config(&cfg); @@ -399,7 +435,7 @@ void test_dots_to_dashes(logger l) { void test_env_var_already_set(logger l) { setenv("JAVA_TOOL_OPTIONS", "hello", 0); cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_true, "java", fake_load_config, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_true, "java", fake_load_config_svcname_unspecified, cr, fake_send_otlp_metric); char *funcname = "test_env_var_already_set"; require_env(funcname, "hello", java_tool_options_var); cmdline_reader_close(cr); @@ -440,17 +476,18 @@ void test_is_legal_module(logger l) { } void test_eq_true(logger l) { - require_true("test_eq_true", eq_true("true")); - require_true("test_eq_true", eq_true("1")); - require_true("test_eq_true", eq_true("TRUE")); - require_false("test_eq_true", eq_true("false")); - require_false("test_eq_true", eq_true("0")); - require_false("test_eq_true", eq_true(NULL)); + require_true("test_eq_true", str_eq_true("true")); + require_true("test_eq_true", str_eq_true("1")); + require_true("test_eq_true", str_eq_true("TRUE")); + require_false("test_eq_true", str_eq_true("false")); + require_false("test_eq_true", str_eq_true("0")); + require_false("test_eq_true", str_eq_true(NULL)); } void test_enable_telemetry(logger l) { cmdline_reader cr = new_default_test_cmdline_reader(); - auto_instrument(l, access_check_true, "java", fake_load_config_disable_telemetry_not_specified, cr, fake_send_otlp_metric); + auto_instrument(l, access_check_true, "java", fake_load_config_disable_telemetry_not_specified, cr, + fake_send_otlp_metric); char *logs[256]; get_logs(l, logs); require_equal_strings("test_enable_telemetry", "sending metric", logs[2]); @@ -470,11 +507,19 @@ void fake_send_otlp_metric(logger log, char *service_name) { log_debug(log, "sending metric"); } -void fake_load_config(logger log, struct config *cfg, char *path) { +void fake_load_config_svcname_unspecified(logger log, struct config *cfg, char *path) { + cfg->java_agent_jar = strdup("/foo/asdf.jar"); + cfg->service_name = strdup("my.service"); + cfg->resource_attributes = strdup("myattr=myval"); + cfg->disable_telemetry = strdup("false"); +} + +void fake_load_config_generate_svcname_enabled(logger log, struct config *cfg, char *path) { cfg->java_agent_jar = strdup("/foo/asdf.jar"); cfg->service_name = strdup("my.service"); cfg->resource_attributes = strdup("myattr=myval"); cfg->disable_telemetry = strdup("false"); + cfg->generate_service_name = strdup("true"); } void fake_load_config_no_svcname(logger log, struct config *cfg, char *path) { diff --git a/instrumentation/src/test_main.h b/instrumentation/src/test_main.h index 70661447af9..b80957ed8b6 100644 --- a/instrumentation/src/test_main.h +++ b/instrumentation/src/test_main.h @@ -16,6 +16,8 @@ void test_auto_instrument_not_java(logger l); void test_auto_instrument_svc_name_in_config(logger l); +void test_auto_instrument_svc_name_explicitly_enabled(logger l); + void test_auto_instrument_no_svc_name_in_config(logger l); void test_auto_instrument_no_access(logger l); @@ -30,6 +32,8 @@ void test_auto_instrument_splunk_env_var_zero(logger l); void test_read_config(logger l); +void test_read_config_svcname(logger l); + void test_read_config_missing_file(logger l); void test_read_args_simple(logger l); @@ -90,7 +94,9 @@ void test_disable_telemetry(logger l); void fake_send_otlp_metric(logger log, char *service_name); -void fake_load_config(logger log, struct config *cfg, char *path); +void fake_load_config_svcname_unspecified(logger log, struct config *cfg, char *path); + +void fake_load_config_generate_svcname_enabled(logger log, struct config *cfg, char *path); void fake_load_config_no_svcname(logger log, struct config *cfg, char *path); diff --git a/instrumentation/src/test_utils.c b/instrumentation/src/test_utils.c index 6cd16032274..ad0a87aa090 100644 --- a/instrumentation/src/test_utils.c +++ b/instrumentation/src/test_utils.c @@ -3,6 +3,12 @@ #include "test_utils.h" #include "splunk.h" +void print_logs(char **logs, int n) { + for (int i = 0; i < n; ++i) { + printf("logs[%d]: %s\n", i, logs[i]); + } +} + void require_true(char *funcname, bool actual) { if (!actual) { printf("%s: require_true: got false\n", funcname); diff --git a/instrumentation/src/test_utils.h b/instrumentation/src/test_utils.h index ec837d402f1..99071c60c87 100644 --- a/instrumentation/src/test_utils.h +++ b/instrumentation/src/test_utils.h @@ -3,6 +3,8 @@ #include +void print_logs(char **logs, int n); + void require_true(char *funcname, bool actual); void require_false(char *funcname, bool actual); diff --git a/instrumentation/testdata/instrumentation-svcname.conf b/instrumentation/testdata/instrumentation-svcname.conf new file mode 100644 index 00000000000..551dfd23233 --- /dev/null +++ b/instrumentation/testdata/instrumentation-svcname.conf @@ -0,0 +1,5 @@ +service_name=default.service +java_agent_jar=/usr/lib/splunk-instrumentation/splunk-otel-javaagent.jar +resource_attributes=deployment.environment=test +disable_telemetry=true +generate_service_name=true