[cmd/opampsupervisor] Allow configuring Collector process execution

[evan-bradley]

Component(s)

cmd/opampsupervisor

Is your feature request related to a problem? Please describe.

When a user normally runs a Collector process, they can pass arguments, environment variables, and multiple configuration files. The Supervisor needs to support passing these.

Describe the solution you'd like

Implement the following config, as defined in the Supervisor design document.

collector:
  # Extra command line flags to pass to the Collector executable.
  args:

  # Extra environment variables to set when executing the Collector.
  env:
  
  # Path to optional local Collector config files to be merged with the
  # config provided by the OpAMP server.
  config_file: /etc/otelcol/config.yaml

For the config_file key, determine if multiple config files should be passed and update the design document if they should.

Describe alternatives you've considered

No response

Additional context

No response

[srikanthccv]

I started looking into this.

[srikanthccv]

Can we get rid of the config_file field? It's an optional field and its contents alone may not provide the effective config (we should use the opamp extension for this which is being tracked here) so why not just let it be part of the args because ultimately we provide it as args to the command. One can also use multiple config sources using the args.

[evan-bradley]

I see your argument there, but a Collector built with only upstream components actually only supports a few different flags, so it would be nice to allow configuring it entirely through YAML if at all possible. What if we made the option config_sources (we should allow providers other than files, e.g. HTTP) and provided a feature_gates key as well to configure feature gates? I think those are the two most common arguments passed to a Collector.

[srikanthccv]

There is one other flag I haven't personally seen many use --set. I guess we can start with config_sources and feature_gates. Does this look good?

collector:
  # Extra command line flags to pass to the Collector executable.
  args:

  # Extra environment variables to set when executing the Collector.
  env:
  
  config_sources:
    file:
      - /etc/collector/otlp.yaml
      - /etc/collector/internal-metrics.yaml
      - ...
    http:
      - 
      - 
    https:
      -
    env:
    yaml:
    s3:
  
  feature_gates:
    - +a
    - -b

Should we limit the config sources to the supported sources in core distribution?

[cwegener]

There is one other flag I haven't personally seen many use --set.

For reference, the way I have made use of the --set flag is when running the Agent collector without a supervisor in order to provide the FQDN of the OTLP Gateway Collector. Used that way, I can have a static config file that doesn't need to change, and I have the flexibility to use the same Agent collector package everywhere and provide the environment specific OTLP Gateway Collector FQDN via a variable during installation time of the Agent collector.

I believe that my example would still be required in an OpAMP managed Agent collector, but I would need to read the OpAMP spec again in order to confirm if OpAMP would potentially replace the use of --set in my example.

[srikanthccv]

You will still be able to do that using the args config option. The question is whether we want to make that a top-level config field for settings similar to other args or not. I don't have any concerns against adding it.

[cwegener]

You will still be able to do that using the args config option.

Good point. The lack of syntax highlight in the code block made me skip that args: line while reading ... 😉 (Note to self: Start including language identifiers in GH code blocks for syntax highlighting .. turns out syntax highlighting actually does have an effect 😁)

The question is whether we want to make that a top-level config field for settings similar to other args or not. I don't have any concerns against adding it.

I actually don't know. My first intuition is to not include it as a top-level key. And only add it later if it turns out that it would be useful to have as a top-level key once people start launching the collector process with the supervisor and we have gained some field experience.

[odubajDT]

If this issue is free, I would like to have a look at it

[evan-bradley]

@odubajDT It's yours.

@srikanthccv would still appreciate your review if you have time.

[evan-bradley]

Should we limit the config sources to the supported sources in core distribution?

Just now seeing this. I think we can just make it an arbitrary set of keys, we don't know what the Collector we're running will support and it will tell us if we pass an unsupported scheme.

[srikanthccv]

Thanks for picking it up @odubajDT

[odubajDT]

The PR should be ready for review

[cforce]

It would be great you add a test /docs/example how to use this with feature flags, especially for profiling 😃

[odubajDT]

It would be great you add a test /docs/example how to use this with feature flags, especially for profiling 😃

Added some more info into the docs, please have a look