Add new envoy tracing configuration

[jorgemarey]

Description

The current tracing configuration provided to envoy is deprecated. Should be replaced by the HttpConnectionManager.Tracing configuration. That also allows more custom config as allowing us to add custom tags. This is an example of a valid config JSON.

{
    "@type" : "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing",
    "provider" : {
        "name" : "envoy.tracers.zipkin",
        "typed_config" : {
            "@type" : "type.googleapis.com/envoy.config.trace.v3.ZipkinConfig",
            "collector_cluster" : "customcolector",
            "collector_endpoint" : "/api/v2/spans",
            "collector_endpoint_version" : "HTTP_JSON",
            "shared_span_context" : false
        }
    },
    "custom_tags" : [
        {
            "tag" : "custom_tag",
            "request_header" : {
                "name" : "x-tag",
                "default_value" : ""
            }
        },
        {
            "tag" : "alloc_id",
            "environment" : {
                "name" : "NOMAD_ALLOC_ID"
            }
        }
    ]
}

This PR adds a new ListenerTracingJSON that's configured in all listeners (minus in the exposed checks). The old tracing option is not removed for compatiblity.

PR Checklist

• [ ] updated test coverage
• [ ] external facing docs updated
• [ ] not a security concern

[jorgemarey]

Hi @kisunji, thanks for taking the time to look into this. I'll try to explain the reasoning behind this.

The envoy_public_listener_json should be able to define an override for tracing already; what's the reason for escape hatching the tracing configuration only?

When we were implementing tracing in our consul setup, we needed a way to add some nomad tags to get information related to deployment and also being able to send a custom header to the collector due to how our tracing platform works. In the specification it's used a custom header to inform the traceID, so all applications deployed are using that header instead of the common ones.

When looking how we could do this into consul, we saw the envoy_tracing_json configuration option, and thought that maybe we could add there the envoy tracing configuration, but digging into it, we found out that the needed tracing specification is old (and deprecated) and doesn't allow setting custom_tags. So I thought about doing a PR and updating the envoy tracing escape-hatch to be the new tracing provided by envoy instead of the old one, that's going to be removed at some point (in the PR the old remains for compatibility until it's removed in a future release)

Regarding the envoy_public_listener_json option. I guess it could work with that, but having to set this over a lot for services having each one several instances (we're deploying using nomad), I don't know how to work around the port_value for each one. Also that we would need to specify the same for all the configured upstreams with a envoy_listener_json (I don't know if that can be specified several times) so that outgoing connections are also traced. This makes the configuration a lot more complicated than this. With this we can just set this configuration in a proxy-defaults for all the services (or in a specific service but avoiding the public and upstreams listener configuration).

Currently it's hard to maintain a good encapsulation for envoy configurations and my concern would be that this sets a precedent for other "escape hatch for X field" PRs.

I get this, in most cases it should be enough with the already existent escape-hatchs, but this is created as a substitution of a deprecated feature providing more functionality.

[kisunji]

That sounds reasonable. Would you mind making the requested changes and adding a testcase in agent/xds/listeners_test.go? Something like:

{
	name: "custom-trace-listener",
	create: func(t testinf.T) *proxycfg.ConfigSnapshot {
		return proxycfg.TestConfigSnapshot(t, func(ns *structs.NodeService) {
			ns.Proxy.Config["protocol"] = "http"
			ns.Proxy.Config["envoy_listener_tracing_json"] = customTraceJSON(t)
		}, nil)
	},
},

If you pass -update to go test it should generate a golden file similar to agent/xds/testdata/listeners/connect-proxy-exported-to-peers.latest.golden

[kisunji]

This should go out in our next patch release of 1.13!