feat: read config from file

[aldy505]

Closes #382

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

[aldy505]

@phacops do I need to write changelog or do you need to discuss this internally?

[aldy505]

@phacops @viglia any updates?

[williamdes]

Maybe @bmckerry knows where to your this PR for review?

[stayallive]

This would be very nice to see, it would make it very simple for self-hosted to for example store profiles in bucket storage instead of on disk!

[aldy505]

another gentle ping @phacops @viglia

[phacops]

Hi @aldy505,

Sorry for the delay! It seems like this PR will now require us to ship a config file while still being able to override it via environment variables. Why is this needed versus shipping a .env file to be loaded?

[aldy505]

@phacops It's much easier to do configuration by using file instead of environment variables for self-hosted. Also the current state of self-hosted configuration services are using configuration files instead of just bare environment variable (although environment variables are still in place).

• Relay: https://github.com/getsentry/self-hosted/blob/master/relay/config.example.yml
• Sentry: https://github.com/getsentry/self-hosted/blob/master/sentry/config.example.yml and https://github.com/getsentry/self-hosted/blob/master/sentry/sentry.conf.example.py
• Symbolicator: https://github.com/getsentry/self-hosted/blob/master/symbolicator/config.example.yml

Modifying docker-compose.yml file directly on the self-hosted repo to include the environment variables is somewhat discouraged because it's easier for the upgrade process to just do git checkout <VERSION> rather than doing a git diff to the git tag and apply the changes on your own.

[phacops]

I understand how it'll make this easier to have a separate file with your environment variables. I think we could still have this but without having to add anything to vroom.

We could add to the docker-compose.yml a env_file location (https://docs.docker.com/compose/environment-variables/set-environment-variables/#use-the-env_file-attribute) which would load the environment variables into the container and we'd have this file in the gitignore so it's easy to upgrade with git checkout (it won't track changes).

This way, we don't need to modify vroom and it still gives you what you want (which I agree, makes things simpler).

Here is what I'm suggesting: getsentry/self-hosted#2866

[aldy505]

I disagree with the env_file approach for self-hosted. As not everyone runs Sentry with the exact docker compose file from the self-hosted repository. Some, who are already comfortable on self-hosting Sentry might run it via Kubernetes, or other containerization platform. I know that Sentry don't officially support those, but I don't think closing the way other people might deploy Sentry stack is good for the community.

Obviously the changes doesn't requires everyone to immediately change to the config file. It's still backward compatible. But what I'm aiming for is that people can have the same configuration structure as with the other services, and it'll make self-hosted maintenance a lot easier for the OSPO team.

It's also a good way to provide documentation on how to configure vroom as a running service. Maintaining service configuration on the develop docs site is not friendly if they're using older releases, the website doesn't show the CalVer versions.

[phacops]

I don't really see what's easier with a separate YAML file.

We still have to load the configuration via Docker Compose (like https://github.com/getsentry/self-hosted/blob/master/docker-compose.yml#L309-L314) or on Kubernetes.

What I could do to make it more compatible is to source the .env file before we launch vroom. That way, it's compatible with every container platform. You will still have to load the file in the container except, with Docker compose, it'd be a single line.

As for the configuration documentation, is reading a YAML file so much better than a Go struct?

The only argument in the favor is doing this is "other services are doing it that way" but everything else seems more complicated and less integrated with existing tools.

[phacops]

I understand having the same way to configure all Sentry services is useful even if I don't agree.

I think we can merge your contribution if you fix the few comments I have. I'll make the necessary changes in self-hosted to accomodate that after.

[phacops]

Please remove the Occurrences prefix from the name.

[phacops]

Let's remove this since we only have 1 format so far.

[aldy505]

It can be json (default) or simplified (for human readable)

[phacops]

Let's remove the Profiling prefix from this too.

[phacops]

I would prefer not to hardcode the filename and make it an argument like other services.

[hubertdeng123]

Chiming in here, the reason why symbolicator and relay have these yml config files is due to a design choice for those services. If @phacops believes that this is not the best approach for configuring vroom and prefers loading in variables from the env, I don't think we should go forwards with using a yml config file for customizing self-hosted. Having said that, the proposed env_file solution requires docker compose v2.24.0+. Perhaps until we can enforce a min requirement of using docker compose v2.24.0+, we can just add the env variables needed for vroom in the .env file.

[phacops]

We're not going to implement reading configuration from files.