environment settings support for agents

[sultanowskii]

Resolves #82

[basti1302]

Just as a heads up, the next iteration of the injector will use setenv unconditionally to inject environment variables. In particular, it will no longer possess the knowledge whether the host process (the process we are injecting env vars into) is the Node.js runtime, a JVM, .NET or anything else.

With that in mind, I think implementing a mechanism that is supposed to only inject environment variables into specific runtimes (dotnet_auto_instrumentation_agent_env_path vs jvm_auto_instrumentation_agent_env_path) becomes unfeasible.

For context, the plan is to upstream the latest changes from here: https://github.com/dash0hq/dash0-operator/tree/main/images/instrumentation/injector

For the details why we plan to switch to a setenv based approach, you can read up on that here: https://github.com/dash0hq/dash0-operator/blob/main/images/instrumentation/injector/README.md#design-constraints

[sultanowskii]

As we've had discussed in the issue, I removed language-specific options

[sultanowskii]

Oh, by the way, @basti1302, I have a somewhat off-topic question: Why the format of config is not a yaml/toml/json/...?

I'm not against the current simple key-value approach, but was just wondering why not just go for yaml/toml/json/... format and take a handy 3rd-party parser instead of doing it by hand?

[basti1302]

Oh, by the way, @basti1302, I have a somewhat off-topic question: Why the format of config is not a yaml/toml/json/...?

I'm not against the current simple key-value approach, but was just wondering why not just go for yaml/toml/json/... format and take a handy 3rd-party parser instead of doing it by hand?

Good question. I don't have a strong opinion on that, to be honest. I think one motivation was to keep the injector binary as small and free of dependencies as possible. Adding a yaml or toml parser would probably be fine though.

There was also some prior art, even before I added the Zig injector code, there was a simpler injector in this repository, written in C, and it had the same key-value pair config file format https://github.com/open-telemetry/opentelemetry-injector/blob/d41f25f73143adbf0a3041aa467afcee31c851fc/tests/java/otelinject.conf, so I think I just went with that.

[sultanowskii]

Hi @basti1302 ! I think I mostly finished this one.

But I've run into a problem: for some reason, new integration test just won't pass for JVM and dotnet (passes for nodejs). After a little bit of debugging, it seems like JVM and dotnet processes just don't call our substituted getenv() at all.

To see for yourself, you can put something like:

print.printMessage("getenv({s}) called", .{name_z});

in getenv() at root.zig.

My suspicion is that dotnet and jvm load all env variables into memory at the startup, therefore all subsequent "System.getenv" calls don't actually perform a system call

Could you please take a look at it? How should we proceed?

---

upd: I've got an idea: setenv() all custom variables at the library initialization. I'll check this idea later

[basti1302]

After a little bit of debugging, it seems like JVM and dotnet processes just don't call our substituted getenv() at all.

That is absolutely correct, and it is exactly one of the reasons why we will be switching to a setenv based approach soon.

It is described in detail in the writeup I posted earlier:

For the details why we plan to switch to a setenv based approach, you can read up on that here: https://github.com/dash0hq/dash0-operator/blob/main/images/instrumentation/injector/README.md#design-constraints

Admittedly, this section comes way down from what I linked to. I hadn't thought about how this would affect your solution here, but in hindsight it is clear that this issue rears its ugly head here as well.

upd: I've got an idea: setenv() all custom variables at the library initialization. I'll check this idea later

That is the right idea, but am not sure if it is in scope for this PR.

We will switch to using setenv in general soon. The work has already been done downstream in the Dash0 injector: dash0hq/dash0-operator#490

This was a pretty large change. The plan was to upstream these changes into this project relatively soon. I am a bit worried that it will be harder to do this the more changes we introduce to the Zig code in this repository right now.

But maybe if you are using setenv only for the new custom variables from the new config file, it would not be such a large change, so it might be fine.

[sultanowskii]

There is no direct way to "setenv" (std.posix.setenv doesn't exist). So I figured the simplest approach here is to modify __environ and std.os.environ :)

[sultanowskii]

@basti1302 Please take a look at the PR :)

[sultanowskii]

Hey @basti1302 @atoulme, please review the PR :)

Excuse me if I'm bothering you, I'm quite close to the university assignment deadline, so I would really appreciate if you'd review it today or in the next few days, thanks!