openzipkin-attic / docker-zipkin

Docker images for OpenZipkin
Apache License 2.0
689 stars 330 forks source link

Allow command line arguments on the image #227

Closed devinsba closed 4 years ago

devinsba commented 4 years ago

This supports setting the number of spans ala --max-spans=N when launching the docker container

Reported in gitter by @ostera

anuraaga commented 4 years ago

@adriancole can confirm, but if I'm not mistaken, we intentionally prefer users to use environment variables for configuration to decouple configuration of zipkin server from config file format / argument parsing.

Should we instead (or at least in addition) add an environment variable knob here?

https://github.com/openzipkin/zipkin/blob/38720bcab29010e38fdb763d31d6768a43fac334/zipkin-server/src/main/resources/zipkin-server-shared.yml#L85

devinsba commented 4 years ago

Yeah, that makes sense. I was looking shortest route honestly since modifying that file in zipkin will require a release but I'm open to either

codefromthecrypt commented 4 years ago

I don't think we have any args that can't be converted to system parameters.. maybe we should put an example of changing the span count, just to remind folks what JAVA_OPTS is for?

codefromthecrypt commented 4 years ago

actually we should probably map the in-mem param to an ENV variable anyway, to clarify what is supported vs undocumented. Usually our primary things are mapped to ENV

anuraaga commented 4 years ago

Sounds like all of us forgot about JAVA_OPTS, example sounds good :)

devinsba commented 4 years ago

Yup, closing in favor of the env variable PR and hopefully one to add docs about JAVA_OPTS