From 3fae2094107157b93a5e3056baa46a97e4d072b9 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 18 Aug 2021 17:16:56 -0700 Subject: [PATCH 01/27] shellspec --init Shellspec looks like the best option for shell-based testing frameworks. I have no idea if this is gonna work out :) Signed-off-by: Amy Tobey --- .shellspec | 12 ++++++++++++ spec/spec_helper.sh | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 .shellspec create mode 100644 spec/spec_helper.sh diff --git a/.shellspec b/.shellspec new file mode 100644 index 0000000..d567ecf --- /dev/null +++ b/.shellspec @@ -0,0 +1,12 @@ +--require spec_helper + +## Default kcov (coverage) options +# --kcov-options "--include-path=. --path-strip-level=1" +# --kcov-options "--include-pattern=.sh" +# --kcov-options "--exclude-pattern=/.shellspec,/spec/,/coverage/,/report/" + +## Example: Include script "myprog" with no extension +# --kcov-options "--include-pattern=.sh,myprog" + +## Example: Only specified files/directories +# --kcov-options "--include-pattern=myprog,/lib/" diff --git a/spec/spec_helper.sh b/spec/spec_helper.sh new file mode 100644 index 0000000..93f1908 --- /dev/null +++ b/spec/spec_helper.sh @@ -0,0 +1,24 @@ +# shellcheck shell=sh + +# Defining variables and functions here will affect all specfiles. +# Change shell options inside a function may cause different behavior, +# so it is better to set them here. +# set -eu + +# This callback function will be invoked only once before loading specfiles. +spec_helper_precheck() { + # Available functions: info, warn, error, abort, setenv, unsetenv + # Available variables: VERSION, SHELL_TYPE, SHELL_VERSION + : minimum_version "0.28.1" +} + +# This callback function will be invoked after a specfile has been loaded. +spec_helper_loaded() { + : +} + +# This callback function will be invoked after core modules has been loaded. +spec_helper_configure() { + # Available functions: import, before_each, after_each, before_all, after_all + : import 'support/custom_matcher' +} From 159beb03dac8e91dd43833f38b924a341f461ea7 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 18 Aug 2021 17:24:14 -0700 Subject: [PATCH 02/27] empty OTEL_EXPORTER_OTLP_INSECURE should not log Turns out strconv.ParseBool returns error on empty string, so this avoids that while keeping it clear how it fails closed. Signed-off-by: Amy Tobey --- otelinit/config.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/otelinit/config.go b/otelinit/config.go index 58f9fbc..877c65a 100644 --- a/otelinit/config.go +++ b/otelinit/config.go @@ -19,10 +19,17 @@ func newConfig(serviceName string) config { // Use stdlib to parse. If it's an invalid value and doesn't parse, log it // and keep going. It should already be false on error but we force it to // be extra clear that it's failing closed. - insecure, err := strconv.ParseBool(os.Getenv("OTEL_EXPORTER_OTLP_INSECURE")) - if err != nil { + isEnv := os.Getenv("OTEL_EXPORTER_OTLP_INSECURE") + var insecure bool + if isEnv != "" { + var err error + insecure, err = strconv.ParseBool(isEnv) + if err != nil { + insecure = false + log.Println("Invalid boolean value in OTEL_EXPORTER_OTLP_INSECURE. Try true or false.") + } + } else { insecure = false - log.Println("Invalid boolean value in OTEL_EXPORTER_OTLP_INSECURE. Try true or false.") } return config{ From 33c83c898429c43eba5c53ed078670c050d5fda7 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 18 Aug 2021 17:38:06 -0700 Subject: [PATCH 03/27] add a test program This program calls the otel-init-go API, creates one span, then immediately ends it and exits. --- cmd/test-otel-init-go/main.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 cmd/test-otel-init-go/main.go diff --git a/cmd/test-otel-init-go/main.go b/cmd/test-otel-init-go/main.go new file mode 100644 index 0000000..bec3b20 --- /dev/null +++ b/cmd/test-otel-init-go/main.go @@ -0,0 +1,18 @@ +package main + +import ( + "context" + + "github.com/equinix-labs/otel-init-go/otelinit" + "go.opentelemetry.io/otel" +) + +func main() { + ctx := context.Background() + otelShutdown := otelinit.InitOpenTelemetry(ctx, "otel-init-go-test") + defer otelShutdown(ctx) + + tracer := otel.Tracer("otel-init-go-test") + _, span := tracer.Start(ctx, "test span 1") + span.End() +} From ce0ba79504ed8c27e35efeda592be69333952723 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 18 Aug 2021 18:36:24 -0700 Subject: [PATCH 04/27] make test program dump state to json The test program now writes its state to json on stdout. This output is intended to be captured in tests, to compare against pre-set environment variable configurations. Made otelinit.Config public so it can be tunneled through context and picked up in the test program. There are other ways to accomplish this but it might be useful to folks down the road for debugging. So now the "otel-init-config" value in the context can be used to grab the config without an increase in API surface other than returning context, which like the last API change, it probably should have in the first place. (I fully expect these changes to settle down by the time this PR merges.) Signed-off-by: Amy Tobey --- .gitignore | 3 +-- README.md | 2 +- cmd/test-otel-init-go/main.go | 47 +++++++++++++++++++++++++++++++++-- otelinit/config.go | 22 ++++++++-------- otelinit/public.go | 19 +++++++++----- otelinit/tracing.go | 10 ++++---- 6 files changed, 77 insertions(+), 26 deletions(-) diff --git a/.gitignore b/.gitignore index 66fd13c..cfe057f 100644 --- a/.gitignore +++ b/.gitignore @@ -11,5 +11,4 @@ # Output of the go coverage tool, specifically when used with LiteIDE *.out -# Dependency directories (remove the comment below to include it) -# vendor/ +cmd/test-otel-init-go/test-otel-init-go diff --git a/README.md b/README.md index d1ebd5a..ffbeeaa 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ import ( func main() { ctx := context.Background() - otelShutdown := otelinit.InitOpenTelemetry(ctx, "my-amazing-application") + ctx, otelShutdown := otelinit.InitOpenTelemetry(ctx, "my-amazing-application") defer otelShutdown(ctx) } ``` diff --git a/cmd/test-otel-init-go/main.go b/cmd/test-otel-init-go/main.go index bec3b20..8d0eabc 100644 --- a/cmd/test-otel-init-go/main.go +++ b/cmd/test-otel-init-go/main.go @@ -2,6 +2,11 @@ package main import ( "context" + "encoding/json" + "log" + "os" + "strconv" + "strings" "github.com/equinix-labs/otel-init-go/otelinit" "go.opentelemetry.io/otel" @@ -9,10 +14,48 @@ import ( func main() { ctx := context.Background() - otelShutdown := otelinit.InitOpenTelemetry(ctx, "otel-init-go-test") + ctx, otelShutdown := otelinit.InitOpenTelemetry(ctx, "otel-init-go-test") defer otelShutdown(ctx) tracer := otel.Tracer("otel-init-go-test") - _, span := tracer.Start(ctx, "test span 1") + ctx, span := tracer.Start(ctx, "dump state") + + env := make(map[string]string) + for _, e := range os.Environ() { + parts := strings.SplitN(e, "=", 2) + if len(parts) == 2 { + env[parts[0]] = parts[1] + } else { + log.Fatalf("BUG this shouldn't happen") + } + } + + // public.go stuffs the config in the context just so we can do this + // this is totally unsafe like this and will probably crash on nil + conf := ctx.Value("otel-init-config").(*otelinit.Config) + sc := span.SpanContext() + outData := map[string]map[string]string{ + "config": { + "endpoint": conf.Endpoint, + "service_name": conf.Servicename, + "insecure": strconv.FormatBool(conf.Insecure), + }, + "otel": { + "trace_id": sc.TraceID().String(), + "span_id": sc.SpanID().String(), + "trace_flags": sc.TraceFlags().String(), + "is_sampled": strconv.FormatBool(sc.IsSampled()), + }, + "env": env, + } + + js, err := json.MarshalIndent(outData, "", " ") + if err != nil { + log.Fatal(err) + } + + os.Stdout.Write(js) + os.Stdout.WriteString("\n") + span.End() } diff --git a/otelinit/config.go b/otelinit/config.go index 877c65a..85ab45a 100644 --- a/otelinit/config.go +++ b/otelinit/config.go @@ -6,16 +6,18 @@ import ( "strconv" ) -// config holds the typed values of configuration read from environment variables -type config struct { - servicename string - endpoint string - insecure bool +// Config holds the typed values of configuration read from the environment. +// It is public mainly to make testing easier and most users should never +// use it directly. +type Config struct { + Servicename string `json:"service_name"` + Endpoint string `json:"endpoint"` + Insecure bool `json:"insecure"` } // newConfig reads all of the documented environment variables and returns a // config struct. -func newConfig(serviceName string) config { +func newConfig(serviceName string) Config { // Use stdlib to parse. If it's an invalid value and doesn't parse, log it // and keep going. It should already be false on error but we force it to // be extra clear that it's failing closed. @@ -32,9 +34,9 @@ func newConfig(serviceName string) config { insecure = false } - return config{ - servicename: serviceName, - endpoint: os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT"), - insecure: insecure, + return Config{ + Servicename: serviceName, + Endpoint: os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT"), + Insecure: insecure, } } diff --git a/otelinit/public.go b/otelinit/public.go index 2def824..c15d731 100644 --- a/otelinit/public.go +++ b/otelinit/public.go @@ -8,20 +8,27 @@ type OtelShutdown func(context.Context) // It requires a context.Context and service name string that is the name of // your service or application. // TODO: should even this be overrideable via envvars? -// Returns a func() that encapuslates clean shutdown. -func InitOpenTelemetry(ctx context.Context, serviceName string) OtelShutdown { +// Returns context and a func() that encapuslates clean shutdown. +func InitOpenTelemetry(ctx context.Context, serviceName string) (context.Context, OtelShutdown) { c := newConfig(serviceName) - if c.endpoint != "" { - tracingShutdown := c.initTracing(ctx) + // no idea if this is gonna work... + // or even if this is a good idea but it would be well out of most folks' + // way here and I can snag it from test code without burdening anyone else + // and it's a teensy amount of memory + ctx = context.WithValue(ctx, "otel-init-config", &c) + + if c.Endpoint != "" { + ctx, tracingShutdown := c.initTracing(ctx) // TODO: initMetrics() // TODO: initLogs() - return func(ctx context.Context) { + return ctx, func(ctx context.Context) { tracingShutdown(ctx) } } // no configuration, nothing to do, the calling code is inert - return func(context.Context) {} + // config is available in the returned context (for test/debug) + return ctx, func(context.Context) {} } diff --git a/otelinit/tracing.go b/otelinit/tracing.go index b4c0a2d..cdd9a8e 100644 --- a/otelinit/tracing.go +++ b/otelinit/tracing.go @@ -13,16 +13,16 @@ import ( "google.golang.org/grpc/credentials" ) -func (c config) initTracing(ctx context.Context) OtelShutdown { +func (c Config) initTracing(ctx context.Context) (context.Context, OtelShutdown) { // set the service name that will show up in tracing UIs - resAttrs := resource.WithAttributes(semconv.ServiceNameKey.String(c.servicename)) + resAttrs := resource.WithAttributes(semconv.ServiceNameKey.String(c.Servicename)) res, err := resource.New(ctx, resAttrs) if err != nil { log.Fatalf("failed to create OpenTelemetry service name resource: %s", err) } - grpcOpts := []otlpgrpc.Option{otlpgrpc.WithEndpoint(c.endpoint)} - if c.insecure { + grpcOpts := []otlpgrpc.Option{otlpgrpc.WithEndpoint(c.Endpoint)} + if c.Insecure { grpcOpts = append(grpcOpts, otlpgrpc.WithInsecure()) } else { creds := credentials.NewClientTLSFromCert(nil, "") @@ -50,7 +50,7 @@ func (c config) initTracing(ctx context.Context) OtelShutdown { otel.SetTracerProvider(tracerProvider) // the public function will wrap this in its own shutdown function - return func(ctx context.Context) { + return ctx, func(ctx context.Context) { err = tracerProvider.Shutdown(ctx) if err != nil { log.Printf("shutdown of OpenTelemetry tracerProvider failed: %s", err) From c53770207925c393887f297e2a0914e0aef28227 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 18 Aug 2021 18:52:41 -0700 Subject: [PATCH 05/27] add a safe function for extracting config from context Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main.go | 5 ++++- otelinit/public.go | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cmd/test-otel-init-go/main.go b/cmd/test-otel-init-go/main.go index 8d0eabc..e255af7 100644 --- a/cmd/test-otel-init-go/main.go +++ b/cmd/test-otel-init-go/main.go @@ -32,7 +32,10 @@ func main() { // public.go stuffs the config in the context just so we can do this // this is totally unsafe like this and will probably crash on nil - conf := ctx.Value("otel-init-config").(*otelinit.Config) + conf, ok := otelinit.ConfigFromContext(ctx) + if !ok { + log.Println("failed to retrieve otelinit.Config pointer from context, test results may be invalid") + } sc := span.SpanContext() outData := map[string]map[string]string{ "config": { diff --git a/otelinit/public.go b/otelinit/public.go index c15d731..20889eb 100644 --- a/otelinit/public.go +++ b/otelinit/public.go @@ -32,3 +32,16 @@ func InitOpenTelemetry(ctx context.Context, serviceName string) (context.Context // config is available in the returned context (for test/debug) return ctx, func(context.Context) {} } + +// ConfigFromContext extracts the Config struct from the provided context. +// Returns the Config and true if it was retried successfully, false otherwise. +func ConfigFromContext(ctx context.Context) (*Config, bool) { + raw := ctx.Value("otel-init-config") + if raw != nil { + if conf, ok := raw.(*Config); ok { + return conf, true + } + } + + return &Config{}, false +} From 827a6f921e392b9397415e2d83b7009bea4d3847 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 18 Aug 2021 18:53:21 -0700 Subject: [PATCH 06/27] remove outdated comment Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/test-otel-init-go/main.go b/cmd/test-otel-init-go/main.go index e255af7..95f8460 100644 --- a/cmd/test-otel-init-go/main.go +++ b/cmd/test-otel-init-go/main.go @@ -31,7 +31,6 @@ func main() { } // public.go stuffs the config in the context just so we can do this - // this is totally unsafe like this and will probably crash on nil conf, ok := otelinit.ConfigFromContext(ctx) if !ok { log.Println("failed to retrieve otelinit.Config pointer from context, test results may be invalid") From d71540461a251abc1674fd9752ed41e498dd3a29 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 18 Aug 2021 19:00:39 -0700 Subject: [PATCH 07/27] set conf to empty config on retrieval error This code path might never fire but if it does, it probably shouldn't crash :) Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/test-otel-init-go/main.go b/cmd/test-otel-init-go/main.go index 95f8460..5129a4a 100644 --- a/cmd/test-otel-init-go/main.go +++ b/cmd/test-otel-init-go/main.go @@ -34,6 +34,7 @@ func main() { conf, ok := otelinit.ConfigFromContext(ctx) if !ok { log.Println("failed to retrieve otelinit.Config pointer from context, test results may be invalid") + conf = &otelinit.Config{} } sc := span.SpanContext() outData := map[string]map[string]string{ From 0289524f61f9c214cda0f321ed0368206c3dc3d5 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Thu, 19 Aug 2021 16:58:18 -0700 Subject: [PATCH 08/27] add tests! This is the first pass at trying the idea of using a stub program and otel-cli together to validate otel-cli's behavior through real environment variable configuration and connections to an OTLP server (otel-cli). Lots to do, but this is a working start and it's the end of my work day :) Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/00-unconfigured.json | 22 ++ .../01-irrelevant-envvar.json | 24 ++ cmd/test-otel-init-go/main_test.go | 234 ++++++++++++++++++ 3 files changed, 280 insertions(+) create mode 100644 cmd/test-otel-init-go/00-unconfigured.json create mode 100644 cmd/test-otel-init-go/01-irrelevant-envvar.json create mode 100644 cmd/test-otel-init-go/main_test.go diff --git a/cmd/test-otel-init-go/00-unconfigured.json b/cmd/test-otel-init-go/00-unconfigured.json new file mode 100644 index 0000000..fd1ba66 --- /dev/null +++ b/cmd/test-otel-init-go/00-unconfigured.json @@ -0,0 +1,22 @@ +{ + "name": "no configuration at all", + "stub_env": {}, + "stub_data": { + "config": { + "endpoint": "", + "insecure": "false", + "service_name": "otel-init-go-test" + }, + "env": {}, + "otel": { + "is_sampled": "false", + "span_id": "0000000000000000", + "trace_flags": "00", + "trace_id": "00000000000000000000000000000000" + } + }, + "spans_expected": 0, + "timeout": 0, + "should_timeout": false, + "skip_otel_cli": true +} \ No newline at end of file diff --git a/cmd/test-otel-init-go/01-irrelevant-envvar.json b/cmd/test-otel-init-go/01-irrelevant-envvar.json new file mode 100644 index 0000000..3141f7a --- /dev/null +++ b/cmd/test-otel-init-go/01-irrelevant-envvar.json @@ -0,0 +1,24 @@ +{ + "name": "one unrelated envvar", + "stub_env": { + "UNRELATED_ENVVAR": "unrelated data" + }, + "stub_data": { + "config": { + "endpoint": "", + "insecure": "false", + "service_name": "otel-init-go-test" + }, + "env": {}, + "otel": { + "is_sampled": "false", + "span_id": "0000000000000000", + "trace_flags": "00", + "trace_id": "00000000000000000000000000000000" + } + }, + "spans_expected": 0, + "timeout": 0, + "should_timeout": false, + "skip_otel_cli": true +} \ No newline at end of file diff --git a/cmd/test-otel-init-go/main_test.go b/cmd/test-otel-init-go/main_test.go new file mode 100644 index 0000000..1a0a7b7 --- /dev/null +++ b/cmd/test-otel-init-go/main_test.go @@ -0,0 +1,234 @@ +package main + +// testing test-otel-init-go tests otel-init-go by using otel-cli +// to receive spans and validate things are working +// +// this is still very much a work in progress idea and might not fully +// pan out but the first part is looking good +// +// TODOs: +// [ ] write data tests +// [ ] replace that time.Sleep with proper synchronization +// [ ] use random ports for listener address? + +import ( + "encoding/json" + "fmt" + "io/fs" + "io/ioutil" + "log" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" + "time" +) + +// CliEvent is mostly the same as otel-cli's internal event format, with +// the addition that it has a place to stuff events. +type CliEvent struct { + TraceID string `json:"trace_id"` + SpanID string `json:"span_id"` + ParentID string `json:"parent_span_id"` + Library string `json:"library"` + Name string `json:"name"` + Kind string `json:"kind"` + Start string `json:"start"` + End string `json:"end"` + ElapsedMS int `json:"elapsed_ms"` + Attributes map[string]string `json:"attributes"` + Events []CliEvent // reader code will stuff kind=event in here +} + +// tid sid +type CliEvents map[string]map[string]CliEvent + +// StubData is the structure of the data that the stub program +// prints out. +type StubData struct { + Config map[string]string `json:"config"` + Env map[string]string `json:"env"` + Otel map[string]string `json:"otel"` +} + +// Scenario represents the configuration of a test scenario. Scenarios +// are found in json files in this directory. +type Scenario struct { + Name string `json:"name"` + StubEnv map[string]string `json:"stub_env"` // given to stub + StubData StubData `json:"stub_data"` // data from stub, exact match + SpansExpected int `json:"spans_expected"` + Timeout int `json:"timeout"` + ShouldTimeout bool `json:"should_timeout"` // otel connection stub->cli should fail + SkipOtelCli bool `json:"skip_otel_cli"` // don't run otel-cli at all +} + +func TestMain(m *testing.M) { + // wipe out this process's envvars right away to avoid pollution & leakage + os.Clearenv() + os.Exit(m.Run()) +} + +// TestOtelInit loads all the json files in this directory and executes the +// tests they define. +func TestOtelInit(t *testing.T) { + // get a list of all json files in this directory + // https://dave.cheney.net/2016/05/10/test-fixtures-in-go + wd, _ := os.Getwd() + files, err := ioutil.ReadDir(wd) + if err != nil { + t.Fatalf("Failed to list test directory %q to detect json files.", wd) + } + + scenarios := []Scenario{} + for _, file := range files { + if strings.HasSuffix(file.Name(), ".json") { + scenario := Scenario{StubEnv: map[string]string{}} + js, err := os.ReadFile(file.Name()) + if err != nil { + t.Fatalf("Failed to read json test file %q: %s", file.Name(), err) + } + err = json.Unmarshal(js, &scenario) + if err != nil { + t.Fatalf("Failed to parse json test file %q: %s", file.Name(), err) + } + scenarios = append(scenarios, scenario) + } + } + + t.Logf("Loaded %d tests.", len(scenarios)) + + // run all the scenarios + for _, s := range scenarios { + stubData, events := runPrograms(t, s) + checkData(t, s, stubData, events) + + // temporary debug print + for tid, trace := range events { + for sid, span := range trace { + t.Logf("trace id %s span id %s is %q", tid, sid, span) + } + } + } +} + +// TODO: write these checks :) +func checkData(t *testing.T, scenario Scenario, stubData StubData, events CliEvents) { + +} + +// runPrograms runs the stub program and otel-cli together and captures their +// output as data to return for further testing. +// all failures are fatal, no point in testing if this is broken +func runPrograms(t *testing.T, scenario Scenario) (StubData, CliEvents) { + tmpdir, err := os.MkdirTemp(os.TempDir(), "otel-init-go-test") + defer os.RemoveAll(tmpdir) + if err != nil { + t.Fatalf("MkdirTemp failed: %s", err) + } + + cliArgs := []string{"server", "json", "--dir", tmpdir} + + if scenario.Timeout > 0 { + cliArgs = append(cliArgs, "--timeout", strconv.Itoa(scenario.Timeout)) + } + + if scenario.SpansExpected > 0 { + cliArgs = append(cliArgs, "--max-spans", strconv.Itoa(scenario.SpansExpected)) + } + + // MAYBE: server json --stdout is maybe better? and could add a graceful exit on closed fds + // TODO: obviously this is horrible + otelcli := exec.Command("/home/atobey/src/otel-cli/otel-cli", cliArgs...) + + if !scenario.SkipOtelCli { + go func() { + err = otelcli.Run() + if err != nil { + log.Fatalf("Executing command %q failed: %s", otelcli.String(), err) + } + }() + } + + // yes yes I know this is horrible + time.Sleep(time.Millisecond * 10) + + // TODO: obviously this is horrible + stub := exec.Command("/home/atobey/src/otel-init-go/cmd/test-otel-init-go/test-otel-init-go") + stub.Env = mkEnviron(scenario.StubEnv) + stubOut, err := stub.Output() + if err != nil { + t.Fatalf("Executing stub command %q failed: %s", stub.String(), err) + } + + fmt.Printf("\n\n%s\n\n", string(stubOut)) + + stubData := StubData{ + Config: map[string]string{}, + Env: map[string]string{}, + Otel: map[string]string{}, + } + err = json.Unmarshal(stubOut, &stubData) + if err != nil { + fmt.Printf("\n\n%s\n\n", string(stubOut)) + t.Fatalf("Unmarshaling stub output failed: %s", err) + } + + if !scenario.SkipOtelCli { + otelcli.Wait() + } + + events := make(CliEvents) + filepath.WalkDir(tmpdir, func(path string, d fs.DirEntry, err error) error { + // TODO: make sure to read span.json before events.json + // so maybe a directory walk would be better anyways + if strings.HasSuffix(path, ".json") { + pi := strings.Split(path, string(os.PathSeparator)) + if len(pi) >= 3 { + js, err := ioutil.ReadFile(path) + if err != nil { + t.Fatalf("error while reading file %q: %s", path, err) + } + + evt := CliEvent{ + Attributes: make(map[string]string), + Events: make([]CliEvent, 0), + } + err = json.Unmarshal(js, &evt) + if err != nil { + t.Fatalf("error while parsing json file %q: %s", path, err) + } + + tid := pi[len(pi)-3] + sid := pi[len(pi)-2] + if trace, ok := events[tid]; ok { + if _, ok := trace[sid]; ok { + t.Fatal("unfinished code path") + } + trace[sid] = evt + } else { + events[tid] = make(map[string]CliEvent) + events[tid][sid] = evt + } + // TODO: events + } + } + return nil + }) + + return stubData, events +} + +// mkEnviron converts a string map to a list of k=v strings. +func mkEnviron(env map[string]string) []string { + mapped := make([]string, len(env)) + var i int + for k, v := range env { + mapped[i] = k + "=" + v + i++ + } + + return mapped +} From 539aec40c0f98a1aa6e5d3b220ac4dc938afdd01 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Thu, 19 Aug 2021 17:00:08 -0700 Subject: [PATCH 09/27] remove shellspec not gonna need it, doing it all in Go :) Signed-off-by: Amy Tobey --- .shellspec | 12 ------------ spec/spec_helper.sh | 24 ------------------------ 2 files changed, 36 deletions(-) delete mode 100644 .shellspec delete mode 100644 spec/spec_helper.sh diff --git a/.shellspec b/.shellspec deleted file mode 100644 index d567ecf..0000000 --- a/.shellspec +++ /dev/null @@ -1,12 +0,0 @@ ---require spec_helper - -## Default kcov (coverage) options -# --kcov-options "--include-path=. --path-strip-level=1" -# --kcov-options "--include-pattern=.sh" -# --kcov-options "--exclude-pattern=/.shellspec,/spec/,/coverage/,/report/" - -## Example: Include script "myprog" with no extension -# --kcov-options "--include-pattern=.sh,myprog" - -## Example: Only specified files/directories -# --kcov-options "--include-pattern=myprog,/lib/" diff --git a/spec/spec_helper.sh b/spec/spec_helper.sh deleted file mode 100644 index 93f1908..0000000 --- a/spec/spec_helper.sh +++ /dev/null @@ -1,24 +0,0 @@ -# shellcheck shell=sh - -# Defining variables and functions here will affect all specfiles. -# Change shell options inside a function may cause different behavior, -# so it is better to set them here. -# set -eu - -# This callback function will be invoked only once before loading specfiles. -spec_helper_precheck() { - # Available functions: info, warn, error, abort, setenv, unsetenv - # Available variables: VERSION, SHELL_TYPE, SHELL_VERSION - : minimum_version "0.28.1" -} - -# This callback function will be invoked after a specfile has been loaded. -spec_helper_loaded() { - : -} - -# This callback function will be invoked after core modules has been loaded. -spec_helper_configure() { - # Available functions: import, before_each, after_each, before_all, after_all - : import 'support/custom_matcher' -} From 5ab65ba7aabd9a7cb34fcedae999325e75758790 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Thu, 19 Aug 2021 17:11:48 -0700 Subject: [PATCH 10/27] add an explanatory comment to main.go Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/test-otel-init-go/main.go b/cmd/test-otel-init-go/main.go index 5129a4a..95b5dbf 100644 --- a/cmd/test-otel-init-go/main.go +++ b/cmd/test-otel-init-go/main.go @@ -1,5 +1,9 @@ package main +// All this program does is load up otel-init-go, create one trace, dump state +// in json for all these things, then exit. This data is intended to be consumed +// in main_test.go, which is really about testing otel-cli-init itself. + import ( "context" "encoding/json" From e6a4a27408f65766170ae56cbf992f277022c201 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 12:56:06 -0700 Subject: [PATCH 11/27] first pass at data tests This was easier than I thought it would be? Hooray for comparing string/string maps I guess. Also fixed a test failure in 01, that was a mistake in the data :) Signed-off-by: Amy Tobey --- .../01-irrelevant-envvar.json | 6 ++++-- cmd/test-otel-init-go/main_test.go | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/cmd/test-otel-init-go/01-irrelevant-envvar.json b/cmd/test-otel-init-go/01-irrelevant-envvar.json index 3141f7a..2d18156 100644 --- a/cmd/test-otel-init-go/01-irrelevant-envvar.json +++ b/cmd/test-otel-init-go/01-irrelevant-envvar.json @@ -9,7 +9,9 @@ "insecure": "false", "service_name": "otel-init-go-test" }, - "env": {}, + "env": { + "UNRELATED_ENVVAR": "unrelated data" + }, "otel": { "is_sampled": "false", "span_id": "0000000000000000", @@ -21,4 +23,4 @@ "timeout": 0, "should_timeout": false, "skip_otel_cli": true -} \ No newline at end of file +} diff --git a/cmd/test-otel-init-go/main_test.go b/cmd/test-otel-init-go/main_test.go index 1a0a7b7..fd245d8 100644 --- a/cmd/test-otel-init-go/main_test.go +++ b/cmd/test-otel-init-go/main_test.go @@ -20,6 +20,7 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "strconv" "strings" "testing" @@ -114,9 +115,26 @@ func TestOtelInit(t *testing.T) { } } -// TODO: write these checks :) +// checkData takes the data returned from the stub and compares it to the +// preset data in the scenario and fails the tests if anything doesn't match. func checkData(t *testing.T, scenario Scenario, stubData StubData, events CliEvents) { + // check the env + if !reflect.DeepEqual(stubData.Env, scenario.StubData.Env) { + t.Log("env in stub output did not match test fixture") + t.Fail() + } + + // check the otel-init-go config + if !reflect.DeepEqual(stubData.Config, scenario.StubData.Config) { + t.Log("config in stub output did not match test fixture") + t.Fail() + } + // check the otel span values + if !reflect.DeepEqual(stubData.Otel, scenario.StubData.Otel) { + t.Log("span in stub output did not match test fixture") + t.Fail() + } } // runPrograms runs the stub program and otel-cli together and captures their From 9a395d97f25f876766772b5b874a709c31c08e04 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 15:00:46 -0700 Subject: [PATCH 12/27] move fixtures to testdata/, clean up Now that this seems to be working, move the test fixtures into the testdata/ directory that is a Go standard for things like fixture data. Along the way remove printf stuff that's no longer needed. Also map filename through in Scenario struct so it can be used in errors. Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main_test.go | 21 ++++++++----------- .../{ => testdata}/00-unconfigured.json | 0 .../{ => testdata}/01-irrelevant-envvar.json | 0 3 files changed, 9 insertions(+), 12 deletions(-) rename cmd/test-otel-init-go/{ => testdata}/00-unconfigured.json (100%) rename cmd/test-otel-init-go/{ => testdata}/01-irrelevant-envvar.json (100%) diff --git a/cmd/test-otel-init-go/main_test.go b/cmd/test-otel-init-go/main_test.go index fd245d8..72eea4a 100644 --- a/cmd/test-otel-init-go/main_test.go +++ b/cmd/test-otel-init-go/main_test.go @@ -58,6 +58,7 @@ type StubData struct { // are found in json files in this directory. type Scenario struct { Name string `json:"name"` + Filename string `json:"-"` StubEnv map[string]string `json:"stub_env"` // given to stub StubData StubData `json:"stub_data"` // data from stub, exact match SpansExpected int `json:"spans_expected"` @@ -78,7 +79,7 @@ func TestOtelInit(t *testing.T) { // get a list of all json files in this directory // https://dave.cheney.net/2016/05/10/test-fixtures-in-go wd, _ := os.Getwd() - files, err := ioutil.ReadDir(wd) + files, err := ioutil.ReadDir(filepath.Join(wd, "testdata")) if err != nil { t.Fatalf("Failed to list test directory %q to detect json files.", wd) } @@ -87,7 +88,8 @@ func TestOtelInit(t *testing.T) { for _, file := range files { if strings.HasSuffix(file.Name(), ".json") { scenario := Scenario{StubEnv: map[string]string{}} - js, err := os.ReadFile(file.Name()) + fp := filepath.Join("testdata", file.Name()) + js, err := os.ReadFile(fp) if err != nil { t.Fatalf("Failed to read json test file %q: %s", file.Name(), err) } @@ -95,23 +97,20 @@ func TestOtelInit(t *testing.T) { if err != nil { t.Fatalf("Failed to parse json test file %q: %s", file.Name(), err) } + scenario.Filename = filepath.Base(file.Name()) // for error reporting scenarios = append(scenarios, scenario) } } t.Logf("Loaded %d tests.", len(scenarios)) + if len(scenarios) == 0 { + t.Fatal("no test fixtures loaded!") + } // run all the scenarios for _, s := range scenarios { stubData, events := runPrograms(t, s) checkData(t, s, stubData, events) - - // temporary debug print - for tid, trace := range events { - for sid, span := range trace { - t.Logf("trace id %s span id %s is %q", tid, sid, span) - } - } } } @@ -120,7 +119,7 @@ func TestOtelInit(t *testing.T) { func checkData(t *testing.T, scenario Scenario, stubData StubData, events CliEvents) { // check the env if !reflect.DeepEqual(stubData.Env, scenario.StubData.Env) { - t.Log("env in stub output did not match test fixture") + t.Logf("env in stub output did not match test fixture in %q", stubData) t.Fail() } @@ -181,8 +180,6 @@ func runPrograms(t *testing.T, scenario Scenario) (StubData, CliEvents) { t.Fatalf("Executing stub command %q failed: %s", stub.String(), err) } - fmt.Printf("\n\n%s\n\n", string(stubOut)) - stubData := StubData{ Config: map[string]string{}, Env: map[string]string{}, diff --git a/cmd/test-otel-init-go/00-unconfigured.json b/cmd/test-otel-init-go/testdata/00-unconfigured.json similarity index 100% rename from cmd/test-otel-init-go/00-unconfigured.json rename to cmd/test-otel-init-go/testdata/00-unconfigured.json diff --git a/cmd/test-otel-init-go/01-irrelevant-envvar.json b/cmd/test-otel-init-go/testdata/01-irrelevant-envvar.json similarity index 100% rename from cmd/test-otel-init-go/01-irrelevant-envvar.json rename to cmd/test-otel-init-go/testdata/01-irrelevant-envvar.json From c7c0106ab8731aa4936ea855187acfd8cda84c03 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 17:16:42 -0700 Subject: [PATCH 13/27] add a full-loop test using otel-cli Discovered I needed to add a wildcard match for especially trace and span ids, but did it for the whole map while I was there. It's not the prettiest approach but I tried writing a cmp.Comparator and it didn't work out, so for now this gets the job done nicely. Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main_test.go | 79 +++++++++++++++---- .../testdata/10-local-server-no-tls.json | 28 +++++++ go.mod | 1 + 3 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 cmd/test-otel-init-go/testdata/10-local-server-no-tls.json diff --git a/cmd/test-otel-init-go/main_test.go b/cmd/test-otel-init-go/main_test.go index 72eea4a..e201303 100644 --- a/cmd/test-otel-init-go/main_test.go +++ b/cmd/test-otel-init-go/main_test.go @@ -20,11 +20,13 @@ import ( "os" "os/exec" "path/filepath" - "reflect" + "regexp" "strconv" "strings" "testing" "time" + + "github.com/google/go-cmp/cmp" ) // CliEvent is mostly the same as otel-cli's internal event format, with @@ -48,10 +50,11 @@ type CliEvents map[string]map[string]CliEvent // StubData is the structure of the data that the stub program // prints out. +type StubSpan map[string]string type StubData struct { Config map[string]string `json:"config"` Env map[string]string `json:"env"` - Otel map[string]string `json:"otel"` + Otel StubSpan `json:"otel"` } // Scenario represents the configuration of a test scenario. Scenarios @@ -76,7 +79,7 @@ func TestMain(m *testing.M) { // TestOtelInit loads all the json files in this directory and executes the // tests they define. func TestOtelInit(t *testing.T) { - // get a list of all json files in this directory + // get a list of all json fixtures in the testdata directory // https://dave.cheney.net/2016/05/10/test-fixtures-in-go wd, _ := os.Getwd() files, err := ioutil.ReadDir(filepath.Join(wd, "testdata")) @@ -107,7 +110,7 @@ func TestOtelInit(t *testing.T) { t.Fatal("no test fixtures loaded!") } - // run all the scenarios + // run all the scenarios, check the results for _, s := range scenarios { stubData, events := runPrograms(t, s) checkData(t, s, stubData, events) @@ -118,23 +121,69 @@ func TestOtelInit(t *testing.T) { // preset data in the scenario and fails the tests if anything doesn't match. func checkData(t *testing.T, scenario Scenario, stubData StubData, events CliEvents) { // check the env - if !reflect.DeepEqual(stubData.Env, scenario.StubData.Env) { - t.Logf("env in stub output did not match test fixture in %q", stubData) - t.Fail() + if diff := cmp.Diff(scenario.StubData.Env, stubData.Env); diff != "" { + t.Errorf("env data did not match fixture in %q (-want +got):\n%s", scenario.Filename, diff) } // check the otel-init-go config - if !reflect.DeepEqual(stubData.Config, scenario.StubData.Config) { - t.Log("config in stub output did not match test fixture") - t.Fail() + if diff := cmp.Diff(scenario.StubData.Config, stubData.Config); diff != "" { + t.Errorf("config data did not match fixture in %q (-want +got):\n%s", scenario.Filename, diff) } // check the otel span values - if !reflect.DeepEqual(stubData.Otel, scenario.StubData.Otel) { - t.Log("span in stub output did not match test fixture") - t.Fail() + // find usages of *, do the check on the stub data manually, and set up cmpSpan + scSpan := map[string]string{} // to be passed to cmp.Diff + cmpSpan := map[string]string{} // to be passed to cmp.Diff + for what, re := range map[string]*regexp.Regexp{ + "trace_id": regexp.MustCompile("^[0-9a-fA-F]{32}$"), + "span_id": regexp.MustCompile("^[0-9a-fA-F]{16}$"), + "is_sampled": regexp.MustCompile("^true|false$"), + "trace_flags": regexp.MustCompile("^[0-9]{2}$"), + } { + if cv, ok := scenario.StubData.Otel[what]; ok { + scSpan[what] = cv // make a straight copy to make cmp.Diff happy + if sv, ok := stubData.Otel[what]; ok { + cmpSpan[what] = sv // default to the existing value + if cv == "*" { + if re.MatchString(sv) { + cmpSpan[what] = "*" // success!, make the Cmp test succeed + } else { + t.Errorf("stub span value %q for key %s is not valid", sv, what) + } + } + } + } + } + + // do a diff on a generated map that sets values to * when the * check succeeded + if diff := cmp.Diff(scSpan, cmpSpan); diff != "" { + t.Errorf("otel data did not match fixture in %q (-want +got):\n%s", scenario.Filename, diff) + } +} + +// checkOtelSplat is a helper for checking trace and span id in the otel output +// so that the fixtures can put "*" in those fields to mean "any valid-looking id" +// TODO: maybe can use cmp custom comparator to implement this cleaner in the diff +/* +func checkOtelSplat(t *testing.T, what string, re *regexp.Regexp, scenario Scenario, stubData *StubData) bool { + if v, ok := scenario.StubData.Otel[what]; ok { + if v == "*" { + if sv, ok := stubData.Otel[what]; ok { + if re.MatchString(sv) { + // override the * so the following diff test passes ok + scenario.StubData.Otel[what] = sv + + return true + } else { + t.Errorf("%s id %q does not look like a valid id", what, sv) + } + } + } } + + return false } +*/ // runPrograms runs the stub program and otel-cli together and captures their // output as data to return for further testing. @@ -159,11 +208,13 @@ func runPrograms(t *testing.T, scenario Scenario) (StubData, CliEvents) { // MAYBE: server json --stdout is maybe better? and could add a graceful exit on closed fds // TODO: obviously this is horrible otelcli := exec.Command("/home/atobey/src/otel-cli/otel-cli", cliArgs...) + otelcli.Env = []string{"PATH=/bin"} // apparently this is required for 'getent', no idea why if !scenario.SkipOtelCli { go func() { - err = otelcli.Run() + err, output := otelcli.CombinedOutput() if err != nil { + log.Println(output) log.Fatalf("Executing command %q failed: %s", otelcli.String(), err) } }() diff --git a/cmd/test-otel-init-go/testdata/10-local-server-no-tls.json b/cmd/test-otel-init-go/testdata/10-local-server-no-tls.json new file mode 100644 index 0000000..3ad2288 --- /dev/null +++ b/cmd/test-otel-init-go/testdata/10-local-server-no-tls.json @@ -0,0 +1,28 @@ +{ + "name": "local server without tls", + "stub_env": { + "OTEL_EXPORTER_OTLP_ENDPOINT": "localhost:4317", + "OTEL_EXPORTER_OTLP_INSECURE": "true" + }, + "stub_data": { + "config": { + "endpoint": "localhost:4317", + "insecure": "true", + "service_name": "otel-init-go-test" + }, + "env": { + "OTEL_EXPORTER_OTLP_ENDPOINT": "localhost:4317", + "OTEL_EXPORTER_OTLP_INSECURE": "true" + }, + "otel": { + "is_sampled": "true", + "span_id": "*", + "trace_flags": "01", + "trace_id": "*" + } + }, + "spans_expected": 1, + "timeout": 1, + "should_timeout": false, + "skip_otel_cli": false +} diff --git a/go.mod b/go.mod index 4333e75..09dcc00 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/equinix-labs/otel-init-go go 1.15 require ( + github.com/google/go-cmp v0.5.6 // indirect go.opentelemetry.io/otel v1.0.0-RC2 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.0.0-RC2 go.opentelemetry.io/otel/sdk v1.0.0-RC2 From 2945fe14c174c3be317e0ecd769190c814c42492 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 17:19:50 -0700 Subject: [PATCH 14/27] add basic github actions test runner Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..4febb76 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,20 @@ +name: CI +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] +jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v2 + - name: Setup + uses: actions/setup-go@v2 + with: + go-version: 1.16 + - name: Build + run: go build -v ./... + - name: Test + run: go test -v ./... From 0a26dbe01839f2d45e354f7a65a859a2d54cea79 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 17:35:48 -0700 Subject: [PATCH 15/27] add unit tests for otelinit/config.go --- otelinit/config_test.go | 95 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 otelinit/config_test.go diff --git a/otelinit/config_test.go b/otelinit/config_test.go new file mode 100644 index 0000000..a90067a --- /dev/null +++ b/otelinit/config_test.go @@ -0,0 +1,95 @@ +package otelinit + +import ( + "os" + "testing" + + "github.com/google/go-cmp/cmp" +) + +var testServiceName = "unitTestService" + +func TestNewConfig(t *testing.T) { + tests := map[string]struct { + envIn map[string]string + wantConfig Config + }{ + "empty env gets empty config": { + envIn: map[string]string{}, + wantConfig: Config{ + Servicename: testServiceName, + }, + }, + "irrelevant envvar changes nothing": { + envIn: map[string]string{ + "OTEL_SOMETHING_SOMETHING": "this should impact nothing", + }, + wantConfig: Config{ + Servicename: testServiceName, + }, + }, + "insecure false stays false": { + envIn: map[string]string{ + "OTEL_EXPORTER_OTLP_INSECURE": "false", + }, + wantConfig: Config{ + Servicename: testServiceName, + Insecure: false, + }, + }, + "insecure true configs true": { + envIn: map[string]string{ + "OTEL_EXPORTER_OTLP_INSECURE": "true", + }, + wantConfig: Config{ + Servicename: testServiceName, + Insecure: true, + }, + }, + // this is by far the most common configuration expected + "otlp endpoint and insecure": { + envIn: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "localhost:4317", + "OTEL_EXPORTER_OTLP_INSECURE": "true", + }, + wantConfig: Config{ + Servicename: testServiceName, + Insecure: true, + Endpoint: "localhost:4317", + }, + }, + // TODO: maybe should NOT do this, and have newConfig() check + // incoming values and ignore obviously bad ones + "otlp endpoint allows arbitrary value": { + envIn: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "asdf asdf asdf", + }, + wantConfig: Config{ + Servicename: testServiceName, + Insecure: false, + Endpoint: "asdf asdf asdf", + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + // wipes the whole process's env every test run + os.Clearenv() + // set up the test envvars + for k, v := range tc.envIn { + err := os.Setenv(k, v) + if err != nil { + t.Fatalf("could not set test environment: %s", err) + } + } + // generate a config + c := newConfig(testServiceName) + // see if it's any good + if diff := cmp.Diff(c, tc.wantConfig); diff != "" { + t.Errorf(diff) + } + }) + } + +} From 61a34aac6961f2d6a706367c5b72106d38b19fa5 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 17:38:40 -0700 Subject: [PATCH 16/27] add godoc to type --- otelinit/public.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/otelinit/public.go b/otelinit/public.go index 20889eb..33271db 100644 --- a/otelinit/public.go +++ b/otelinit/public.go @@ -2,6 +2,9 @@ package otelinit import "context" +// OtelShutdown is a function that should be called with context +// when you want to shut down OpenTelemetry, usually as a defer +// in main. type OtelShutdown func(context.Context) // InitOpenTelemetry sets up the OpenTelemetry plumbing so it's ready to use. From da8a9c7f2ea796f10578ea9d0ac6921fe9a3a710 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 17:52:11 -0700 Subject: [PATCH 17/27] try not to leak envvars prefixed with GITHUB This could be better but for now should filter out most of the noise and dangerous stuff. And it's a test program so maybe it's ok to be a leetle gross. Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/test-otel-init-go/main.go b/cmd/test-otel-init-go/main.go index 95b5dbf..0097820 100644 --- a/cmd/test-otel-init-go/main.go +++ b/cmd/test-otel-init-go/main.go @@ -28,7 +28,12 @@ func main() { for _, e := range os.Environ() { parts := strings.SplitN(e, "=", 2) if len(parts) == 2 { - env[parts[0]] = parts[1] + // it's not great to hard-code a specific envvar here but this is + // probably the most dangerous one we don't want to blithely print + // here. open to suggestions... + if !strings.HasPrefix(parts[0], "GITHUB") { + env[parts[0]] = parts[1] + } } else { log.Fatalf("BUG this shouldn't happen") } From 8fa63e5cd2ecca0deb171f66f61e1b26f970ada2 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:15:09 -0700 Subject: [PATCH 18/27] get rid of hard-coded paths The setup for the test stub is a little gross but seems to work ok. This is better than a hard-coded path, so let's see how it goes. Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main_test.go | 61 +++++++++++++++++------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/cmd/test-otel-init-go/main_test.go b/cmd/test-otel-init-go/main_test.go index e201303..1e3921f 100644 --- a/cmd/test-otel-init-go/main_test.go +++ b/cmd/test-otel-init-go/main_test.go @@ -7,7 +7,6 @@ package main // pan out but the first part is looking good // // TODOs: -// [ ] write data tests // [ ] replace that time.Sleep with proper synchronization // [ ] use random ports for listener address? @@ -70,7 +69,39 @@ type Scenario struct { SkipOtelCli bool `json:"skip_otel_cli"` // don't run otel-cli at all } +// needs to be discovered right at startup before env is cleared +var otelCliPath, testStubPath string + func TestMain(m *testing.M) { + // find otel-cli in PATH before clearing the environment + var err error + otelCliPath, err = exec.LookPath("otel-cli") + if err != nil { + log.Fatalf("cannot run tests: otel-cli must be in PATH: %s", err) + } + + // this is an unusual test in that it needs the command built + // to run so we do that here + goCmdPath, err := exec.LookPath("go") + if err != nil { + log.Fatalf("could not locate 'go' command in PATH: %s", err) + } + // should already be in the right directory + err = exec.Command(goCmdPath, "build").Run() + if err != nil { + log.Fatalf("failed to build stub program: %s", err) + } + // hacky: add cwd to PATH so we can do a lookup, mostly because this + // gets around e.g. Windows.exe + wd, _ := os.Getwd() + path := os.Getenv("PATH") + os.Setenv("PATH", wd+string(os.PathListSeparator)+path) + // finally, set the var + testStubPath, err = exec.LookPath("test-otel-init-go") + if err != nil { + log.Fatalf("could not locate 'test-otel-init-go' command in PATH: %s", err) + } + // wipe out this process's envvars right away to avoid pollution & leakage os.Clearenv() os.Exit(m.Run()) @@ -161,30 +192,6 @@ func checkData(t *testing.T, scenario Scenario, stubData StubData, events CliEve } } -// checkOtelSplat is a helper for checking trace and span id in the otel output -// so that the fixtures can put "*" in those fields to mean "any valid-looking id" -// TODO: maybe can use cmp custom comparator to implement this cleaner in the diff -/* -func checkOtelSplat(t *testing.T, what string, re *regexp.Regexp, scenario Scenario, stubData *StubData) bool { - if v, ok := scenario.StubData.Otel[what]; ok { - if v == "*" { - if sv, ok := stubData.Otel[what]; ok { - if re.MatchString(sv) { - // override the * so the following diff test passes ok - scenario.StubData.Otel[what] = sv - - return true - } else { - t.Errorf("%s id %q does not look like a valid id", what, sv) - } - } - } - } - - return false -} -*/ - // runPrograms runs the stub program and otel-cli together and captures their // output as data to return for further testing. // all failures are fatal, no point in testing if this is broken @@ -207,7 +214,7 @@ func runPrograms(t *testing.T, scenario Scenario) (StubData, CliEvents) { // MAYBE: server json --stdout is maybe better? and could add a graceful exit on closed fds // TODO: obviously this is horrible - otelcli := exec.Command("/home/atobey/src/otel-cli/otel-cli", cliArgs...) + otelcli := exec.Command(otelCliPath, cliArgs...) otelcli.Env = []string{"PATH=/bin"} // apparently this is required for 'getent', no idea why if !scenario.SkipOtelCli { @@ -224,7 +231,7 @@ func runPrograms(t *testing.T, scenario Scenario) (StubData, CliEvents) { time.Sleep(time.Millisecond * 10) // TODO: obviously this is horrible - stub := exec.Command("/home/atobey/src/otel-init-go/cmd/test-otel-init-go/test-otel-init-go") + stub := exec.Command(testStubPath) stub.Env = mkEnviron(scenario.StubEnv) stubOut, err := stub.Output() if err != nil { From 38a6c3f8ba2f1dc3b0dd74569ee1a71ef1617337 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:15:50 -0700 Subject: [PATCH 19/27] download the latest otel-cli release for tests Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4febb76..6cf6223 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,5 +16,9 @@ jobs: go-version: 1.16 - name: Build run: go build -v ./... + - name: Install otel-cli for tests + uses: jaxxstorm/action-install-gh-release@release/v1-alpha + with: + repo: equinix-labs/otel-cli - name: Test run: go test -v ./... From bf0c89b006154714304eba3c0b6e2ba94b6edf47 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:24:20 -0700 Subject: [PATCH 20/27] try the engineerd/configurator downloader instead Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6cf6223..059dcff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,8 +17,9 @@ jobs: - name: Build run: go build -v ./... - name: Install otel-cli for tests - uses: jaxxstorm/action-install-gh-release@release/v1-alpha + uses: engineerd/[email protected] with: - repo: equinix-labs/otel-cli + name: "otel-cli" + url: "https://github.com/equinix-labs/otel-cli/releases/download/v0.0.5/otel-cli-0.0.5-Linux-x86_64.tar.gz" - name: Test run: go test -v ./... From 39d5de65338ef980edf4ee1842e90c5f907b885d Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:30:26 -0700 Subject: [PATCH 21/27] try templating the url & set token Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 059dcff..41428ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,12 +14,17 @@ jobs: uses: actions/setup-go@v2 with: go-version: 1.16 - - name: Build - run: go build -v ./... - - name: Install otel-cli for tests uses: engineerd/[email protected] with: name: "otel-cli" - url: "https://github.com/equinix-labs/otel-cli/releases/download/v0.0.5/otel-cli-0.0.5-Linux-x86_64.tar.gz" + pathInArchive: /otel-cli + fromGitHubReleases: "true" + repo: equinix-labs/otel-cli + version: "^v0.0.5" + urlTemplate: "https://github.com/equinix-labs/otel-cli/releases/download/{{ version }}/otel-cli-{{ version }}-Linux-x86_64.tar.gz" + token: ${{ secrets.GTIHUB_TOKEN }} + - name: Build + run: go build -v ./... + - name: Install otel-cli for tests - name: Test run: go test -v ./... From 39ee3f30cf176638e9b8517b35b04278bb2f9ef7 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:32:14 -0700 Subject: [PATCH 22/27] fix ordering Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 41428ec..ffd4537 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,6 +14,7 @@ jobs: uses: actions/setup-go@v2 with: go-version: 1.16 + - name: Install otel-cli for tests uses: engineerd/[email protected] with: name: "otel-cli" @@ -25,6 +26,5 @@ jobs: token: ${{ secrets.GTIHUB_TOKEN }} - name: Build run: go build -v ./... - - name: Install otel-cli for tests - name: Test run: go test -v ./... From d722e1f9f8d3e9b8ab4d1c2c21102d6247ed5411 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:35:30 -0700 Subject: [PATCH 23/27] avoid releases to avoid needing a token for now Looks like I need to set up token passing for the release-based download to work. Instead, for now, just download the tarball and pull otel-cli out. Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ffd4537..8b15549 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,11 +19,7 @@ jobs: with: name: "otel-cli" pathInArchive: /otel-cli - fromGitHubReleases: "true" - repo: equinix-labs/otel-cli - version: "^v0.0.5" - urlTemplate: "https://github.com/equinix-labs/otel-cli/releases/download/{{ version }}/otel-cli-{{ version }}-Linux-x86_64.tar.gz" - token: ${{ secrets.GTIHUB_TOKEN }} + url: "https://github.com/equinix-labs/otel-cli/releases/download/v0.0.5/otel-cli-0.0.5-Linux-x86_64.tar.gz" - name: Build run: go build -v ./... - name: Test From 65b3c1038233d5c10a56db2ff57b45f604c95c10 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:42:52 -0700 Subject: [PATCH 24/27] simplify stub path, have CI build stub before test Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 5 ++++- cmd/test-otel-init-go/main_test.go | 22 ++-------------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b15549..e8d4ec9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,13 +14,16 @@ jobs: uses: actions/setup-go@v2 with: go-version: 1.16 + - name: Build test stub + path: cmd/test-otel-init-go + run: go build -v - name: Install otel-cli for tests uses: engineerd/[email protected] with: name: "otel-cli" pathInArchive: /otel-cli url: "https://github.com/equinix-labs/otel-cli/releases/download/v0.0.5/otel-cli-0.0.5-Linux-x86_64.tar.gz" - - name: Build + - name: Build otelinit run: go build -v ./... - name: Test run: go test -v ./... diff --git a/cmd/test-otel-init-go/main_test.go b/cmd/test-otel-init-go/main_test.go index 1e3921f..02d775a 100644 --- a/cmd/test-otel-init-go/main_test.go +++ b/cmd/test-otel-init-go/main_test.go @@ -80,27 +80,9 @@ func TestMain(m *testing.M) { log.Fatalf("cannot run tests: otel-cli must be in PATH: %s", err) } - // this is an unusual test in that it needs the command built - // to run so we do that here - goCmdPath, err := exec.LookPath("go") - if err != nil { - log.Fatalf("could not locate 'go' command in PATH: %s", err) - } - // should already be in the right directory - err = exec.Command(goCmdPath, "build").Run() - if err != nil { - log.Fatalf("failed to build stub program: %s", err) - } - // hacky: add cwd to PATH so we can do a lookup, mostly because this - // gets around e.g. Windows.exe + // it is expected that the stub binary has already been built and CI does this wd, _ := os.Getwd() - path := os.Getenv("PATH") - os.Setenv("PATH", wd+string(os.PathListSeparator)+path) - // finally, set the var - testStubPath, err = exec.LookPath("test-otel-init-go") - if err != nil { - log.Fatalf("could not locate 'test-otel-init-go' command in PATH: %s", err) - } + testStubPath = filepath.Join(wd, "test-otel-init-go") // wipe out this process's envvars right away to avoid pollution & leakage os.Clearenv() From ded69d4d2740c33dcb64f84e6f9bd845991b702f Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:44:41 -0700 Subject: [PATCH 25/27] try working-directory instead of path Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8d4ec9..5b52484 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: with: go-version: 1.16 - name: Build test stub - path: cmd/test-otel-init-go + working-directory: ./cmd/test-otel-init-go run: go build -v - name: Install otel-cli for tests uses: engineerd/[email protected] From 09e88c70cb9bf3747c47d4746ff36d180604e4d1 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 18:45:57 -0700 Subject: [PATCH 26/27] move build up Signed-off-by: Amy Tobey --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5b52484..ea3f4f2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,6 +14,8 @@ jobs: uses: actions/setup-go@v2 with: go-version: 1.16 + - name: Build otelinit + run: go build -v ./... - name: Build test stub working-directory: ./cmd/test-otel-init-go run: go build -v @@ -23,7 +25,5 @@ jobs: name: "otel-cli" pathInArchive: /otel-cli url: "https://github.com/equinix-labs/otel-cli/releases/download/v0.0.5/otel-cli-0.0.5-Linux-x86_64.tar.gz" - - name: Build otelinit - run: go build -v ./... - name: Test run: go test -v ./... From ac348065c4136d7c084891c242fdd8af0101ea45 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Mon, 23 Aug 2021 19:12:31 -0700 Subject: [PATCH 27/27] fix up horrible comments Signed-off-by: Amy Tobey --- cmd/test-otel-init-go/main_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/test-otel-init-go/main_test.go b/cmd/test-otel-init-go/main_test.go index 02d775a..686bb41 100644 --- a/cmd/test-otel-init-go/main_test.go +++ b/cmd/test-otel-init-go/main_test.go @@ -195,7 +195,6 @@ func runPrograms(t *testing.T, scenario Scenario) (StubData, CliEvents) { } // MAYBE: server json --stdout is maybe better? and could add a graceful exit on closed fds - // TODO: obviously this is horrible otelcli := exec.Command(otelCliPath, cliArgs...) otelcli.Env = []string{"PATH=/bin"} // apparently this is required for 'getent', no idea why @@ -209,10 +208,13 @@ func runPrograms(t *testing.T, scenario Scenario) (StubData, CliEvents) { }() } - // yes yes I know this is horrible + // TODO: replace this with something more reliable + // the problem here is we need to wait for otel-cli to start and listen + // so the solution is probably to do some kind of healthcheck on otel-cli's port + // but this works ok for now time.Sleep(time.Millisecond * 10) - // TODO: obviously this is horrible + // run the stub with the scenario's environment stub := exec.Command(testStubPath) stub.Env = mkEnviron(scenario.StubEnv) stubOut, err := stub.Output()