Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How configuration would work in .NET #82

Open
lmolkova opened this issue Mar 27, 2024 · 7 comments
Open

How configuration would work in .NET #82

lmolkova opened this issue Mar 27, 2024 · 7 comments

Comments

@lmolkova
Copy link

lmolkova commented Mar 27, 2024

.NET configuration is a very common and convenient way to configure all sorts of .net apps.
It supports multiple configuration sources, but merges them in one common configuration object.

The common pattern is to define configuration for all components in a single file - appsettings.json and add/override things via other sources.

Given that it's the idiomatic way to do things in this ecosystem:

  • users will be surprised that they need to keep their otel config in a different file than the rest of their configuration.
  • they would likely be surprised they need to configure the file with the env var

The ideal outcome as I see it is that configuration story should leave a room for SDKs to decide how onboarding happens and whether it's done within a separate file or as a part of existing configuration system.
I assume there could be a similar ask to integrate otel configuration with spring config, microprofile config or something else.

In .NET it might look like a otel-specific section in the appsettings.yaml and no env var to opt into - this is pretty much how it could happen today, but I assume the schema will need to change to match new one defined in this repo.

@tsloughter
Copy link
Member

I believe that is already the case. At least that is how I planned it for Erlang/Elixir. We have similar in Erlang config. A Release contains at least 1 sys.config that configures the Applications within it. Today users configure OpenTelemetry with this file so it picks it up on boot and no user code is necessary to to start the SDK (allowing it to start before all other Applications in the Release).

But if the user passes a configuration file the plan was to ignore per-Application configuration on config since any sort of merging may be too painful and confusing.

@lmolkova
Copy link
Author

Thanks for the clarification, @tsloughter

Reading https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-configuration.md#other-mechanisms, I get an impression that 'other mechanisms' are unregulated by the spec.
I.e. the schema and configuration API are part of file configuration and don't apply to other mechanisms.

Also, the OTEP says that SDK MUST provide file option, which would probably be a rare case for .NET users.

It'd be great to have more clarity eventually.

@tsloughter
Copy link
Member

Right, the other mechanisms are unregulated.

Also, the OTEP says that SDK MUST provide file option, which would probably be a rare case for .NET users.

Oh, were you saying .NET didn't want to provide support for the file at all?

@lmolkova
Copy link
Author

lmolkova commented Mar 27, 2024

I'm not speaking on behalf of @open-telemetry/dotnet-maintainers and @open-telemetry/dotnet-approvers and would be interested in their opinion on this.

This is my personal perspective: as a .NET developer I would not benefit from separate config file and I don't think it should be required. But I'd really appreciate if .NET OTel json config structure matched common config properties defined here.

@CodeBlanch
Copy link
Member

We've had some talks in .NET but haven't made any decisions yet. We definitely want to support the config file. We also would like to support the standard .NET way of configuring things.

Here are my personal thoughts...

Today we support .NET's IConfiguration. The IConfiguration given to the app is composited from any number of sources. Typically, envvars, command-line, appSettings.json files. But you can add in other things like cloud config repositories. One simple thing we could do is pick up the open telemetry config file and just treat it as just another config source (transposing it into IConfiguration style). In theory, users who want to use the config file will be happy and users who want to stick to appSettings.json (or whatever) will also be able to do everything they could using the config file.

Now the config file itself has some dependency requirements. Like you can turn on OtlpExporter via the config. The .NET SDK alone can't do that without a dependency to the OtlpExporter assembly. So we might have to ship all of this as an add-on meta package (or something) which brings in all the assemblies for what is possible to configure. Or we try to dynamically load assemblies from the SDK for what is requested and blow up on startup for missing dependencies of things requested via config.

Anyway I feel we have options to explore and we should be able to do something to support the config file. Likely needs some prototyping.

@tsloughter
Copy link
Member

So we might have to ship all of this as an add-on meta package (or something) which brings in all the assemblies for what is possible to configure.

I think this is a problem for every implementation. Esp when third party propagators, id generators, exporters and eventually instrumentation libraries are taken into account. But a separate issue to this discussion.

But I will ask, shouldn't it already be an issue? What happens today if the user defines OTEL_TRACES_EXPORTER to OTLP?

@CodeBlanch
Copy link
Member

At the moment .NET doesn't support OTEL_TRACES_EXPORTER. The only component-configuring key we support is OTEL_TRACES_SAMPLER / OTEL_TRACES_SAMPLER_ARG because the SDK happens to have the samplers on hand. That was just added in the last week or so coincidentally.

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

No branches or pull requests

3 participants