[TT-774] fix loading jaeger tracing config (TykTechnologies#3418)

* fix loading jaeger tracing config * fix typo * update yaml to v3 * fix and add config for jaeger * update TestLoadJaeger * update TestLoadZipkin * use jsondiff to compare configs * cleanup * add expected configs for tracers * add tests for env only config * update jaeger tests * cleanup test * update zipkin tests * reduce noise in sample files
nvta-sbiyyala · Apr 15, 2021 · f4f1212 · f4f1212
1 parent 4235775
commit f4f1212
Show file tree

Hide file tree

Showing 18 changed files with 423 additions and 83 deletions.
diff --git a/config/config_test.go b/config/config_test.go
@@ -1,6 +1,7 @@
 package config
 
 import (
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -10,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/TykTechnologies/tyk/apidef"
+	"github.com/nsf/jsondiff"
 )
 
 func TestDefaultValueAndWriteDefaultConf(t *testing.T) {
@@ -63,6 +65,9 @@ func TestDefaultValueAndWriteDefaultConf(t *testing.T) {
 			}
 			expectedValue := fmt.Sprint(tc.expectedValue)
 			os.Setenv(tc.EnvVarName, expectedValue)
+			defer func() {
+				os.Unsetenv(tc.EnvVarName)
+			}()
 			if err := WriteDefault("", conf); err != nil {
 				t.Fatal(err)
 			}
@@ -184,3 +189,98 @@ func TestConfig_GetEventTriggers(t *testing.T) {
 	})
 
 }
+
+func TestLoad_tracing(t *testing.T) {
+	dir, err := ioutil.TempDir("", "tyk")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+	t.Run("Read and write config with tracing", func(t *testing.T) {
+		files := []string{"testdata/jaeger.json", "testdata/zipkin.json"}
+		for _, f := range files {
+			t.Run(f, func(t *testing.T) {
+				var c Config
+				err = Load([]string{f}, &c)
+				if err != nil {
+					t.Fatal(err)
+				}
+				o := filepath.Join(
+					filepath.Dir(f),
+					"expect."+filepath.Base(f),
+				)
+				expect, err := ioutil.ReadFile(o)
+				if err != nil {
+					t.Fatal(err)
+				}
+				got, err := json.MarshalIndent(c.Tracer.Options, "", "    ")
+				if err != nil {
+					t.Fatal(err)
+				}
+				diff, s := jsondiff.Compare(expect, got, &jsondiff.Options{
+					PrintTypes: true,
+				})
+				if diff == jsondiff.NoMatch {
+					t.Error(s)
+				}
+			})
+		}
+	})
+	t.Run("Env only", func(t *testing.T) {
+		type env struct {
+			name, value string
+		}
+		sample := []struct {
+			file string
+			env  []env
+		}{
+			{"testdata/env.jaeger.json", []env{
+				{"TYK_GW_TRACER_OPTIONS_SERVICENAME", "jaeger-test-service"},
+			}},
+			{"testdata/env.zipkin.json", []env{
+				{"TYK_GW_TRACER_OPTIONS_REPORTER_URL", "http://example.com"},
+				{"TYK_GW_TRACER_OPTIONS_REPORTER_BATCHSIZE", "10"},
+				{"TYK_GW_TRACER_OPTIONS_REPORTER_MAXBACKLOG", "20"},
+				{"TYK_GW_TRACER_OPTIONS_SAMPLER_NAME", "boundary"},
+				{"TYK_GW_TRACER_OPTIONS_SAMPLER_RATE", "10.1"},
+				{"TYK_GW_TRACER_OPTIONS_SAMPLER_SALT", "10"},
+				{"TYK_GW_TRACER_OPTIONS_SAMPLER_MOD", "12"},
+			}},
+		}
+		for _, v := range sample {
+			t.Run(v.file, func(t *testing.T) {
+				for _, e := range v.env {
+					os.Setenv(e.name, e.value)
+				}
+				defer func() {
+					for _, e := range v.env {
+						os.Unsetenv(e.name)
+					}
+				}()
+				var c Config
+				err = Load([]string{v.file}, &c)
+				if err != nil {
+					t.Fatal(err)
+				}
+				o := filepath.Join(
+					filepath.Dir(v.file),
+					"expect."+filepath.Base(v.file),
+				)
+				expect, err := ioutil.ReadFile(o)
+				if err != nil {
+					t.Fatal(err)
+				}
+				got, err := json.MarshalIndent(c.Tracer.Options, "", "    ")
+				if err != nil {
+					t.Fatal(err)
+				}
+				diff, s := jsondiff.Compare(expect, got, &jsondiff.Options{
+					PrintTypes: true,
+				})
+				if diff == jsondiff.NoMatch {
+					t.Error(s)
+				}
+			})
+		}
+	})
+}
diff --git a/config/opentracing_custom_env_loader.go b/config/opentracing_custom_env_loader.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/kelseyhightower/envconfig"
 	jaeger "github.com/uber/jaeger-client-go/config"
