opentelemetry-php
opentelemetry-php copied to clipboard
Configuration Interface and default configuration
We need to implement the Default SDK Configuration.
This has two pieces:
- The SDK MUST provide a programmatic interface for all configuration. This interface SHOULD be written in the language of the SDK itself. All other configuration mechanisms SHOULD be built on top of this interface. An example of this programmatic interface is accepting a well-defined struct on an SDK builder class. From that, one could build a CLI that accepts a file (YAML, JSON, TOML, ...) and then transforms into that well-defined struct consumable by the programatic interface.
- Establish a default configuration
I can take this on (after I finished the exporter factory issues, which however relate to this), and I guess no one is working this yet. Basically I solved a lot of he tasks involved already in the Symfony SDKBundle, though there need to be some adjustments.
Regarding the example from the spec:
Structs will only ship with PHP8.1 as first class citizens, so that's not an option, however in principle that's what the SF configuration does, only that it's a bit more flexible than structs themselve. I don't even see many places where structs would make sense, if we want to leave the SDK open for extension as mentioned somewhere else in the spec.
Creating a CLI is very simple , defacto standard is to use the Console Component, however the config can be cached anyway, so there might be no real need for that
I think the best way is to go with some dependeny injection container to build the SDK. Symfony of course uses it's own PSR11 (Container Interface) implementation which is very powerful, but I would not be set to this one. There are many PSR11 implementations, which all have their pros and cons, so I would rather evaluate appropriate options. However I think DI could also help remove some of the code smells currently in the library (like inline creation of instances for example, which is a problem with testing, debugging and SRP).
Another option for configuration might be thephpleague/config. It's a more simplistic approach than symfony/config and creates a data structure, which comes close to a 'well-defined struct'.
I like the look of thephpleague/config - having an organised/structured config does seem nice, and I like the dot-notation access.
Do we need to be able to inject more config providers into it, though? (config from environment, at least).
I mentioned in SIG today that I had a rough idea for config (which doesn't use any 3rd party libraries):
Config\ConfigProviderInterface.php
public function get(string $key): ?string;
public function has(string $key): bool;
Add some config providers
Config\Env.php
class Env implements ConfigProviderInterface
public function get($key) { return getenv($key) ?? null; }
public function has($key) { return getenv($key) !== false; }
Config\User.php
class User implements ConfigProviderInterface
public function __construct(array $data)
{
$this->data = $data;
}
//get + has methods...
Config.php
private array $providers;
public function __construct(array $userSettings = [])
{
//a priority order of providers to check for a given config, first match wins
$this->providers = [
new Config\User($userSettings), //provided from user, eg from yaml/json
new Config\Env(), //sourced from environment
new Config\Defaults(), //fallback/defaults settings
];
}
public function addProvider(ConfigProviderInterface $provider)
{
array_unshift($this->providers, $provider); //todo, priority?
}
public function get(string $key): ?string
{
foreach ($this->providers as $provider) {
if ($provider->has($key) { return $provider->get($key); }
}
return null;
}
I would then imagine that a factory might look like:
SomeFactory.php
public function create(Config $config)
{
$setting1 = $config->get('otel-key-one');
$setting2 = $config->get('otel-key-two');
return new Thing($setting1, $setting2);
}
and could be called from userland:
$config = new Config(Yaml::parse('settings.yml'));
$tracer (new TracerProviderFactory())->create($config)->getTracer();
@brettmc Looks good, I had something very similar in mind. I was also thinking of only using the underlying nette/schema, but if you like the the get/set/has access and the dot notation of thephpleague/config, that's totally fine with me. However you can take a look at nette/schema to get a better clue, what's possible with the Expect classes. Do we want to settle on thephpleague/config? I would then rework the exporter factory to expect a nette schema from the the exporters to know how to instantiate them. Something like this:
class ExporterFactory
{
[...]
public static register(string $exporterName, Schema $configSchema)
{
}
[...]
}
or even create a dediacted "Registry" the exporter can query. And I know a static registry is global scope, but there are exceptions to every strong opinion, and in this case I don't see any way around it for now. ;)
One thing however the SomeFactory could look like this, as I don't think the interim variables are really needed:
public function create(Config $config)
{
return new Thing(
$config->get('otel-key-one'),
$config->get('otel-key-two')
);
}
I also like
$config = new Config(Yaml::parse('settings.yml'));
however parsing yaml can be a bit costly in production, that's why smfony/config has a cache layer in between. So we could just later do the same and accept a PSR cache-implementation and/or create a little console application wich turns the yaml into an PHP array/"struct" in a file, as suggested in this issue's description (That's pretty easy with the Symfony/ConsoleComponent)
@tidal - I think nette/schema is really good just by itself - it supports nested objects, and has a processMultiple() method which can accept multiple data sets (ie, user-provided + from-environment).
re: costs of parsing yaml - I'm inclined to leave that for now, it feels like a user problem: "you can use whatever config mechanism you like, provided you can convert it to an array that matches this schema"...?
I'm happy to put together a draft PR, with struct that will probably look something like:
logger:
level: <string>
stream: <string>
...
trace:
exporter:
type: <string>
params:
endpoint: <string>
timeout: <int>
...
processor:
type: <string>
params:
endpoint: <string>
...
sampler:
type: <string>
params:
probability: <double>
metrics:
...
log:
...
I think
nette/schemais really good just by itself - it supports nested objects, and has aprocessMultiple()method which can accept multiple data sets (ie, user-provided + from-environment).
Ok. cool.
re: costs of parsing yaml - I'm inclined to leave that for now, it feels like a user problem: "you can use whatever config mechanism you like, provided you can convert it to an array that matches this schema"...?
Yes, I agree. This could be solved by instrumenting libraries/frameworks or the user. But we culd also offer one solution in the contrib repo later on.
I'm happy to put together a draft PR, with struct that will probably look something like:
Looks good, however keep in mind that according to the spec multiple exporters/processors should be configurable as well as custom implementations of sampler, processor and exporter. You can take a look at all the config options for the Symfony SDKBundle (but it's only tracing yet). This is a full configuration example, and you can find more examples here (These are the config files the integration tests are running aganist)
But please don't follow the config structure. There is much room for improvement, as this was my first pass at OTEL. I am happy to change the config structure of the SDK bundle according to what we will have in the SDK, and having configuration in the SDK will make a lot of code in the bundle redundant, and that's good!!! (In case you are wondering, things like "@my_span_processor" are pointers for Symfony's DI system, so you can simply ignore it)
@tidal - first draft of config: https://github.com/open-telemetry/opentelemetry-php/pull/498 It's just generating the config out to a stdClass, but it looks like we could cast that to a real class for better visibility. I've based the structure loosely on the symfony sdk bundle.
Discussed in SIG today to put this on hold, awaiting the outcome of https://github.com/open-telemetry/opentelemetry-specification/issues/2207
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
I added a draft proposal to support plain HTTP transport in OpAMP: https://github.com/open-telemetry/opamp-spec/pull/70
The primary use case I have is for Otel SDK remote configuration, so it would be great to know what folks here think about it.
Am I understanding the spec correctly in that it's saying all SDK configuration needs to be possible via PHP, and then anything else (e.g. environment variable handling) needs to be built on top of that?
Or is it saying something deeper or more broad? (the PR that introduced it is coy on motivations - https://github.com/open-telemetry/opentelemetry-specification/issues/390)
If not, it really feels like it's saying "don't accept just YAML or JSON for configuration, and not allow folks the option to change settings in code" to me. Like just don't exclusively offer a declarative option.
If that's the case I'm assuming there's not much more to be done with this? Like the PHP library doesn't do that anywhere today? (I assume making that programmatic config and env vars on top of it easier to use/more consistent would be a different issue)
@tidal - @brettmc mentioned in the SIG meeting you may have ideas for this? Talked about not making it required for beta
I went over what the java SIG does for configuration, and found that they don't have a way to provide input via a well-defined structure (xml/json/yaml). AFAICT, having an SDK builder is the extent of their "configuration interface" implementation. I think we only need to go that far as well, so I've started prototyping an SDK builder similar to java's.