[DNM] Zero-config C code to inject environment variables for Java and Node.js #3514

atoulme · 2023-08-18T18:55:13Z

** Work in progress **

This PR shows the current work in progress to adopt a zero-config solution that is minimally inserting environment variables into the runtime environment of Java and Node.js processes.

Please feel free to review and help strengthen this approach.

…m different files for java and nodejs

atoulme · 2023-08-18T18:56:15Z

instrumentation/src/main.c

+
+// TODO change to systemd drop in file paths
+static char *const env_var_file_java = "/etc/splunk/zeroconfig_java.conf";
+static char *const env_var_file_node = "/etc/splunk/zeroconfig_node.conf";


@jeffreyc-splunk would you have the path of the systemd drop-in env var files handy? Did you also separate them by language by any chance?

If we go with #3506, the deb/rpm packages can include sample systemd drop-in files in /usr/lib/splunk-instrumentation/examples/systemd/ for each language (see https://github.com/signalfx/splunk-otel-collector/pull/3506/files#diff-71d510eec30ef5d3556193068fc6726c24202c439f5bd9f25989cb511ff113f9 for java). We can then add a sample splunk-otel-js.conf once we add support for the node.js agent. These sample files can be used for manual installations.

On the other hand, I'm thinking that the installers will automatically create the actual drop-in file to be loaded by systemd, for example /usr/lib/systemd/system.conf.d/splunk-otel-auto-instrumentation.conf, based on the installation options and defaults (see https://github.com/signalfx/splunk-otel-collector/pull/3506/files#diff-4dca014cfa3f56017a31c6a9e698de416a5bcc9cf6450a8ef9d088295f19ca98 for the test config).

Side note: A reoccurring issue we have been noticing with being swappable with upstream auto-instrumentation solutions vs Splunk solutions is Splunk file paths are unique to Splunk vs what upstream uses. I don't think this issue should be addressed in this PR, but it is worth noting for future compatibility.

We can make those configurable via macros. I'll do that.

atoulme · 2023-08-18T18:56:51Z

instrumentation/src/main.c

+    "OTEL_RESOURCE_ATTRIBUTES",
+    "SPLUNK_PROFILER_ENABLED",
+    "SPLUNK_PROFILER_MEMORY_ENABLED",
+    "SPLUNK_METRICS_ENABLED",


this list needs to be completed. It doesn't represent the whole set yet.

For java, would we need to support all of the splunk-specific env vars?

Any from otel java and otel sdk not included in the above list?

yes we're missing quite a few.

Quick question for clarification: Correct me if I am wrong, wouldn't the allowed env variables be injected from the language specific drop-in files and not the constant ones defined here?

This allows us to check the file contents, and ensure that only the keys we allow here are injected in the environment of processes.

Will this library read JAVA_TOOL_OPTIONS and NODE_OPTIONS from the conf files, or are you expecting them to be defined somewhere else? For systemd, we can define them directly in the drop-in file(s), but should they be added to the list for the preloader method?

If we are allowing arbitrary input I think instrumentation checksum validation from .rodata should be required. This will add complexity and overhead but the benefit is that we'd only respect known target values.

I would just read all the env vars from a file. We can harden our stance around specific instrumentation files being placed in specific places and matching checksums.

jeffreyc-splunk · 2023-08-18T19:54:31Z

If we go with these changes, should we just use the LD_PRELOAD env var for systemd-only instrumentation? So instead of setting the env vars directly in a systemd config like:

[Manager]
DefaultEnvironment="JAVA_TOOL_OPTIONS=-javaagent:/usr/lib/splunk-instrumentation/splunk-otel-javaagent.jar"
DefaultEnvironment="OTEL_EXPORTER_OTLP_ENDPOINT=http://1.2.3.4:5678"
DefaultEnvironment="OTEL_RESOURCE_ATTRIBUTES=deployment.environment=my_deployment_environment"
DefaultEnvironment="OTEL_SERVICE_NAME=my_service_name"
DefaultEnvironment="SPLUNK_PROFILER_ENABLED=true"

The systemd config would just be something like:

