Skip to content

Commit

Permalink
separate w3c traceparent to its own package (equinix-labs#225)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tobert authored Jun 23, 2023
1 parent bdbfcbb commit 819cae9
Show file tree
Hide file tree
Showing 12 changed files with 338 additions and 252 deletions.
12 changes: 6 additions & 6 deletions data_for_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
Expand All @@ -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",
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions otelcli/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
Expand Down
3 changes: 2 additions & 1 deletion otelcli/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion otelcli/span_background_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions otelcli/span_end.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
}
5 changes: 3 additions & 2 deletions otelcli/span_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
}
3 changes: 1 addition & 2 deletions otelcli/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,14 @@ func doStatus(cmd *cobra.Command, args []string) {
config.SoftLog("%s", err)
}
}
tp := otlpclient.TraceparentFromSpan(span)

outData := StatusOutput{
Config: config,
Env: env,
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,
}
Expand Down
82 changes: 80 additions & 2 deletions otlpclient/protobuf_span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
37 changes: 37 additions & 0 deletions otlpclient/protobuf_span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package otlpclient

import (
"bytes"
"encoding/hex"
"fmt"
"os"
"strconv"
"testing"

Expand Down Expand Up @@ -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())
}
}
Loading

0 comments on commit 819cae9

Please sign in to comment.