+	"gopkg.in/yaml.v3"
 )
 
 // ZipkinConfig configuration options used to initialize openzipkin opentracing
@@ -56,6 +57,14 @@ func DecodeJSON(dest, src interface{}) error {
 	return json.Unmarshal(b, dest)
 }
 
+func DecodeYAML(dest, src interface{}) error {
+	b, err := yaml.Marshal(src)
+	if err != nil {
+		return err
+	}
+	return yaml.Unmarshal(b, dest)
+}
+
 // loadZipkin tries to lad zipkin configuration from environment variables.
 //
 // list of zipkin configuration env variables
@@ -123,7 +132,7 @@ func loadJaeger(prefix string, c *Config) error {
 		return nil
 	}
 	var j jaeger.Configuration
-	if err := DecodeJSON(&j, c.Tracer.Options); err != nil {
+	if err := DecodeYAML(&j, c.Tracer.Options); err != nil {
 		return err
 	}
 	qualifyPrefix := prefix + "_TRACER_OPTIONS"
@@ -132,7 +141,7 @@ func loadJaeger(prefix string, c *Config) error {
 		return err
 	}
 	o := make(map[string]interface{})
-	if err := DecodeJSON(&o, j); err != nil {
+	if err := DecodeYAML(&o, j); err != nil {
 		return err
 	}
 	c.Tracer.Options = o

diff --git a/config/opentracing_custom_env_loader_test.go b/config/opentracing_custom_env_loader_test.go
@@ -11,7 +11,7 @@ import (
 func TestLoadZipkin(t *testing.T) {
 	base := ZipkinConfig{
 		Reporter: Reporter{
-			URL:        "repoturl",
+			URL:        "http://example.com",
 			BatchSize:  10,
 			MaxBacklog: 20,
 		},
@@ -33,16 +33,6 @@ func TestLoadZipkin(t *testing.T) {
 		{"TYK_GW_TRACER_OPTIONS_SAMPLER_SALT", fmt.Sprint(base.Sampler.Salt)},
 		{"TYK_GW_TRACER_OPTIONS_SAMPLER_MOD", fmt.Sprint(base.Sampler.Mod)},
 	}
-	t.Run("Returns nil when it is not zipkin config", func(t *testing.T) {
-		conf := &Config{}
-		err := loadZipkin(envPrefix, conf)
-		if err != nil {
-			t.Fatal(err)
-		}
-		if conf.Tracer.Options != nil {
-			t.Error("expected options to be nil")
-		}
-	})
 
 	t.Run("loads env vars", func(t *testing.T) {
 		for _, v := range sample {
@@ -56,8 +46,8 @@ func TestLoadZipkin(t *testing.T) {
 				os.Unsetenv(v.env)
 			}
 		}()
-		conf := &Config{Tracer: Tracer{Name: "zipkin"}}
-		err := loadZipkin(envPrefix, conf)
+		var conf Config
+		err := Load([]string{"testdata/zipkin.json"}, &conf)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -76,23 +66,13 @@ func TestLoadZipkin(t *testing.T) {
 }
 
 func TestLoadJaeger(t *testing.T) {
-	base := &jaeger.Configuration{ServiceName: "jaeger-test-service"}
+	name := "jaeger-test-service"
 	sample := []struct {
 		env   string
 		value string
 	}{
-		{"TYK_GW_TRACER_OPTIONS_SERVICENAME", base.ServiceName},
+		{"TYK_GW_TRACER_OPTIONS_SERVICENAME", name},
 	}
-	t.Run("Returns nil when it is not jaeger config", func(t *testing.T) {
-		conf := &Config{}
-		err := loadJaeger(envPrefix, conf)
-		if err != nil {
-			t.Fatal(err)
-		}
-		if conf.Tracer.Options != nil {
-			t.Error("expected options to be nil")
-		}
-	})
 
 	t.Run("Loads env vars", func(t *testing.T) {
 		for _, v := range sample {
@@ -107,18 +87,18 @@ func TestLoadJaeger(t *testing.T) {
 			}
 		}()
 
-		conf := &Config{Tracer: Tracer{Name: "jaeger"}}
-		err := loadJaeger(envPrefix, conf)
+		var conf Config
+		err := Load([]string{"testdata/jaeger.json"}, &conf)
 		if err != nil {
 			t.Fatal(err)
 		}
 		var got jaeger.Configuration
-		err = DecodeJSON(&got, conf.Tracer.Options)
+		err = DecodeYAML(&got, conf.Tracer.Options)
 		if err != nil {
 			t.Fatal(err)
 		}
-		if base.ServiceName != got.ServiceName {
-			t.Errorf("expected %#v got %#v", base.ServiceName, got.ServiceName)
+		if got.ServiceName != name {
+			t.Errorf("expected %#v got %#v", name, got.ServiceName)
 		}
 	})
 }
diff --git a/config/testdata/env.jaeger.json b/config/testdata/env.jaeger.json
@@ -0,0 +1,7 @@
+{
+    "tracing": {
+        "name": "jaeger",
+        "enabled": true,
+        "options": null
+    }
+}
diff --git a/config/testdata/env.zipkin.json b/config/testdata/env.zipkin.json
@@ -0,0 +1,7 @@
+{
+    "tracing": {
+        "name": "zipkin",
+        "enabled": true,
+        "options": null
+    }
+}
diff --git a/config/testdata/expect.env.jaeger.json b/config/testdata/expect.env.jaeger.json
@@ -0,0 +1,38 @@
+{
+    "baggage_restrictions": {
+        "denyBaggageOnInitializationFailure": false,
+        "hostPort": "",
+        "refreshInterval": "0s"
+    },
+    "disabled": false,
+    "headers": {
+        "TraceContextHeaderName": "",
+        "jaegerBaggageHeader": "",
+        "jaegerDebugHeader": "",
+        "traceBaggageHeaderPrefix": ""
+    },
+    "reporter": {
+        "bufferflushinterval": "0s",
+        "collectorEndpoint": "",
+        "localAgentHostPort": "",
+        "logSpans": false,
+        "password": "",
+        "queueSize": 0,
+        "user": ""
+    },
+    "rpc_metrics": false,
+    "sampler": {
+        "maxOperations": 0,
+        "param": 0,
+        "samplingRefreshInterval": "0s",
+        "samplingServerURL": "",
+        "type": ""
+    },
+    "serviceName": "jaeger-test-service",
+    "tags": [],
+    "throttler": {
+        "hostPort": "",
+        "refreshInterval": "0s",
+        "synchronousInitialization": false
+    }
+}
diff --git a/config/testdata/expect.env.zipkin.json b/config/testdata/expect.env.zipkin.json
@@ -0,0 +1,13 @@
+{
+    "reporter": {
+        "batch_size": 10,
+        "max_backlog": 20,
+        "url": "http://example.com"
+    },
+    "sampler": {
+        "mod": 12,
+        "name": "boundary",
+        "rate": 10.1,
+        "salt": 10
+    }
+}
diff --git a/config/testdata/expect.jaeger.json b/config/testdata/expect.jaeger.json
@@ -0,0 +1,38 @@
+{
+    "baggage_restrictions": {
+        "denyBaggageOnInitializationFailure": false,
+        "hostPort": "",
+        "refreshInterval": "0s"
+    },
+    "disabled": false,
+    "headers": {
+        "TraceContextHeaderName": "",
+        "jaegerBaggageHeader": "",
+        "jaegerDebugHeader": "",
+        "traceBaggageHeaderPrefix": ""
+    },
+    "reporter": {
+        "bufferflushinterval": "0s",
+        "collectorEndpoint": "",
+        "localAgentHostPort": "jaeger:6831",
+        "logSpans": true,
+        "password": "",
+        "queueSize": 0,
+        "user": ""
+    },
+    "rpc_metrics": false,
+    "sampler": {
+        "maxOperations": 0,
+        "param": 1,
+        "samplingRefreshInterval": "0s",
+        "samplingServerURL": "",
+        "type": "const"
+    },
+    "serviceName": "tyk-gateway",
+    "tags": [],
+    "throttler": {
+        "hostPort": "",
+        "refreshInterval": "0s",
+        "synchronousInitialization": false
+    }
+}
diff --git a/config/testdata/expect.zipkin.json b/config/testdata/expect.zipkin.json
@@ -0,0 +1,13 @@
+{
+    "reporter": {
+        "batch_size": 0,
+        "max_backlog": 0,
+        "url": "http:localhost:9411/api/v2/spans"
+    },
+    "sampler": {
+        "mod": 0,
+        "name": "",
+        "rate": 0,
+        "salt": 0
+    }
+}