Command line arguments

[bilbof]

Command line arguments

[BlueHatbRit]

Hi @bilbof, thanks for the suggestion. I'd be up for someone adding environment variable support in general to our configuration. I think we could probably go a bit more light-weight than Yargs though.

That said, you should be able to provide the config file in docker using a mounted volume, does that solve your problem? It sounds like it might be a faster approach for you, it might also be preferrable as we have quite a lot of options that can be toggled as well.

[chranmat]

Hi, thanks for you reply again :)

Do you have another link to your fork comparison, the one above seems to show 0 changes to me and I can't see any branches on the repo that have the work. Have you pushed the changes up?

Oh, right. I pushed changes to my fork first, but then I re-forked it because I wanted add changes to another branch. Haven't pushed there yet. Also planning to add support for another backend and planned to have this in different branches. I will push it again and then give you a link.

Yeah you're right we can scrap quite a few files from it. I think when I made it originally it was just to provide a sample image rather than a fully fledged one that people would rely on in a production scenario. We can update that but it's probably something best done in a separate issue/PR. Again, I'd happily accept PR's for improving the quality here.

Have used quite some time to optimizing docker images so I will see what I can do here.

I'm actually in the process of moving house right now and I'm getting married next weekend so I don't have the time to submit some of these PR's myself or review them for a couple weeks but if they bring changes which make your life easier then please do submit then and I'll be happy to review them and work on them with you when I get back.

No hurry, just in experimental phase here.

[chranmat]

Hi @bilbof and @BlueHatbRit,

I recently started to look at a use case, because statsd is a supported metrics target for traefik. I agree with @bilbof in this case. When running an application in docker, options should be configurable.

Modern application design principles (12-factor) states that configuration should be stored in environment (variables). The reason for this is because application stacks are often built with compose files (infrastructure as code). Mapping additional configuration files from outside the container environment is not considered as a sustainable solution.

I have built several micro-service applications using Node.JS and Docker and this is very easy to implement. I implement environment variables for configuration by design.

The way I do it is to have one default configuration mapping file (i.e. config.js) containing a config object (like statsd do in a way) and mapping environment variables there. This file also contains all the default configuration.

Example:

const config = { server: process.env.server || 'example.server.address', port: process.env.port || 2501 }

Objects like "config.server" and "config.port" are used in the code. Statsd does this in a kind of way. Only difference is that "process.env" is not used, and default values is defined in the source code rather than in the config mapping from what i understand.

I would be a heavy job for me to refactor this, but I have made a fork that suits my current needs for testing:
master...chranmat:master

To not break anything I made a new file dockerConfig.js containing the environment variables. I've also modified Dockerfile a little bit to use dockerConfig.js instead of exampleConfig.js.

In addition I have removed "ls -al" (unnecessary), and removed the replacement of graphite url. The reason for this is because I also believe that a 12-factor app should be able to work out of the box with default config alone, and in combination in a application stack. Graphite is not necessarily the only possible backend and should not be a default requirement.

Because "backends" is a an array I understand its possible to use multiple backends. Split is used to make array from space devided string.

If you like my ideas and thoughts around this I can possibly make a pull request for this.

Best regards,
Christian

[BlueHatbRit]

Hey there, thanks for the feedback. I'm definitely not against a PR to support environment variables, they're always my preferred approach to things. It just wasn't when statsd was first created.

I'm curious why a config file doesn't work in a container situation, but I'm not against adding environment variables even if we don't have an answer to that. It sure would make life easier for people.

Feel free to submit a PR, I would ask that we try and keep the approach lightweight and don't break compatibility with our existing approach. As long as we can do that, I'm all for a PR to add environment variable support.

[chranmat]

An additional comment:

I can see that Dockerfile have a line like this:
COPY . /usr/src/app

This means that it copies the complete repository folder when building the docker image. There is many files that is not necessary to build and run the application. This gives the image a larger footprint and potential security risks. The recommended is to make docker images with a little footprint as possible. For security, also delete unnecessary files in end of build, even if it does not help for image footprint. Especially when web servers (i.e. apache, nginx) are used.

