NinesStack / sidecar

Gossip-based service discovery. Docker native, but supports non-container discovery, too.
MIT License
69 stars 7 forks source link

Fix Envoy example in Readme #36

Closed mihaitodor closed 6 years ago

mihaitodor commented 6 years ago

This fixes #29.

For now, gonitro/envoyproxy on Docker Hub has both tracing and non-tracing versions as separate tags, so we don't use the latest tag. We'll address this in the future.

relistan commented 6 years ago

Can you explain this further? I'm not a big fan of this.

mihaitodor commented 6 years ago

Oops, didn't mean to close this one @relistan. We don't have a latest tag for the gonitro/envoyproxy container because, the way it's set up right now, both the tracing version and the non-tracing version are under the same DockerHub repo. I don't like that and I'm thinking to create a separate DockerHub repo for tracing, when we'll actually enable that, I just haven't gotten to it yet. I wanted to make sure that the example in the readme here is usable until then. Does that make sense?

relistan commented 6 years ago

Why would the tracing one need a separate repo? IMO it should just be a config flag that can turn it on or off.

mihaitodor commented 6 years ago

True, I can do that via the s6 run script (because the envoy.yaml file needs to contain the tracing config). Good idea :)

sbz commented 6 years ago

Agreed we should only have one and use a runtime flag to enable tracing or not? As a Feature flags

relistan commented 6 years ago

@mihaitodor can't it just be a flag in envconfig? Why does the S6 script need to be involved?

mihaitodor commented 6 years ago

@relistan From what we have in the tracing version of Envoy, envoy.yaml needs to contain this stuff to enable tracing:

tracing:
  http:
    driver:
      type: zipkin
      config:
        collector_cluster: "zipkin-collector"
        collector_endpoint: "/api/v1/spans"

My idea was to have an env var that tells s6 which envoy.yaml to pass to envoy when it starts it up.

Otherwise, I'm open to suggestions.