Add support for configuring LightStep as a tracing backend (istio#9270)

* Add LightStep configuration - Validate LightStep ProxyConfig options - Clean up formatting of envoy_bootstrap by using a single indentation scheme - Update bootstrap golden files and add new ones for tracing - Update bootstrap tests to check new tracing options * Add root cacert.pem into proxy containers * Update bootstrap config test * Update golden files * Move tracer configuration up a level (replacing old one) * Remove lightstep as default proxy tracer * Set default proxy.tracer to zipkin * Make fixes to yaml - Make istio-remote configmap set the same default zipkin address as istio configmap - Explicitly check that tracer == "zipkin" instead of always defaulting to it * Replace check for global.remoteZipkinAddress with global.proxy.tracer * Enable mounting a custom cacert file * Remove default cacert.pem * Fix issues after rebase
biefy · Oct 23, 2018 · d3eed9a · d3eed9a
1 parent b6e7de7
commit d3eed9a
Show file tree

Hide file tree

Showing 67 changed files with 1,749 additions and 769 deletions.
diff --git a/install/kubernetes/helm/istio-remote/templates/configmap.yaml b/install/kubernetes/helm/istio-remote/templates/configmap.yaml
@@ -17,7 +17,7 @@ data:
     disablePolicyChecks: {{ .Values.global.disablePolicyChecks }}
   {{- end }}  
 
-  {{- if .Values.global.remoteZipkinAddress }}
+  {{- if .Values.global.proxy.tracer }}
     # Set enableTracing to false to disable request tracing.
     enableTracing: {{ .Values.global.enableTracing }}
   {{- end }}  
@@ -76,12 +76,29 @@ data:
       # Set concurrency to a specific number to control the number of Proxy worker threads.
       # If set to 0 (default), then start worker thread for each CPU thread/core.
       concurrency: {{ .Values.global.proxy.concurrency }}
-
-    {{- if .Values.global.remoteZipkinAddress }}
       #
-      # Zipkin trace collector
-      zipkinAddress: {{ .Values.global.remoteZipkinAddress }}:9411
-    {{- end }}
+      tracing:
+      {{- if eq .Values.global.proxy.tracer "lightstep" }}
+        lightstep:
+          # Address of the LightStep Satellite pool
+          address: {{ .Values.global.tracer.lightstep.address }}
+          # Access Token used to communicate with the Satellite pool
+          accessToken: {{ .Values.global.tracer.lightstep.accessToken }}
+          # Whether communication with the Satellite pool should be secure
+          secure: {{ .Values.global.tracer.lightstep.secure }}
+          # Path to the file containing the cacert to use when verifying TLS
+          cacertPath: {{ .Values.global.tracer.lightstep.cacertPath }}
+      {{- else if eq .Values.global.proxy.tracer "zipkin" }}
+        zipkin:
+          # Address of the Zipkin collector
+        {{- if .Values.global.tracer.zipkin.address }}
+          address: {{ .Values.global.tracer.zipkin.address }}
+        {{- else if .Values.global.remoteZipkinAddress }}
+          address: {{ .Values.global.remoteZipkinAddress }}:9411
+        {{- else }}
+          address: zipkin.{{ .Release.Namespace }}:9411
+        {{- end }}
+      {{- end }}
 
     {{- if .Values.global.proxy.envoyStatsd.enabled }}
       #

diff --git a/install/kubernetes/helm/istio-remote/templates/sidecar-injector-configmap.yaml b/install/kubernetes/helm/istio-remote/templates/sidecar-injector-configmap.yaml
@@ -99,9 +99,17 @@ data:
         - {{ "[[ formatDuration .ProxyConfig.ParentShutdownDuration ]]" }}
         - --discoveryAddress
         - {{ "[[ or (index .ObjectMeta.Annotations \"sidecar.istio.io/discoveryAddress\") .ProxyConfig.DiscoveryAddress ]]" }}
-      {{- if .Values.global.zipkinAddress }}
+      {{- if eq .Values.global.proxy.tracer "lightstep" }}
+        - --lightstepAddress
+        - {{ "[[ .ProxyConfig.GetTracing.GetLightstep.GetAddress ]]" }}
+        - --lightstepAccessToken
+        - {{ "[[ .ProxyConfig.GetTracing.GetLightstep.GetAccessToken ]]" }}
+        - --lightstepSecure={{ "[[ .ProxyConfig.GetTracing.GetLightstep.GetSecure ]]" }}
+        - --lightstepCacertPath
+        - {{ "[[ .ProxyConfig.GetTracing.GetLightstep.GetCacertPath ]]" }}
+      {{- else if eq .Values.global.proxy.tracer "zipkin" }}
         - --zipkinAddress
-        - {{ "[[ .ProxyConfig.ZipkinAddress ]]" }}
+        - {{ "[[ .ProxyConfig.GetTracing.GetZipkin.GetAddress ]]" }}
       {{- end }}
         - --connectTimeout
         - {{ "[[ formatDuration .ProxyConfig.ConnectTimeout ]]" }}

diff --git a/install/kubernetes/helm/istio-remote/values.yaml b/install/kubernetes/helm/istio-remote/values.yaml
@@ -111,10 +111,8 @@ global:
       host: # example: statsd-svc
       port: # example: 9125
 
-    # This controls the stats collection for proxies. To disable stats
-    # collection, set the prometheusPort to 0.
-    stats:
-      prometheusPort: 15090
+    # Specify which tracer to use. One of: lightstep, zipkin
+    tracer: "zipkin"
 
   proxy_init:
     # Base name for the proxy_init container, used to configure iptables.
@@ -140,10 +138,25 @@ global:
   # EnableTracing sets the value with same name in istio config map, requires pilot restart to take effect.
   enableTracing: true
 
+  # Configuration for each of the supported tracers
   tracer:
-    # Host:Port for reporting trace data in zipkin format. If not specified, will default to
-    # zipkin service (port 9411) in the same namespace as the other istio components.
-    zipkinAddress:
+    # Configuration for envoy to send trace data to LightStep.
+    # Disabled by default.
+    # address: the <host>:<port> of the satellite pool
+    # accessToken: required for sending data to the pool
+    # secure: specifies whether data should be sent with TLS
+    # cacertPath: the path to the file containing the cacert to use when verifying TLS. If secure is true, this is
+    #   required. If a value is specified then a secret called "lightstep.cacert" must be created in the destination
+    #   namespace with the key matching the base of the provided cacertPath and the value being the cacert itself.
+    lightstep:
+      address: ""                # example: lightstep-satellite:443
+      accessToken: ""            # example: abcdefg1234567
+      secure: true               # example: true|false
+      cacertPath: ""             # example: /etc/lightstep/cacert.pem
+    zipkin:
+      # Host:Port for reporting trace data in zipkin format. If not specified, will default to
+      # zipkin service (port 9411) in the same namespace as the other istio components.
+      address: ""
 
   # Default mtls policy. If true, mtls between services will be enabled by default.
   mtls:

diff --git a/install/kubernetes/helm/istio/templates/configmap.yaml b/install/kubernetes/helm/istio/templates/configmap.yaml
@@ -83,11 +83,25 @@ data:
       # If set to 0 (default), then start worker thread for each CPU thread/core.
       concurrency: {{ .Values.global.proxy.concurrency }}
       #
-      # Zipkin trace collector
-      {{- if .Values.global.tracer.zipkinAddress }}
-      zipkinAddress: {{ .Values.global.tracer.zipkinAddress }}
-      {{- else }}
-      zipkinAddress: zipkin.{{ .Release.Namespace }}:9411
+      tracing:
+      {{- if eq .Values.global.proxy.tracer "lightstep" }}
+        lightstep:
+          # Address of the LightStep Satellite pool
+          address: {{ .Values.global.tracer.lightstep.address }}
+          # Access Token used to communicate with the Satellite pool
+          accessToken: {{ .Values.global.tracer.lightstep.accessToken }}
+          # Whether communication with the Satellite pool should be secure
+          secure: {{ .Values.global.tracer.lightstep.secure }}
+          # Path to the file containing the cacert to use when verifying TLS
+          cacertPath: {{ .Values.global.tracer.lightstep.cacertPath }}
+      {{- else if eq .Values.global.proxy.tracer "zipkin" }}
+        zipkin:
+          # Address of the Zipkin collector
+        {{- if .Values.global.tracer.zipkin.address }}
+          address: {{ .Values.global.tracer.zipkin.address }}
+        {{- else }}
+          address: zipkin.{{ .Release.Namespace }}:9411
+        {{- end }}
       {{- end }}
 
     {{- if .Values.global.proxy.envoyStatsd.enabled }}

diff --git a/install/kubernetes/helm/istio/templates/sidecar-injector-configmap.yaml b/install/kubernetes/helm/istio/templates/sidecar-injector-configmap.yaml
@@ -105,8 +105,18 @@ data:
         - {{ "[[ formatDuration .ProxyConfig.ParentShutdownDuration ]]" }}
         - --discoveryAddress
         - {{ "[[ .ProxyConfig.DiscoveryAddress ]]" }}
+      {{- if eq .Values.global.proxy.tracer "lightstep" }}
+        - --lightstepAddress
+        - {{ "[[ .ProxyConfig.GetTracing.GetLightstep.GetAddress ]]" }}
+        - --lightstepAccessToken
+        - {{ "[[ .ProxyConfig.GetTracing.GetLightstep.GetAccessToken ]]" }}
+        - --lightstepSecure={{ "[[ .ProxyConfig.GetTracing.GetLightstep.GetSecure ]]" }}
+        - --lightstepCacertPath
+        - {{ "[[ .ProxyConfig.GetTracing.GetLightstep.GetCacertPath ]]" }}
+      {{- else if eq .Values.global.proxy.tracer "zipkin" }}
         - --zipkinAddress
-        - {{ "[[ .ProxyConfig.ZipkinAddress ]]" }}
+        - {{ "[[ .ProxyConfig.GetTracing.GetZipkin.GetAddress ]]" }}
+      {{- end }}
         - --connectTimeout
         - {{ "[[ formatDuration .ProxyConfig.ConnectTimeout ]]" }}
       {{- if .Values.global.proxy.envoyStatsd.enabled }}
@@ -200,12 +210,23 @@ data:
         - mountPath: /var/run/sds
           name: sds-uds-path
         {{- end }}
+        {{ if and (eq .Values.global.proxy.tracer "lightstep") .Values.global.tracer.lightstep.cacertPath }}
+        - mountPath: {{ "[[ directory .ProxyConfig.GetTracing.GetLightstep.GetCacertPath ]]" }}
+          name: lightstep-certs
+          readOnly: true
+        {{- end }}
       volumes:
       {{ if .Values.global.sdsEnabled }}
       - name: sds-uds-path
         hostPath:
           path: /var/run/sds
       {{- end }}
+      {{ if and (eq .Values.global.proxy.tracer "lightstep") .Values.global.tracer.lightstep.cacertPath }}
+      - name: lightstep-certs
+        secret:
+          optional: true
+          secretName: lightstep.cacert
+      {{- end }}
       - emptyDir:
           medium: Memory
         name: istio-envoy

diff --git a/install/kubernetes/helm/istio/values.yaml b/install/kubernetes/helm/istio/values.yaml
@@ -181,6 +181,9 @@ global:
     stats:
       prometheusPort: 15090
 
+    # Specify which tracer to use. One of: lightstep, zipkin
+    tracer: "zipkin"
+
   proxy_init:
     # Base name for the proxy_init container, used to configure iptables.
     image: proxy_init
@@ -205,10 +208,26 @@ global:
   # EnableTracing sets the value with same name in istio config map, requires pilot restart to take effect.
   enableTracing: true
 
+  # Configuration for each of the supported tracers
   tracer:
-    # Host:Port for reporting trace data in zipkin format. If not specified, will default to
-    # zipkin service (port 9411) in the same namespace as the other istio components.
-    zipkinAddress:
+    # Configuration for envoy to send trace data to LightStep.
+    # Disabled by default.
+    # address: the <host>:<port> of the satellite pool
+    # accessToken: required for sending data to the pool
+    # secure: specifies whether data should be sent with TLS
+    # cacertPath: the path to the file containing the cacert to use when verifying TLS. If secure is true, this is
+    #   required. If a value is specified then a secret called "lightstep.cacert" must be created in the destination
+    #   namespace with the key matching the base of the provided cacertPath and the value being the cacert itself.
+    #
+    lightstep:
+      address: ""                # example: lightstep-satellite:443
+      accessToken: ""            # example: abcdefg1234567
+      secure: true               # example: true|false
+      cacertPath: ""             # example: /etc/lightstep/cacert.pem
+    zipkin:
+      # Host:Port for reporting trace data in zipkin format. If not specified, will default to
+      # zipkin service (port 9411) in the same namespace as the other istio components.
+      address: ""
 
   # Default mtls policy. If true, mtls between services will be enabled by default.
   mtls:

diff --git a/install/kubernetes/helm/subcharts/gateways/templates/deployment.yaml b/install/kubernetes/helm/subcharts/gateways/templates/deployment.yaml
@@ -88,8 +88,8 @@ spec:
           - --serviceCluster
           - {{ $key }}
           - --zipkinAddress
-        {{- if $.Values.global.tracer.zipkinAddress }}
-          - {{ $.Values.global.tracer.zipkinAddress }}
+        {{- if $.Values.global.tracer.zipkin.address }}
+          - {{ $.Values.global.tracer.zipkin.address }}
         {{- else if $.Values.global.istioNamespace }}
           - zipkin.{{ $.Values.global.istioNamespace }}:9411
         {{- else }}

diff --git a/install/kubernetes/helm/subcharts/mixer/templates/deployment.yaml b/install/kubernetes/helm/subcharts/mixer/templates/deployment.yaml
@@ -40,8 +40,8 @@
           - --configStoreURL=k8s://
 {{- end }}
           - --configDefaultNamespace={{ $.Release.Namespace }}
-          {{- if $.Values.global.tracer.zipkinAddress }}
-          - --trace_zipkin_url=http://{{- $.Values.global.tracer.zipkinAddress }}/api/v1/spans
+          {{- if $.Values.global.tracer.zipkin.address }}
+          - --trace_zipkin_url=http://{{- $.Values.global.tracer.zipkin.address }}/api/v1/spans
           {{- else }}
           - --trace_zipkin_url=http://zipkin:9411/api/v1/spans
           {{- end }}
@@ -167,8 +167,8 @@
           - --configStoreURL=k8s://
 {{- end }}
           - --configDefaultNamespace={{ $.Release.Namespace }}
-          {{- if $.Values.global.tracer.zipkinAddress }}
-          - --trace_zipkin_url=http://{{- $.Values.global.tracer.zipkinAddress }}/api/v1/spans
+          {{- if $.Values.global.tracer.zipkin.address }}
+          - --trace_zipkin_url=http://{{- $.Values.global.tracer.zipkin.address }}/api/v1/spans
           {{- else }}
           - --trace_zipkin_url=http://zipkin:9411/api/v1/spans
           {{- end }}

diff --git a/pilot/cmd/pilot-agent/main.go b/pilot/cmd/pilot-agent/main.go
@@ -57,6 +57,10 @@ var (
 	parentShutdownDuration   time.Duration
 	discoveryAddress         string
 	zipkinAddress            string
+	lightstepAddress         string
+	lightstepAccessToken     string
+	lightstepSecure          bool
+	lightstepCacertPath      string
 	connectTimeout           time.Duration
 	statsdUDPAddress         string
 	proxyAdminPort           uint16
@@ -141,7 +145,6 @@ var (
 			proxyConfig.DrainDuration = types.DurationProto(drainDuration)
 			proxyConfig.ParentShutdownDuration = types.DurationProto(parentShutdownDuration)
 			proxyConfig.DiscoveryAddress = discoveryAddress
-			proxyConfig.ZipkinAddress = zipkinAddress
 			proxyConfig.ConnectTimeout = types.DurationProto(connectTimeout)
 			proxyConfig.StatsdUdpAddress = statsdUDPAddress
 			proxyConfig.ProxyAdminPort = int32(proxyAdminPort)
@@ -191,6 +194,28 @@ var (
 				}
 			}
 
+			// set tracing config
+			if lightstepAddress != "" {
+				proxyConfig.Tracing = &meshconfig.Tracing{
+					Tracer: &meshconfig.Tracing_Lightstep_{
+						Lightstep: &meshconfig.Tracing_Lightstep{
+							Address:     lightstepAddress,
+							AccessToken: lightstepAccessToken,
+							Secure:      lightstepSecure,
+							CacertPath:  lightstepCacertPath,
+						},
+					},
+				}
+			} else if zipkinAddress != "" {
+				proxyConfig.Tracing = &meshconfig.Tracing{
+					Tracer: &meshconfig.Tracing_Zipkin_{
+						Zipkin: &meshconfig.Tracing_Zipkin{
+							Address: zipkinAddress,
+						},
+					},
+				}
+			}
+
 			if err := model.ValidateProxyConfig(&proxyConfig); err != nil {
 				return err
 			}
@@ -340,8 +365,16 @@ func init() {
 		"The time in seconds that Envoy will wait before shutting down the parent process during a hot restart")
 	proxyCmd.PersistentFlags().StringVar(&discoveryAddress, "discoveryAddress", values.DiscoveryAddress,
 		"Address of the discovery service exposing xDS (e.g. istio-pilot:8080)")
-	proxyCmd.PersistentFlags().StringVar(&zipkinAddress, "zipkinAddress", values.ZipkinAddress,
+	proxyCmd.PersistentFlags().StringVar(&zipkinAddress, "zipkinAddress", "",
 		"Address of the Zipkin service (e.g. zipkin:9411)")
+	proxyCmd.PersistentFlags().StringVar(&lightstepAddress, "lightstepAddress", "",
+		"Address of the LightStep Satellite pool")
+	proxyCmd.PersistentFlags().StringVar(&lightstepAccessToken, "lightstepAccessToken", "",
+		"Access Token for LightStep Satellite pool")
+	proxyCmd.PersistentFlags().BoolVar(&lightstepSecure, "lightstepSecure", false,
+		"Should connection to the LightStep Satellite pool be secure")
+	proxyCmd.PersistentFlags().StringVar(&lightstepCacertPath, "lightstepCacertPath", "",
+		"Path to the trusted cacert used to authenticate the pool")
 	proxyCmd.PersistentFlags().DurationVar(&connectTimeout, "connectTimeout",
 		timeDuration(values.ConnectTimeout),
 		"Connection timeout used by Envoy for supporting services")

diff --git a/pilot/pkg/kube/inject/inject.go b/pilot/pkg/kube/inject/inject.go
@@ -24,6 +24,7 @@ import (
 	"io"
 	"net"
 	"os"
+	"path"
 	"reflect"
 	"strconv"
 	"strings"
@@ -475,6 +476,11 @@ func isset(m map[string]string, key string) bool {
 	return ok
 }
 
+func directory(filepath string) string {
+	dir, _ := path.Split(filepath)
+	return dir
+}
+
 func injectionData(sidecarTemplate, version string, deploymentMetadata *metav1.ObjectMeta, spec *corev1.PodSpec, metadata *metav1.ObjectMeta, proxyConfig *meshconfig.ProxyConfig, meshConfig *meshconfig.MeshConfig) (*SidecarInjectionSpec, string, error) { // nolint: lll
 	if err := validateAnnotations(metadata.GetAnnotations()); err != nil {
 		return nil, "", err
@@ -497,6 +503,7 @@ func injectionData(sidecarTemplate, version string, deploymentMetadata *metav1.O
 		"annotation":          annotation,
 		"valueOrDefault":      valueOrDefault,
 		"toJSON":              toJSON,
+		"directory":           directory,
 	}
 
 	var tmpl bytes.Buffer

diff --git a/pilot/pkg/kube/inject/mesh.go b/pilot/pkg/kube/inject/mesh.go
@@ -103,8 +103,18 @@ containers:
   - [[ formatDuration .ProxyConfig.ParentShutdownDuration ]]
   - --discoveryAddress
   - [[ .ProxyConfig.DiscoveryAddress ]]
+  [[ if .ProxyConfig.GetTracing.GetLightstep -]]
+  - --lightstepAddress
+  - [[ .ProxyConfig.GetTracing.GetLightstep.GetAddress ]]
+  - --lightstepAccessToken
+  - [[ .ProxyConfig.GetTracing.GetLightstep.GetAccessToken ]]
+  - --lightstepSecure=[[ .ProxyConfig.GetTracing.GetLightstep.GetSecure ]]
+  - --lightstepCacertPath
+  - [[ .ProxyConfig.GetTracing.GetLightstep.GetCacertPath ]]
+  [[ else if .ProxyConfig.GetTracing.GetZipkin -]]
   - --zipkinAddress
-  - [[ .ProxyConfig.ZipkinAddress ]]
+  - [[ .ProxyConfig.GetTracing.GetZipkin.GetAddress ]]
+  [[ end -]]
   - --connectTimeout
   - [[ formatDuration .ProxyConfig.ConnectTimeout ]]
   - --statsdUdpAddress

diff --git a/pilot/pkg/kube/inject/testdata/inject/auth.cert-dir.yaml.injected b/pilot/pkg/kube/inject/testdata/inject/auth.cert-dir.yaml.injected
@@ -38,8 +38,6 @@ spec:
         - 3s
         - --discoveryAddress
         - istio-pilot:15007
-        - --zipkinAddress
-        - ""
         - --connectTimeout
         - 1s
         - --statsdUdpAddress

diff --git a/pilot/pkg/kube/inject/testdata/inject/auth.non-default-service-account.yaml.injected b/pilot/pkg/kube/inject/testdata/inject/auth.non-default-service-account.yaml.injected
@@ -38,8 +38,6 @@ spec:
         - 3s
         - --discoveryAddress
         - istio-pilot:15007
-        - --zipkinAddress
-        - ""
         - --connectTimeout
         - 1s
         - --statsdUdpAddress

diff --git a/pilot/pkg/kube/inject/testdata/inject/auth.yaml.injected b/pilot/pkg/kube/inject/testdata/inject/auth.yaml.injected
@@ -38,8 +38,6 @@ spec:
         - 3s
         - --discoveryAddress
         - istio-pilot:15007
-        - --zipkinAddress
-        - ""
         - --connectTimeout
         - 1s
         - --statsdUdpAddress