From 6962f14d443901afc7aa5bd1b3aa43325841449f Mon Sep 17 00:00:00 2001 From: Nghia Tran Date: Tue, 18 Jun 2024 19:32:50 -0700 Subject: [PATCH] Use `traceparent` instead of `X-Cloud-Trace-Context` (#16) According to https://cloud.google.com/trace/docs/trace-context#http-requests, `X-Cloud-Trace-Context` is a legacy header that precedes the W3C standard `traceparent` header. Google Cloud services that support trace context propagation typically support both the `traceparent` and the legacy `X-Cloud-Trace-Context` header. Switching to use this so that we don't have to use the legacy header `X-Cloud-Trace-Context` when restoring trace context header for pubsub eventing, but only using `traceparent` every where. --- gcp/trace.go | 16 ++++++++++++---- gcp/trace_test.go | 25 +++++++++++++------------ 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/gcp/trace.go b/gcp/trace.go index da606f4..ff0ad6b 100644 --- a/gcp/trace.go +++ b/gcp/trace.go @@ -96,10 +96,10 @@ func WithCloudTraceContext(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if projectID != "" { var trace string - traceHeader := r.Header.Get("X-Cloud-Trace-Context") - traceParts := strings.Split(traceHeader, "/") - if len(traceParts) > 0 && len(traceParts[0]) > 0 { - trace = fmt.Sprintf("projects/%s/traces/%s", projectID, traceParts[0]) + traceHeader := r.Header.Get("traceparent") + traceID := parseTraceFromW3CHeader(traceHeader) + if traceID != "" { + trace = fmt.Sprintf("projects/%s/traces/%s", projectID, traceID) } r = r.WithContext(context.WithValue(r.Context(), "trace", trace)) } @@ -114,3 +114,11 @@ func traceFromContext(ctx context.Context) string { } return trace.(string) } + +func parseTraceFromW3CHeader(traceparent string) string { + traceParts := strings.Split(traceparent, "-") + if len(traceParts) > 1 { + return traceParts[1] + } + return "" +} diff --git a/gcp/trace_test.go b/gcp/trace_test.go index cdfdeb8..109357a 100644 --- a/gcp/trace_test.go +++ b/gcp/trace_test.go @@ -4,7 +4,6 @@ import ( "log/slog" "net/http" "net/http/httptest" - "strings" "testing" "github.com/chainguard-dev/clog" @@ -22,10 +21,10 @@ func TestTrace(t *testing.T) { for _, c := range []struct { name string env string - wantTrace bool + wantTrace string }{ - {"no env set", "", false}, - {"env set", "my-project", true}, + {"no env set", "", ""}, + {"env set", "my-project", "projects/my-project/traces/traceid"}, } { t.Run(c.name, func(t *testing.T) { t.Setenv("GOOGLE_CLOUD_PROJECT", c.env) @@ -39,16 +38,18 @@ func TestTrace(t *testing.T) { // TODO: This doesn't propagate the trace context to the logger. //clog.FromContext(ctx).Info("hello world") - if r.Header.Get("X-Cloud-Trace-Context") == "" { + if r.Header.Get("traceparent") == "" { t.Error("got empty trace context header, want non-empty") } - if found := ctx.Value("trace") != nil; found != c.wantTrace { - t.Fatalf("got trace context %t, want %t", found, c.wantTrace) - if c.wantTrace { - if trace := ctx.Value("trace"); !strings.Contains(trace.(string), "/"+c.env+"/") { - t.Errorf("got trace context %q, want %q", trace, c.env) - } + traceCtx := ctx.Value("trace") + if traceCtx == nil { + if c.wantTrace != "" { + t.Fatalf("want %s, not found", c.wantTrace) + } + } else { + if traceCtx != c.wantTrace { + t.Fatalf("got %s, want %s", traceCtx, c.wantTrace) } } })) @@ -60,7 +61,7 @@ func TestTrace(t *testing.T) { if err != nil { t.Fatal(err) } - req.Header.Set("X-Cloud-Trace-Context", "trace/id/yay") + req.Header.Set("traceparent", "00-traceid-spanid-01") if _, err := http.DefaultClient.Do(req); err != nil { t.Fatal(err) }