The way I do it is to make a folder src/ where all the source code exist. Then i just copy that dir + necessary files when building from docker.

[chranmat]

Thanks for quick reply 👍

I'm curious why a config file doesn't work in a container situation, but I'm not against adding environment variables even if we don't have an answer to that. It sure would make life easier for people.

It's not that it doesn't work, because it's fully possible to link in a file from outside the container. I have done that now to make it work before my modification. The case is that when you build an application stack that contains statsd as a service or component , you normally do this with a docker compose-file or kubernetes spec files. This can be considered as a recipe to bring the application stack up. In the compose file scenario this is often a single file to run the stack. This can also be considered as "infrastructure as code". From this single file you should be able to start up the stack in any Docker Swarm environment. It's not very convenient to when need to save several other additional files. Especially when deploying a single compose file to a remote environment, additional files will not work.

Feel free to submit a PR, I would ask that we try and keep the approach lightweight and don't break compatibility with our existing approach. As long as we can do that, I'm all for a PR to add environment variable support.

Yes, that's probably the challenge. I guess my current fork will break something since it does not contain graphite in default config? Also porting all configuration options will be difficult in regards of testing purposes?

[BlueHatbRit]

Hey thanks for the reply, I'm actually pretty familiar with containerised environments, that's why I was curious what problems you were hitting as I've not really found many issues running statsd with a config file in k8s and similar. Especially given that we have to do this already for other software such as nginx which is very heavy on the config files. I guess it boils down to this point.

It's not very convenient to when need to save several other additional files.

That seems pretty reasonable to me given the small size of our configuration, environment variables would knockout another config file for us to template which isn't a bad thing at all.

Do you have another link to your fork comparison, the one above seems to show 0 changes to me and I can't see any branches on the repo that have the work. Have you pushed the changes up?

I think we should be able to do it reasonably easily without making any breaking changes. Personally I'm less fussed by the idea of making it run out of the box without any configuration. While I agree that's a nice-to-have for something like a web app, statsd ultimately needs a backend configured and would be pretty useless without one. I think it's reasonable to expect people to add valid configuration to get it working in a way that's useful to them.

This means that it copies the complete repository folder when building the docker image.

Yeah you're right we can scrap quite a few files from it. I think when I made it originally it was just to provide a sample image rather than a fully fledged one that people would rely on in a production scenario. We can update that but it's probably something best done in a separate issue/PR. Again, I'd happily accept PR's for improving the quality here.

I'm actually in the process of moving house right now and I'm getting married next weekend so I don't have the time to submit some of these PR's myself or review them for a couple weeks but if they bring changes which make your life easier then please do submit then and I'll be happy to review them and work on them with you when I get back.

[chranmat]

Just a quick question. I see you are quite conservative regarding dependencies. I plan to make a backend for Azure Storage and dependencies would be convenient for that. What do you think?

[BlueHatbRit]

We don't ship that many backends inside statsd itself, only really a graphite one as that's our primary use case since day one really. I think it would be best to keep new ones outside of statsd so we don't have the weight of them in the main daemon and people aren't then getting too many more than they need bundled in.

I'd suggest spinning up new backends as separate packages such as this elasticsearch one. You're free to add it to the wiki for people to find if you like.

This would give you the freedom to build it as you see fit and loading in whatever packages would be most useful to you maintaining it.

[mrmarcsmith]

I would love to see ENV vars and command line options too! I pulled the docker image expected to be able to quickly configure it and was disappointed to find a config file was the only way.

[0xYao]

An additional comment:

I can see that Dockerfile have a line like this:
COPY . /usr/src/app

This means that it copies the complete repository folder when building the docker image. There is many files that is not necessary to build and run the application. This gives the image a larger footprint and potential security risks. The recommended is to make docker images with a little footprint as possible. For security, also delete unnecessary files in end of build, even if it does not help for image footprint. Especially when web servers (i.e. apache, nginx) are used.

The way I do it is to make a folder src/ where all the source code exist. Then i just copy that dir + necessary files when building from docker.

When building the docker image, doesn't .dockerignore excludes all the files specified when copying the directory?