Revert "java zc: don't generate service name by default (open-telemet…

…ry#2250)" (open-telemetry#2266)

This reverts commit bba4914.

instrumentation/Dockerfile

@@ -7,6 +7,5 @@ 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

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 environment variables that will cause the
+executable starts normally. Otherwise, it attempts to set two 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,12 +35,6 @@ 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

instrumentation/src/config.c

@@ -29,9 +29,6 @@ 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) {
@@ -66,8 +63,6 @@ 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);
         }
     }
 }
@@ -77,7 +72,7 @@ void split_on_eq(char *string, struct kv *pair) {
     pair->v = string;
 }
 
-bool str_eq_true(char *v) {
+bool eq_true(char *v) {
     return v != NULL && !streq("false", v) && !streq("FALSE", v) && !streq("0", v);
 }
 

instrumentation/src/config.h

@@ -10,12 +10,11 @@ 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 str_eq_true(char *v);
+bool eq_true(char *v);
 
 void free_config(struct config *cfg);
 

instrumentation/src/splunk.c

@@ -33,8 +33,6 @@ 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.
@@ -87,22 +85,24 @@ void auto_instrument(
     }
 
     char service_name[MAX_CMDLINE_LEN] = "";
-    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);
+    if (cfg.service_name == NULL) {
+        get_service_name_from_cmdline(log, service_name, cr);
     } else {
-        log_debug(log, "service name generation disabled");
+        strncpy(service_name, cfg.service_name, MAX_CMDLINE_LEN);
     }
 
+    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 (str_eq_true(cfg.disable_telemetry)) {
+    if (eq_true(cfg.disable_telemetry)) {
         log_info(log, "disabling telemetry as per config");
     } else {
         send_otlp_metric_func(log, service_name);
@@ -111,14 +111,6 @@ 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);

instrumentation/src/test_main.c

@@ -8,7 +8,6 @@
 #include <string.h>
 
 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();
@@ -19,7 +18,6 @@ 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,
@@ -28,7 +26,6 @@ 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,
@@ -75,32 +72,18 @@ 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_svcname_unspecified, cr, fake_send_otlp_metric);
+    auto_instrument(l, access_check_true, "java", fake_load_config, 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, "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_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, "-javaagent:/foo/asdf.jar", java_tool_options_var);
     require_env(funcname, "my.service", otel_service_name_var);
+    cmdline_reader_close(cr);
 }
 
 void test_auto_instrument_no_svc_name_in_config(logger l) {
@@ -110,17 +93,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, "service name generation disabled", logs[0]);
+    require_equal_strings(funcname, "setting OTEL_SERVICE_NAME='foo'", 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_unset_env(funcname, otel_service_name_var);
+    require_env(funcname, "foo", 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_svcname_unspecified, cr, fake_send_otlp_metric);
+    auto_instrument(l, access_check_true, "foo", fake_load_config, cr, fake_send_otlp_metric);
     char *funcname = "test_auto_instrument_not_java";
     require_unset_env(funcname, java_tool_options_var);
     char *logs[256];
@@ -131,7 +114,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_svcname_unspecified, cr, fake_send_otlp_metric);
+    auto_instrument(l, access_check_false, "java", fake_load_config, 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";
@@ -143,31 +126,31 @@ 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_svcname_unspecified, cr, fake_send_otlp_metric);
+    auto_instrument(l, access_check_true, "java", fake_load_config, cr, fake_send_otlp_metric);
     require_unset_env("test_auto_instrument_splunk_env_var_true", "JAVA_TOOL_OPTIONS");
     cmdline_reader_close(cr);
 }
 
 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_svcname_unspecified, cr, fake_send_otlp_metric);
+    auto_instrument(l, access_check_true, "java", fake_load_config, 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);
 }
 
 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_svcname_unspecified, cr, fake_send_otlp_metric);
+    auto_instrument(l, access_check_true, "java", fake_load_config, 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);
 }
 
 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_svcname_unspecified, cr, fake_send_otlp_metric);
+    auto_instrument(l, access_check_true, "java", fake_load_config, 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);
 }
@@ -178,30 +161,12 @@ void test_read_config(logger l) {
     char *logs[256];
     int n = get_logs(l, logs);
     char *funcname = "test_read_config";
-    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, "reading config file: testdata/instrumentation.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);
 }
 
@@ -211,13 +176,12 @@ 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, 6, n);
+    require_equal_ints(funcname, 5, 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);
@@ -435,7 +399,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_svcname_unspecified, cr, fake_send_otlp_metric);
+    auto_instrument(l, access_check_true, "java", fake_load_config, cr, fake_send_otlp_metric);
     char *funcname = "test_env_var_already_set";
     require_env(funcname, "hello", java_tool_options_var);
     cmdline_reader_close(cr);
@@ -476,18 +440,17 @@ void test_is_legal_module(logger l) {
 }
 
 void test_eq_true(logger l) {
-    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));
+    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));
 }
 
 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]);
@@ -507,19 +470,11 @@ void fake_send_otlp_metric(logger log, char *service_name) {
     log_debug(log, "sending metric");
 }
 
-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) {
+void fake_load_config(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) {

instrumentation/src/test_main.h

@@ -16,8 +16,6 @@ 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);
@@ -32,8 +30,6 @@ 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);
@@ -94,9 +90,7 @@ void test_disable_telemetry(logger l);
 
 void fake_send_otlp_metric(logger log, char *service_name);
 
-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(logger log, struct config *cfg, char *path);
 
 void fake_load_config_no_svcname(logger log, struct config *cfg, char *path);
 

instrumentation/src/test_utils.c

@@ -3,12 +3,6 @@
 #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);