Skip to content

change config file to manage composition#291

Open
atoulme wants to merge 6 commits intoopen-telemetry:mainfrom
atoulme:config
Open

change config file to manage composition#291
atoulme wants to merge 6 commits intoopen-telemetry:mainfrom
atoulme:config

Conversation

@atoulme
Copy link
Copy Markdown
Contributor

@atoulme atoulme commented Mar 17, 2026

BREAKING CHANGE: To facilitate the composability of configuration needed in system packages, the configuration now moves under /etc/opentelemetry/injector/, with the main configuration file a /etc/opentelemetry/injector/injector.conf, merging with drop-in files under /etc/opentelemetry/injector/conf.d/.

The environment variable configuration file also moves from /etc/opentelemetry/default_auto_instrumentation_env.conf to /etc/opentelemetry/injector/default_env.conf.

The default auto-instrumentation agent paths are now empty. Paths are provided by per-language conf.d drop-in files installed by the respective language packages (e.g., opentelemetry-java-autoinstrumentation installs jvm.conf).

Users building custom container images must now explicitly configure the agent paths via conf.d files, the main configuration file, or environment variables.

This change has been extracted from #239.

@atoulme atoulme requested a review from a team as a code owner March 17, 2026 22:19
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Mar 17, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

Comment thread src/config.zig Outdated
@atoulme atoulme force-pushed the config branch 2 times, most recently from 9465b6d to e88ebc2 Compare March 18, 2026 06:14
Comment thread src/config.zig
Comment thread src/config.zig
@atoulme atoulme force-pushed the config branch 2 times, most recently from bbcfb7b to 1f73f24 Compare March 18, 2026 19:04
Copy link
Copy Markdown
Contributor

@mmanciop mmanciop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to avoid breaking changes like this, which likely will go unnoticed by users and will just result in apps not instrumenting (so, almost a silent failure)

Comment thread .chloggen/config.yaml
Comment thread src/config.zig
@mmanciop mmanciop dismissed their stale review April 13, 2026 20:28

Agreed in the SIG to this being a breaking change

Comment thread src/config.zig
Comment thread .chloggen/config-composable-directory.yaml Outdated
Comment on lines +25 to +28
The default auto-instrumentation agent paths are now empty. Paths are provided by per-language conf.d drop-in files
installed by the respective language packages (e.g., `opentelemetry-java-autoinstrumentation` installs `java.conf`).
Users building custom container images must now explicitly configure the agent paths via conf.d files, the main
configuration file, or environment variables.
Copy link
Copy Markdown
Contributor

@basti1302 basti1302 Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: We could maybe improve this a bit to give clear upgrade instructions:

Suggested change
The default auto-instrumentation agent paths are now empty. Paths are provided by per-language conf.d drop-in files
installed by the respective language packages (e.g., `opentelemetry-java-autoinstrumentation` installs `java.conf`).
Users building custom container images must now explicitly configure the agent paths via conf.d files, the main
configuration file, or environment variables.
Breaking change: On systems where custom settings have been configured in `/etc/opentelemetry/otelinject.conf`, these need to move to one of the new locations, for example `/etc/opentelemetry/injector/otelinject.conf`.
Breaking change: The in-code defaults for the auto-instrumentation agent paths are now empty. Paths must be provided via external configuration. Users who use the injector binary directly, but do not use the system packages published by this project, and who also have previously relied on the default auto-instrumentation agent paths must now explicitly configure the agent paths. This can be done either via the main configuration file (default: `/etc/opentelemetry/injector/otelinject.conf`), or conf.d-style files in the configuration directory (e.g. files under `/etc/opentelemetry/injector.d`), or via environment variables.

We should discuss #291 (comment) first though

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taken a stab at this in e5fd8da

Comment thread src/config.zig Outdated
Comment thread src/config.zig
@atoulme
Copy link
Copy Markdown
Contributor Author

atoulme commented Apr 15, 2026

+1 on Basti’s suggestion

@mmanciop
Copy link
Copy Markdown
Contributor

mmanciop commented Apr 15, 2026

So we now have two directories under /etc/opentelemetry, one is called injector and one is called injector.d. Is this idiomatic for a conf.d style approach?

Well, yes. See /etc/sudoers vs /etd/sudoers.d. However, tbh both the "sibling" approach like I implement, and the nested one that Basti proposes are both widespread.

I could even see how rename conf.d to something like injected-sdks.d or so, to better set expectations about what goes in that folder:

/etc/opentelemetry/injector/
├── injector.conf          ← main configuration file
├── default_env.conf
└── injected-sdks.d/       ← drop-in directory
    ├── 00-java.conf
    ├── 10-nodejs.conf
    └── 99-whatever.conf

@basti1302
Copy link
Copy Markdown
Contributor

Well, yes. See /etc/sudoers vs /etd/sudoers.d.

But /etc/sudoers is the main config file and its sibling /etd/sudoers.d the drop-in directory, right? This is different from what the PR currently does. I have not come across any wide-spread use of two sibling directories like we have here, /etc/opentelemetry/injector and /etc/opentelemetry/injector.d, which are both directories.

The current structure would look like this:

/etc/opentelemetry/injector/
├── injector.conf 
├── default_env.conf

/etc/opentelemetry/injector.d/
├── 00-java.conf
├── 10-nodejs.conf
└── 99-whatever.conf

I think the /etc/sudoers plus /etd/sudoers.d is not a good fit for us because we have two default files, the main config file and default_env.conf.

@basti1302
Copy link
Copy Markdown
Contributor

/etc/opentelemetry/injector/
├── injector.conf          ← main configuration file
├── default_env.conf
└── injected-sdks.d/       ← drop-in directory
    ├── 00-java.conf
    ├── 10-nodejs.conf
    └── 99-whatever.conf

That works for me. I would slightly prefer to not be over-specific with the name injected-sdks.d/ and use a generic name, because we cannot foresee all possible use cases for this directory. But I'm fine with both.

@mmanciop
Copy link
Copy Markdown
Contributor

/etc/opentelemetry/injector/
├── injector.conf          ← main configuration file
├── default_env.conf
└── injected-sdks.d/       ← drop-in directory
    ├── 00-java.conf
    ├── 10-nodejs.conf
    └── 99-whatever.conf

That works for me. I would slightly prefer to not be over-specific with the name injected-sdks.d/ and use a generic name, because we cannot foresee all possible use cases for this directory. But I'm fine with both.

Alright, then /etc/opentelemetry/injector/conf.d it is. We likely do not have enough clarity about the configurations to go as semantic as I proposed.

Copy link
Copy Markdown
Contributor

@basti1302 basti1302 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants