From 819cae9c80deec2bc4708372ecca92dd0408280a Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Fri, 23 Jun 2023 12:44:32 -0400 Subject: [PATCH] separate w3c traceparent to its own package (#225) The traceparent code was still sitting in otlpclient after the breakup of CLI and OTLP because it was kinda a toss-up where it should land at that stage. It is now in its own package. The w3c directory is there, to make it clear it's not an otel-specific thing, and as a place to put a baggage API that's next after this PR merges. I also made a small change to functional tests to reflect a behavior change. The way trace ids and spans were printed in comments in non-recording mode. The comments no longer show a zero-valued trace id in non-recording mode and instead show the traceparent's trace & span id. In recording mode behavior is unchanged. No new functionality. No changes to functional tests. Squashed commitlog follows: * move files * refactor w3c/traceparent so it stands alone Still some leftover commented code that will move to protobuf span code. * move span-dependent code to otlpclient/protobuf_span * finish refactoring traceparent to its own package * functionality change: just print the tp trace/span id Making it work consistently was a pain and the code was ugly so I decided to drop the feature where span/trace id would be zeroed if the trace isn't recording. * add testing & fix bugs & clean up --- data_for_test.go | 12 +- otelcli/exec.go | 4 +- otelcli/span.go | 3 +- otelcli/span_background_server.go | 2 +- otelcli/span_end.go | 5 +- otelcli/span_event.go | 5 +- otelcli/status.go | 3 +- otlpclient/protobuf_span.go | 82 ++++++++- otlpclient/protobuf_span_test.go | 37 +++++ otlpclient/traceparent_test.go | 126 -------------- .../traceparent}/traceparent.go | 157 ++++++------------ w3c/traceparent/traceparent_test.go | 154 +++++++++++++++++ 12 files changed, 338 insertions(+), 252 deletions(-) delete mode 100644 otlpclient/traceparent_test.go rename {otlpclient => w3c/traceparent}/traceparent.go (50%) create mode 100644 w3c/traceparent/traceparent_test.go diff --git a/data_for_test.go b/data_for_test.go index 819c57e..2809780 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -592,8 +592,8 @@ var suites = []FixtureSuite{ Expect: Results{ Config: otlpclient.DefaultConfig(), CliOutput: "" + // empty so the text below can indent and line up - "# trace id: 00000000000000000000000000000000\n" + - "# span id: 0000000000000000\n" + + "# trace id: f6c109f48195b451c4def6ab32f47b61\n" + + "# span id: a5d2a35f2483004e\n" + "TRACEPARENT=00-f6c109f48195b451c4def6ab32f47b61-a5d2a35f2483004e-01\n", }, }, @@ -605,15 +605,15 @@ var suites = []FixtureSuite{ Config: FixtureConfig{ CliArgs: []string{"span", "--tp-print", "--tp-export"}, Env: map[string]string{ - "TRACEPARENT": "00-f6c109f48195b451c4def6ab32f47b61-a5d2a35f2483004e-01", + "TRACEPARENT": "00-f6c109f48195b451c4def6ab32f47b61-a5d2a35f2483004e-00", }, }, Expect: Results{ Config: otlpclient.DefaultConfig(), CliOutput: "" + - "# trace id: 00000000000000000000000000000000\n" + - "# span id: 0000000000000000\n" + - "export TRACEPARENT=00-f6c109f48195b451c4def6ab32f47b61-a5d2a35f2483004e-01\n", + "# trace id: f6c109f48195b451c4def6ab32f47b61\n" + + "# span id: a5d2a35f2483004e\n" + + "export TRACEPARENT=00-f6c109f48195b451c4def6ab32f47b61-a5d2a35f2483004e-00\n", }, }, }, diff --git a/otelcli/exec.go b/otelcli/exec.go index b43f2b1..f99c658 100644 --- a/otelcli/exec.go +++ b/otelcli/exec.go @@ -83,11 +83,11 @@ func doExec(cmd *cobra.Command, args []string) { // set the traceparent to the current span to be available to the child process if config.IsRecording() { - tp := otlpclient.TraceparentFromSpan(span) + tp := otlpclient.TraceparentFromProtobufSpan(config, span) child.Env = append(child.Env, fmt.Sprintf("TRACEPARENT=%s", tp.Encode())) // when not recording, and a traceparent is available, pass it through } else if !config.TraceparentIgnoreEnv { - tp := otlpclient.LoadTraceparent(config) + tp := otlpclient.LoadTraceparent(config, span) if tp.Initialized { child.Env = append(child.Env, fmt.Sprintf("TRACEPARENT=%s", tp.Encode())) } diff --git a/otelcli/span.go b/otelcli/span.go index de8223d..447702a 100644 --- a/otelcli/span.go +++ b/otelcli/span.go @@ -49,6 +49,7 @@ func doSpan(cmd *cobra.Command, args []string) { config := getConfig(ctx) ctx, client := otlpclient.StartClient(ctx, config) span := otlpclient.NewProtobufSpanWithConfig(config) - otlpclient.SendSpan(ctx, client, config, span) + err := otlpclient.SendSpan(ctx, client, config, span) + config.SoftFailIfErr(err) otlpclient.PropagateTraceparent(config, span, os.Stdout) } diff --git a/otelcli/span_background_server.go b/otelcli/span_background_server.go index ffa7bde..363bcfe 100644 --- a/otelcli/span_background_server.go +++ b/otelcli/span_background_server.go @@ -44,7 +44,7 @@ type BgEnd struct { func (bs BgSpan) AddEvent(bse *BgSpanEvent, reply *BgSpan) error { reply.TraceID = hex.EncodeToString(bs.span.TraceId) reply.SpanID = hex.EncodeToString(bs.span.SpanId) - reply.Traceparent = otlpclient.TraceparentFromSpan(bs.span).Encode() + reply.Traceparent = otlpclient.TraceparentFromProtobufSpan(bs.config, bs.span).Encode() ts, err := time.Parse(time.RFC3339Nano, bse.Timestamp) if err != nil { diff --git a/otelcli/span_end.go b/otelcli/span_end.go index 5ad3b9c..4dde467 100644 --- a/otelcli/span_end.go +++ b/otelcli/span_end.go @@ -4,6 +4,7 @@ import ( "os" "github.com/equinix-labs/otel-cli/otlpclient" + "github.com/equinix-labs/otel-cli/w3c/traceparent" "github.com/spf13/cobra" ) @@ -52,8 +53,8 @@ func doSpanEnd(cmd *cobra.Command, args []string) { } shutdown() - tp, _ := otlpclient.ParseTraceparent(res.Traceparent) + tp, _ := traceparent.Parse(res.Traceparent) if config.TraceparentPrint { - otlpclient.PrintSpanData(os.Stdout, tp, nil, config.TraceparentPrintExport) + tp.Fprint(os.Stdout, config.TraceparentPrintExport) } } diff --git a/otelcli/span_event.go b/otelcli/span_event.go index 4ff4c39..81e689f 100644 --- a/otelcli/span_event.go +++ b/otelcli/span_event.go @@ -5,6 +5,7 @@ import ( "time" "github.com/equinix-labs/otel-cli/otlpclient" + "github.com/equinix-labs/otel-cli/w3c/traceparent" "github.com/spf13/cobra" ) @@ -63,10 +64,10 @@ func doSpanEvent(cmd *cobra.Command, args []string) { } if config.TraceparentPrint { - tp, err := otlpclient.ParseTraceparent(res.Traceparent) + tp, err := traceparent.Parse(res.Traceparent) if err != nil { config.SoftFail("Could not parse traceparent: %s", err) } - otlpclient.PrintSpanData(os.Stdout, tp, nil, config.TraceparentPrintExport) + tp.Fprint(os.Stdout, config.TraceparentPrintExport) } } diff --git a/otelcli/status.go b/otelcli/status.go index b07b5c0..471075f 100644 --- a/otelcli/status.go +++ b/otelcli/status.go @@ -79,7 +79,6 @@ func doStatus(cmd *cobra.Command, args []string) { config.SoftLog("%s", err) } } - tp := otlpclient.TraceparentFromSpan(span) outData := StatusOutput{ Config: config, @@ -87,7 +86,7 @@ func doStatus(cmd *cobra.Command, args []string) { SpanData: map[string]string{ "trace_id": hex.EncodeToString(span.TraceId), "span_id": hex.EncodeToString(span.SpanId), - "is_sampled": strconv.FormatBool(tp.Sampling), + "is_sampled": strconv.FormatBool(config.IsRecording()), }, Diagnostics: otlpclient.Diag, } diff --git a/otlpclient/protobuf_span.go b/otlpclient/protobuf_span.go index 8ed09ad..9b6b8fd 100644 --- a/otlpclient/protobuf_span.go +++ b/otlpclient/protobuf_span.go @@ -10,13 +10,18 @@ import ( "crypto/rand" "encoding/hex" "fmt" + "io" "strconv" "time" + "github.com/equinix-labs/otel-cli/w3c/traceparent" commonpb "go.opentelemetry.io/proto/otlp/common/v1" tracepb "go.opentelemetry.io/proto/otlp/trace/v1" ) +var emptyTraceId = []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} +var emptySpanId = []byte{0, 0, 0, 0, 0, 0, 0, 0} + // NewProtobufSpan returns an initialized OpenTelemetry protobuf Span. func NewProtobufSpan() *tracepb.Span { now := time.Now() @@ -80,11 +85,11 @@ func NewProtobufSpanWithConfig(c Config) *tracepb.Span { } if c.IsRecording() { - if tp := LoadTraceparent(c); tp.Initialized { + tp := LoadTraceparent(c, span) + if tp.Initialized { span.TraceId = tp.TraceId span.ParentSpanId = tp.SpanId } - } else { span.TraceId = emptyTraceId span.SpanId = emptySpanId @@ -299,3 +304,76 @@ func SpanToStringMap(span *tracepb.Span, rss *tracepb.ResourceSpans) map[string] "status_description": span.Status.GetMessage(), } } + +// TraceparentFromProtobufSpan builds a Traceparent struct from the provided span. +func TraceparentFromProtobufSpan(c Config, span *tracepb.Span) traceparent.Traceparent { + return traceparent.Traceparent{ + Version: 0, + TraceId: span.TraceId, + SpanId: span.SpanId, + Sampling: c.IsRecording(), + Initialized: true, + } +} + +// PropagateTraceparent saves the traceparent to file if necessary, then prints +// span info to the console according to command-line args. +func PropagateTraceparent(c Config, span *tracepb.Span, target io.Writer) { + var tp traceparent.Traceparent + if c.IsRecording() { + tp = TraceparentFromProtobufSpan(c, span) + } else { + // when in non-recording mode, and there is a TP available, propagate that + tp = LoadTraceparent(c, span) + } + + if c.TraceparentCarrierFile != "" { + err := tp.SaveToFile(c.TraceparentCarrierFile, c.TraceparentPrintExport) + c.SoftFailIfErr(err) + } + + if c.TraceparentPrint { + tp.Fprint(target, c.TraceparentPrintExport) + } +} + +// LoadTraceparent follows otel-cli's loading rules, start with envvar then file. +// If both are set, the file will override env. +// When in non-recording mode, the previous traceparent will be returned if it's +// available, otherwise, a zero-valued traceparent is returned. +func LoadTraceparent(c Config, span *tracepb.Span) traceparent.Traceparent { + tp := traceparent.Traceparent{ + Version: 0, + TraceId: emptyTraceId, + SpanId: emptySpanId, + Sampling: false, + Initialized: true, + } + + if !c.TraceparentIgnoreEnv { + var err error + tp, err = traceparent.LoadFromEnv() + if err != nil { + Diag.Error = err.Error() + } + } + + if c.TraceparentCarrierFile != "" { + fileTp, err := traceparent.LoadFromFile(c.TraceparentCarrierFile) + if err != nil { + Diag.Error = err.Error() + } else if fileTp.Initialized { + tp = fileTp + } + } + + if c.TraceparentRequired { + if tp.Initialized { + return tp + } else { + c.SoftFail("failed to find a valid traceparent carrier in either environment for file '%s' while it's required by --tp-required", c.TraceparentCarrierFile) + } + } + + return tp +} diff --git a/otlpclient/protobuf_span_test.go b/otlpclient/protobuf_span_test.go index b4d3fd0..ea2ab0b 100644 --- a/otlpclient/protobuf_span_test.go +++ b/otlpclient/protobuf_span_test.go @@ -2,6 +2,9 @@ package otlpclient import ( "bytes" + "encoding/hex" + "fmt" + "os" "strconv" "testing" @@ -254,3 +257,37 @@ func TestCliAttrsToOtel(t *testing.T) { } } } + +func TestPropagateTraceparent(t *testing.T) { + config := DefaultConfig(). + WithTraceparentCarrierFile(""). + WithTraceparentPrint(false). + WithTraceparentPrintExport(false) + + tp := "00-3433d5ae39bdfee397f44be5146867b3-8a5518f1e5c54d0a-01" + tid := "3433d5ae39bdfee397f44be5146867b3" + sid := "8a5518f1e5c54d0a" + os.Setenv("TRACEPARENT", tp) + + span := NewProtobufSpan() + span.TraceId, _ = hex.DecodeString(tid) + span.SpanId, _ = hex.DecodeString(sid) + + buf := new(bytes.Buffer) + PropagateTraceparent(config, span, buf) + if buf.Len() != 0 { + t.Errorf("nothing was supposed to be written but %d bytes were", buf.Len()) + } + + config.TraceparentPrint = true + config.TraceparentPrintExport = true + buf = new(bytes.Buffer) + PropagateTraceparent(config, span, buf) + if buf.Len() == 0 { + t.Error("expected more than zero bytes but got none") + } + expected := fmt.Sprintf("# trace id: %s\n# span id: %s\nexport TRACEPARENT=%s\n", tid, sid, tp) + if buf.String() != expected { + t.Errorf("got unexpected output, expected '%s', got '%s'", expected, buf.String()) + } +} diff --git a/otlpclient/traceparent_test.go b/otlpclient/traceparent_test.go deleted file mode 100644 index 8080b6d..0000000 --- a/otlpclient/traceparent_test.go +++ /dev/null @@ -1,126 +0,0 @@ -package otlpclient - -import ( - "bytes" - "encoding/hex" - "fmt" - "os" - "strings" - "testing" -) - -func TestLoadTraceparent(t *testing.T) { - config := DefaultConfig() - // make sure the environment variable isn't polluting test state - os.Unsetenv("TRACEPARENT") - - // trace id should not change, because there's no envvar and no file - tp := LoadTraceparent(config.WithTraceparentCarrierFile(os.DevNull)) - if tp.Initialized { - t.Error("traceparent detected where there should be none") - } - - // load from file only - testFileTp := "00-f61fc53f926e07a9c3893b1a722e1b65-7a2d6a804f3de137-01" - file, err := os.CreateTemp(t.TempDir(), "go-test-otel-cli") - if err != nil { - t.Fatalf("unable to create tempfile for testing: %s", err) - } - defer os.Remove(file.Name()) - // write in the full shell snippet format so that stripping gets tested - // in this pass too - file.WriteString("export TRACEPARENT=" + testFileTp) - file.Close() - - // actually do the test... - tp = LoadTraceparent(config.WithTraceparentCarrierFile(file.Name())) - if tp.Encode() != testFileTp { - t.Errorf("loadTraceparent with file failed, expected '%s', got '%s'", testFileTp, tp.Encode()) - } - - // load from environment only - testEnvTp := "00-b122b620341449410b9cd900c96d459d-aa21cda35388b694-01" - os.Setenv("TRACEPARENT", testEnvTp) - tp = LoadTraceparent(config.WithTraceparentCarrierFile(os.DevNull)) - if tp.Encode() != testEnvTp { - t.Errorf("loadTraceparent with envvar failed, expected '%s', got '%s'", testEnvTp, tp.Encode()) - } - - // now try with both file and envvar set by the previous tests - // the file is expected to win - tp = LoadTraceparent(config.WithTraceparentCarrierFile(file.Name())) - if tp.Encode() != testFileTp { - t.Errorf("loadTraceparent with file and envvar set to different values failed, expected '%s', got '%s'", testFileTp, tp.Encode()) - } -} - -func TestWriteTraceparentToFile(t *testing.T) { - config := DefaultConfig() - testTp := "00-ce1c6ae29edafc52eb6dd223da7d20b4-1c617f036253531c-01" - tp, _ := ParseTraceparent(testTp) - - // create a tempfile for messing with - file, err := os.CreateTemp(t.TempDir(), "go-test-otel-cli") - if err != nil { - t.Fatalf("unable to create tempfile for testing: %s", err) - } - file.Close() - defer os.Remove(file.Name()) // not strictly necessary - - tp.saveToFile(config.WithTraceparentCarrierFile(file.Name()), nil) - - // read the data back, it should just be the traceparent string - data, err := os.ReadFile(file.Name()) - if err != nil { - t.Fatalf("failed to read tempfile '%s': %s", file.Name(), err) - } - if len(data) == 0 { - t.Errorf("saveTraceparentToFile wrote %d bytes to the tempfile, expected %d", len(data), len(testTp)) - } - - // otel is non-recording in tests so the comments in the output will be zeroed - // while the traceparent should come through just fine at the end of file - if !strings.HasSuffix(strings.TrimSpace(string(data)), testTp) { - t.Errorf("invalid data in traceparent file, expected '%s', got '%s'", testTp, data) - } -} - -func TestPropagateOtelCliSpan(t *testing.T) { - // TODO: should this noop the tracing backend? - - config := DefaultConfig(). - WithTraceparentCarrierFile(""). - WithTraceparentPrint(false). - WithTraceparentPrintExport(false) - - tp := "00-3433d5ae39bdfee397f44be5146867b3-8a5518f1e5c54d0a-01" - tid := "3433d5ae39bdfee397f44be5146867b3" - sid := "8a5518f1e5c54d0a" - os.Setenv("TRACEPARENT", tp) - //tracer := otel.Tracer("testing/propagateOtelCliSpan") - //ctx, span := tracer.Start(context.Background(), "testing propagateOtelCliSpan") - span := NewProtobufSpan() - span.TraceId, _ = hex.DecodeString(tid) - span.SpanId, _ = hex.DecodeString(sid) - - buf := new(bytes.Buffer) - // mostly smoke testing this, will validate printSpanData output - // TODO: maybe validate the file write works, but that's tested elsewhere... - PropagateTraceparent(config, span, buf) - if buf.Len() != 0 { - t.Errorf("nothing was supposed to be written but %d bytes were", buf.Len()) - } - - config.TraceparentPrint = true - config.TraceparentPrintExport = true - buf = new(bytes.Buffer) - ptp, _ := ParseTraceparent(tp) - PrintSpanData(buf, ptp, span, config.TraceparentPrintExport) - if buf.Len() == 0 { - t.Error("expected more than zero bytes but got none") - } - expected := fmt.Sprintf("# trace id: %s\n# span id: %s\nexport TRACEPARENT=%s\n", tid, sid, tp) - if buf.String() != expected { - t.Errorf("got unexpected output, expected '%s', got '%s'", expected, buf.String()) - } -} diff --git a/otlpclient/traceparent.go b/w3c/traceparent/traceparent.go similarity index 50% rename from otlpclient/traceparent.go rename to w3c/traceparent/traceparent.go index baa84d7..e70c610 100644 --- a/otlpclient/traceparent.go +++ b/w3c/traceparent/traceparent.go @@ -1,8 +1,9 @@ -package otlpclient +// Package traceparent contains a lightweight implementation of W3C +// traceparent parsing, loading from files and environment, and the reverse. +package traceparent import ( "bufio" - "bytes" "encoding/hex" "fmt" "io" @@ -10,8 +11,6 @@ import ( "regexp" "strconv" "strings" - - tracepb "go.opentelemetry.io/proto/otlp/trace/v1" ) var traceparentRe *regexp.Regexp @@ -36,76 +35,55 @@ type Traceparent struct { // Encode returns the traceparent as a W3C formatted string. func (tp Traceparent) Encode() string { var sampling int + var traceId, spanId string if tp.Sampling { sampling = 1 } - traceId := tp.TraceIdString() - spanId := tp.SpanIdString() + + if len(tp.TraceId) == 0 { + traceId = hex.EncodeToString(emptyTraceId) + } else { + traceId = tp.TraceIdString() + } + + if len(tp.SpanId) == 0 { + spanId = hex.EncodeToString(emptySpanId) + } else { + spanId = tp.SpanIdString() + } + return fmt.Sprintf("%02d-%s-%s-%02d", tp.Version, traceId, spanId, sampling) } // TraceIdString returns the trace id in string form. func (tp Traceparent) TraceIdString() string { - return hex.EncodeToString(tp.TraceId) + if len(tp.TraceId) == 0 { + return hex.EncodeToString(emptyTraceId) + } else { + return hex.EncodeToString(tp.TraceId) + } } // SpanIdString returns the span id in string form. func (tp Traceparent) SpanIdString() string { - return hex.EncodeToString(tp.SpanId) -} - -// TraceparentFromSpan builds a Traceparent struct from the provided span. -func TraceparentFromSpan(span *tracepb.Span) Traceparent { - return Traceparent{ - Version: 0, - TraceId: span.TraceId, - SpanId: span.SpanId, - Sampling: true, // TODO: fix this: hax - Initialized: true, - } -} - -// LoadTraceparent checks the environment first for TRACEPARENT then if filename -// isn't empty, it will read that file and look for a bare traceparent in that -// file. -func LoadTraceparent(config Config) Traceparent { - tp := loadTraceparentFromEnv(config) - if config.TraceparentCarrierFile != "" { - fileTp, err := loadTraceparentFromFile(config.TraceparentCarrierFile, config.TraceparentRequired) - config.SoftFailIfErr(err) - if fileTp.Initialized { - tp = fileTp - } - } - - if config.TraceparentRequired { - if tp.Initialized { - // return from here if everything looks ok, otherwise fall through to the log.Fatal - if !bytes.Equal(tp.TraceId, emptyTraceId) && !bytes.Equal(tp.SpanId, emptySpanId) { - return tp - } - } - config.SoftFail("failed to find a valid traceparent carrier in either environment for file '%s' while it's required by --tp-required", config.TraceparentCarrierFile) + if len(tp.SpanId) == 0 { + return hex.EncodeToString(emptySpanId) + } else { + return hex.EncodeToString(tp.SpanId) } - return tp } -// loadTraceparentFromFile reads a traceparent from filename and returns a +// LoadFromFile reads a traceparent from filename and returns a // context with the traceparent set. The format for the file as written is // just a bare traceparent string. Whitespace, "export " and "TRACEPARENT=" are // stripped automatically so the file can also be a valid shell snippet. -func loadTraceparentFromFile(filename string, tpRequired bool) (Traceparent, error) { +func LoadFromFile(filename string) (Traceparent, error) { file, err := os.Open(filename) if err != nil { errOut := fmt.Errorf("could not open file '%s' for read: %s", filename, err) - Diag.SetError(errOut) // only fatal when the tp carrier file is required explicitly, otherwise // just silently return the unmodified context - if tpRequired { - return Traceparent{}, errOut - } else { - return Traceparent{}, nil // mask the error - } + return Traceparent{}, errOut } defer file.Close() @@ -136,45 +114,25 @@ func loadTraceparentFromFile(filename string, tpRequired bool) (Traceparent, err return Traceparent{}, fmt.Errorf("file '%s' was read but does not contain a valid traceparent", filename) } - return ParseTraceparent(tp) + return Parse(tp) } -// saveToFile takes a context and filename and writes the tp from +// SaveToFile takes a context and filename and writes the tp from // that context into the specified file. -func (tp Traceparent) saveToFile(config Config, span *tracepb.Span) { - if config.TraceparentCarrierFile == "" { - return - } - - file, err := os.OpenFile(config.TraceparentCarrierFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) +func (tp Traceparent) SaveToFile(carrierFile string, export bool) error { + file, err := os.OpenFile(carrierFile, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) if err != nil { - config.SoftFail("failure opening file '%s' for write: %s", config.TraceparentCarrierFile, err) + return fmt.Errorf("failure opening file '%s' for write: %w", carrierFile, err) } defer file.Close() - PrintSpanData(file, tp, span, config.TraceparentPrintExport) -} - -// PropagateTraceparent saves the traceparent to file if necessary, then prints -// span info to the console according to command-line args. -func PropagateTraceparent(config Config, span *tracepb.Span, target io.Writer) { - var tp Traceparent - if config.IsRecording() { - tp = TraceparentFromSpan(span) - } else { - // when in non-recording mode, and there is a TP available, propagate that - tp = LoadTraceparent(config) - } - tp.saveToFile(config, span) - - if config.TraceparentPrint { - PrintSpanData(target, tp, span, config.TraceparentPrintExport) - } + return tp.Fprint(file, export) } -// PrintSpanData takes the provided strings and prints them in a consitent format, -// depending on which command line arguments were set. -func PrintSpanData(target io.Writer, tp Traceparent, span *tracepb.Span, export bool) { +// Fprint formats a traceparent into otel-cli's shell-compatible text format. +// If the second/export param is true, the statement will be prepended with "export " +// so it can be easily sourced in a shell script. +func (tp Traceparent) Fprint(target io.Writer, export bool) error { // --tp-export will print "export TRACEPARENT" so it's // one less step to print to a file & source, or eval var exported string @@ -182,42 +140,25 @@ func PrintSpanData(target io.Writer, tp Traceparent, span *tracepb.Span, export exported = "export " } - var traceId, spanId string - if span != nil { - // when in non-recording mode, the printed trace/span id should be all zeroes - // and the input TP passes through - // NOTE: this is preserved behavior from before protobuf spans, maybe this isn't - // worth the trouble? - traceId = hex.EncodeToString(span.TraceId) - spanId = hex.EncodeToString(span.SpanId) - } else { - // in recording mode these will match the TP - traceId = tp.TraceIdString() - spanId = tp.SpanIdString() - } - fmt.Fprintf(target, "# trace id: %s\n# span id: %s\n%sTRACEPARENT=%s\n", traceId, spanId, exported, tp.Encode()) + traceId := tp.TraceIdString() + spanId := tp.SpanIdString() + _, err := fmt.Fprintf(target, "# trace id: %s\n# span id: %s\n%sTRACEPARENT=%s\n", traceId, spanId, exported, tp.Encode()) + return err } -// loadTraceparentFromEnv loads the traceparent from the environment variable +// LoadFromEnv loads the traceparent from the environment variable // TRACEPARENT and sets it in the returned Go context. -func loadTraceparentFromEnv(config Config) Traceparent { - // don't load the envvar when --tp-ignore-env is set - if config.TraceparentIgnoreEnv { - return Traceparent{} - } - +func LoadFromEnv() (Traceparent, error) { tp := os.Getenv("TRACEPARENT") if tp == "" { - return Traceparent{} + return Traceparent{}, nil } - tps, err := ParseTraceparent(tp) - config.SoftFailIfErr(err) - return tps + return Parse(tp) } -// ParseTraceparent parses a string traceparent and returns the struct. -func ParseTraceparent(tp string) (Traceparent, error) { +// Parse parses a string traceparent and returns the struct. +func Parse(tp string) (Traceparent, error) { var err error out := Traceparent{} diff --git a/w3c/traceparent/traceparent_test.go b/w3c/traceparent/traceparent_test.go new file mode 100644 index 0000000..46bfd04 --- /dev/null +++ b/w3c/traceparent/traceparent_test.go @@ -0,0 +1,154 @@ +package traceparent + +import ( + "bytes" + "os" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestFprint(t *testing.T) { + for _, tc := range []struct { + tp Traceparent + export bool + want string + }{ + // unconfigured, all zeroes + { + tp: Traceparent{ + Version: 0, + TraceId: []byte{}, + SpanId: []byte{}, + Sampling: false, + Initialized: false, + }, + export: false, + want: "# trace id: 00000000000000000000000000000000\n" + + "# span id: 0000000000000000\n" + + "TRACEPARENT=00-00000000000000000000000000000000-0000000000000000-00\n", + }, + // fully loaded, print all the things + { + tp: Traceparent{ + Version: 0, + TraceId: []byte{0xfe, 0xdc, 0xcb, 0xa9, 0x87, 0x65, 0x43, 0x21, 0xfe, 0xdc, 0xcb, 0xa9, 0x87, 0x65, 0x43, 0x21}, + SpanId: []byte{0xde, 0xea, 0xd6, 0xbb, 0xaa, 0xbb, 0xcc, 0xdd}, + Sampling: true, + Initialized: true, + }, + export: true, + want: "# trace id: fedccba987654321fedccba987654321\n" + + "# span id: deead6bbaabbccdd\n" + + "export TRACEPARENT=00-fedccba987654321fedccba987654321-deead6bbaabbccdd-01\n", + }, + // have a traceparent, but sampling is off, the tp should propagate as-is + { + tp: Traceparent{ + Version: 0, + TraceId: []byte{0xfe, 0xdc, 0xcb, 0xa9, 0x87, 0x65, 0x43, 0x21, 0xfe, 0xdc, 0xcb, 0xa9, 0x87, 0x65, 0x43, 0x21}, + SpanId: []byte{0xde, 0xea, 0xd6, 0xbb, 0xaa, 0xbb, 0xcc, 0xdd}, + Sampling: false, + Initialized: true, + }, + export: false, + want: "# trace id: fedccba987654321fedccba987654321\n" + + "# span id: deead6bbaabbccdd\n" + + // the traceparent provided should get printed + "TRACEPARENT=00-fedccba987654321fedccba987654321-deead6bbaabbccdd-00\n", + }, + } { + buf := bytes.NewBuffer([]byte{}) + err := tc.tp.Fprint(buf, tc.export) + if err != nil { + t.Errorf("got an unexpected error: %s", err) + } + + if diff := cmp.Diff(tc.want, buf.String()); diff != "" { + t.Errorf("printed tp didn't match expected: (-want +got):\n%s", diff) + } + } +} + +func TestLoadTraceparent(t *testing.T) { + // make sure the environment variable isn't polluting test state + os.Unsetenv("TRACEPARENT") + + // trace id should not change, because there's no envvar and no file + tp, err := LoadFromFile(os.DevNull) + if err != nil { + t.Error("LoadFromFile returned an unexpected error: %w", err) + } + if tp.Initialized { + t.Error("traceparent detected where there should be none") + } + + // load from file only + testFileTp := "00-f61fc53f926e07a9c3893b1a722e1b65-7a2d6a804f3de137-01" + file, err := os.CreateTemp(t.TempDir(), "go-test-otel-cli") + if err != nil { + t.Fatalf("unable to create tempfile for testing: %s", err) + } + defer os.Remove(file.Name()) + // write in the full shell snippet format so that stripping gets tested + // in this pass too + file.WriteString("export TRACEPARENT=" + testFileTp) + file.Close() + + // actually do the test... + tp, err = LoadFromFile(file.Name()) + if err != nil { + t.Error("LoadFromFile returned an unexpected error: %w", err) + } + if tp.Encode() != testFileTp { + t.Errorf("LoadFromFile failed, expected '%s', got '%s'", testFileTp, tp.Encode()) + } + + // load from environment + testEnvTp := "00-b122b620341449410b9cd900c96d459d-aa21cda35388b694-01" + os.Setenv("TRACEPARENT", testEnvTp) + tp, err = LoadFromEnv() + if err != nil { + t.Error("LoadFromEnv() returned an unexpected error: %w", err) + } + if tp.Encode() != testEnvTp { + t.Errorf("LoadFromEnv() with envvar failed, expected '%s', got '%s'", testEnvTp, tp.Encode()) + } +} + +func TestWriteTraceparentToFile(t *testing.T) { + testTp := "00-ce1c6ae29edafc52eb6dd223da7d20b4-1c617f036253531c-01" + tp, err := Parse(testTp) + if err != nil { + t.Errorf("failed while parsing test TP %q: %s", testTp, err) + } + + // create a tempfile for messing with + file, err := os.CreateTemp(t.TempDir(), "go-test-otel-cli") + if err != nil { + t.Fatalf("unable to create tempfile for testing: %s", err) + } + file.Close() + defer os.Remove(file.Name()) // not strictly necessary + + err = tp.SaveToFile(file.Name(), false) + if err != nil { + t.Error("SaveToFile returned an unexpected error: %w", err) + } + + // read the data back, it should just be the traceparent string + data, err := os.ReadFile(file.Name()) + if err != nil { + t.Fatalf("failed to read tempfile '%s': %s", file.Name(), err) + } + if len(data) == 0 { + t.Errorf("saveTraceparentToFile wrote %d bytes to the tempfile, expected %d", len(data), len(testTp)) + } + + // otel is non-recording in tests so the comments in the output will be zeroed + // while the traceparent should come through just fine at the end of file + if !strings.HasSuffix(strings.TrimSpace(string(data)), testTp) { + t.Errorf("invalid data in traceparent file, expected '%s', got '%s'", testTp, data) + } +}