[Manager]
DefaultEnvironment="LD_PRELOAD=/usr/lib/splunk-instrumentation/libsplunk.so"

And libsplunk.so would just read/inject the env vars from a separate config file. This would simplify configuration for the installers since we'd be dealing with a single config file for both systemd and /etc/ld.so.preload methods.

Or do we need an alternative configuration that completely excludes preloading libsplunk.so?

atoulme · 2023-08-18T20:29:00Z

That second approach would allow to run everything through the preload function, which we can test for. I'm ok with either way but I tend to the latter option because we can then format the env var file into a simple file to source from.

instrumentation/src/main.c

jeffreyc-splunk · 2023-08-24T15:53:25Z

That second approach would allow to run everything through the preload function, which we can test for. I'm ok with either way but I tend to the latter option because we can then format the env var file into a simple file to source from.

Instead of the systemd drop-in method, another option could be to include and install a simple systemd environment generator script to read and inject env vars from a shared config file. This would bypass libsplunk.so while still allowing us to manage and source env vars from a single config file with either the preloader or systemd-only methods.

instrumentation/tests/java/Dockerfile

instrumentation/tests/nodejs/Dockerfile

Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com>

… Node.js (#3514) * Remove sending a metric as part of auto-instrumentation (#3482) * refactor the code to inject allowed env vars * latest after pair programming * latest after review and pairing * only inject env vars for java and nodejs programs. Apply env vars from different files for java and nodejs * remove docker interactive and tty flags * wip * use arch var to set the name of the file to copy * make tests pass on arm64 * try to support arm * build for the proper platform * fix package build * review with Jason, test for env vars overrides * Make ALLOWED_ENV_VARS a single string defined through the preprocessor * Update instrumentation/tests/java/Dockerfile Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com> * load preload file same way as java test * move file locations to macros * use n functions to handle strings when possible --------- Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com>

* [DNM] Zero-config C code to inject environment variables for Java and Node.js (#3514) * Remove sending a metric as part of auto-instrumentation (#3482) * refactor the code to inject allowed env vars * latest after pair programming * latest after review and pairing * only inject env vars for java and nodejs programs. Apply env vars from different files for java and nodejs * remove docker interactive and tty flags * wip * use arch var to set the name of the file to copy * make tests pass on arm64 * try to support arm * build for the proper platform * fix package build * review with Jason, test for env vars overrides * Make ALLOWED_ENV_VARS a single string defined through the preprocessor * Update instrumentation/tests/java/Dockerfile Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com> * load preload file same way as java test * move file locations to macros * use n functions to handle strings when possible --------- Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com> * Add systemd sample file to splunk-otel-auto-instrumention (#3506) * Add systemd sample file to splunk-otel-auto-instrumention * Update manual instructions * Include default /etc/splunk/zeroconfig_java.conf in deb/rpm packages * Update tests for new libsplunk.so * Run auto instrumentation workflow for the zero-config-dev branch * Include splunk-otel-js in auto instrumentation deb/rpm (#3540) * Add systemd sample file to splunk-otel-auto-instrumention * Update tests for new libsplunk.so * Include splunk-otel-js in auto instrumentation deb/rpm * Include default /etc/splunk/zeroconfig_node.conf in deb/rpm packages * Add tests for express instrumentation (#3566) * Update installer script for systemd auto instrumentation (#3536) * Rename zeroconfig config files (#3682) * Update ZC docs and tests for manual installation/configuration (#3700) * Update docs * Update splunk-otel-js to v2.4.2 * Combine sample systemd config files * Install splunk-otel-js globally for tests * Update deps installation for node tests * Update splunk-otel-js to v2.4.4 * Update linux-manual.md * Update test for centos/oraclelinux 7 arm64 --------- Co-authored-by: Anna U <104845867+aurbiztondo-splunk@users.noreply.github.com> * Update otlp endpoint for linux installer script (#3761) * Update otlp endpoint for linux installer script * remove 0.0.0.0 * Add debian bookworm for instrumentation tests * Update CHANGELOG.md --------- Co-authored-by: Antoine Toulme <antoine@lunar-ocean.com> Co-authored-by: Anna U <104845867+aurbiztondo-splunk@users.noreply.github.com>

atoulme and others added 5 commits August 15, 2023 23:13

Remove sending a metric as part of auto-instrumentation (#3482)

9d72d7e

refactor the code to inject allowed env vars

aff70cc

latest after pair programming

90a5b7a

latest after review and pairing

37f39fa

only inject env vars for java and nodejs programs. Apply env vars fro…

bd718d9

…m different files for java and nodejs

atoulme commented Aug 18, 2023

View reviewed changes

atoulme added 7 commits August 18, 2023 13:30

remove docker interactive and tty flags

d5a09e7

wip

429e2fa

use arch var to set the name of the file to copy

b3339ae

make tests pass on arm64

2865889

try to support arm

b52befc

build for the proper platform

490a1a8

fix package build

016e1a6

pjanotti reviewed Aug 21, 2023

View reviewed changes

instrumentation/src/main.c Outdated Show resolved Hide resolved

atoulme added 2 commits August 22, 2023 18:23

review with Jason, test for env vars overrides

b0513e6

Make ALLOWED_ENV_VARS a single string defined through the preprocessor

0d68beb

atoulme force-pushed the zeroconfc branch from ec65a32 to 0d68beb Compare August 24, 2023 04:32

rmfitzpatrick reviewed Aug 24, 2023

View reviewed changes

instrumentation/src/main.c Show resolved Hide resolved

rmfitzpatrick reviewed Aug 24, 2023

View reviewed changes

instrumentation/src/main.c Show resolved Hide resolved

jeffreyc-splunk reviewed Aug 28, 2023

View reviewed changes

instrumentation/tests/java/Dockerfile Outdated Show resolved Hide resolved

instrumentation/tests/nodejs/Dockerfile Outdated Show resolved Hide resolved

atoulme and others added 4 commits August 28, 2023 13:38

Update instrumentation/tests/java/Dockerfile

a41f5ab

Co-authored-by: Jeff Cheng <83052155+jeffreyc-splunk@users.noreply.github.com>

load preload file same way as java test

e896135

move file locations to macros

c660e2b

use n functions to handle strings when possible

d86416e

atoulme changed the base branch from main to zero-config-dev August 28, 2023 21:40

atoulme marked this pull request as ready for review August 28, 2023 21:40

atoulme requested review from a team as code owners August 28, 2023 21:40

atoulme merged commit bfd4f54 into zero-config-dev Aug 28, 2023

delete-merged-branch bot deleted the zeroconfc branch August 28, 2023 21:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DNM] Zero-config C code to inject environment variables for Java and Node.js #3514

[DNM] Zero-config C code to inject environment variables for Java and Node.js #3514

atoulme commented Aug 18, 2023

atoulme Aug 18, 2023

jeffreyc-splunk Aug 18, 2023

jvoravong Aug 28, 2023

atoulme Aug 28, 2023

atoulme Aug 28, 2023

atoulme Aug 18, 2023

jeffreyc-splunk Aug 18, 2023

atoulme Aug 18, 2023

aunshc Aug 21, 2023

atoulme Aug 21, 2023

jeffreyc-splunk Aug 21, 2023

rmfitzpatrick Aug 24, 2023

atoulme Aug 24, 2023 •

edited

Loading

jeffreyc-splunk commented Aug 18, 2023 •

edited

Loading

atoulme commented Aug 18, 2023

jeffreyc-splunk commented Aug 24, 2023

[DNM] Zero-config C code to inject environment variables for Java and Node.js #3514

[DNM] Zero-config C code to inject environment variables for Java and Node.js #3514

Conversation

atoulme commented Aug 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atoulme Aug 24, 2023 • edited Loading

Choose a reason for hiding this comment

jeffreyc-splunk commented Aug 18, 2023 • edited Loading

atoulme commented Aug 18, 2023

jeffreyc-splunk commented Aug 24, 2023

atoulme Aug 24, 2023 •

edited

Loading

jeffreyc-splunk commented Aug 18, 2023 •

edited

Loading