From ed61add8280373471b92e80c709f4ef8884c448e Mon Sep 17 00:00:00 2001 From: Sam Gunaratne Date: Mon, 18 Nov 2024 13:57:23 -0700 Subject: [PATCH 1/2] Add B3 trace IDs to cf cli commands --- api/cloudcontroller/wrapper/trace_request.go | 31 +++++ .../wrapper/trace_request_test.go | 67 +++++++++ api/router/wrapper/trace_request.go | 31 +++++ api/router/wrapper/trace_request_test.go | 67 +++++++++ api/shared/trace_headers.go | 35 +++++ api/shared/trace_headers_test.go | 42 ++++++ api/uaa/wrapper/trace_request.go | 33 +++++ api/uaa/wrapper/trace_request_test.go | 67 +++++++++ command/commandfakes/fake_config.go | 130 ++++++++++++++++++ command/config.go | 2 + command/v7/shared/new_clients.go | 2 + util/configv3/env.go | 18 +++ util/configv3/load_config.go | 2 + util/random/hex.go | 15 ++ 14 files changed, 542 insertions(+) create mode 100644 api/cloudcontroller/wrapper/trace_request.go create mode 100644 api/cloudcontroller/wrapper/trace_request_test.go create mode 100644 api/router/wrapper/trace_request.go create mode 100644 api/router/wrapper/trace_request_test.go create mode 100644 api/shared/trace_headers.go create mode 100644 api/shared/trace_headers_test.go create mode 100644 api/uaa/wrapper/trace_request.go create mode 100644 api/uaa/wrapper/trace_request_test.go create mode 100644 util/random/hex.go diff --git a/api/cloudcontroller/wrapper/trace_request.go b/api/cloudcontroller/wrapper/trace_request.go new file mode 100644 index 00000000000..87d3298dae2 --- /dev/null +++ b/api/cloudcontroller/wrapper/trace_request.go @@ -0,0 +1,31 @@ +package wrapper + +import ( + "code.cloudfoundry.org/cli/api/cloudcontroller" + "code.cloudfoundry.org/cli/api/shared" +) + +// CCTraceHeaderRequest is a wrapper that adds b3 trace headers to requests. +type CCTraceHeaderRequest struct { + headers *shared.TraceHeaders + connection cloudcontroller.Connection +} + +// NewCCTraceHeaderRequest returns a pointer to a CCTraceHeaderRequest wrapper. +func NewCCTraceHeaderRequest(trace, span string) *CCTraceHeaderRequest { + return &CCTraceHeaderRequest{ + headers: shared.NewTraceHeaders(trace, span), + } +} + +// Add tracing headers +func (t *CCTraceHeaderRequest) Make(request *cloudcontroller.Request, passedResponse *cloudcontroller.Response) error { + t.headers.SetHeaders(request.Request) + return t.connection.Make(request, passedResponse) +} + +// Wrap sets the connection in the CCTraceHeaderRequest and returns itself. +func (t *CCTraceHeaderRequest) Wrap(innerconnection cloudcontroller.Connection) cloudcontroller.Connection { + t.connection = innerconnection + return t +} diff --git a/api/cloudcontroller/wrapper/trace_request_test.go b/api/cloudcontroller/wrapper/trace_request_test.go new file mode 100644 index 00000000000..b4719c7b0fd --- /dev/null +++ b/api/cloudcontroller/wrapper/trace_request_test.go @@ -0,0 +1,67 @@ +package wrapper_test + +import ( + "bytes" + "net/http" + + "code.cloudfoundry.org/cli/api/cloudcontroller" + "code.cloudfoundry.org/cli/api/cloudcontroller/cloudcontrollerfakes" + . "code.cloudfoundry.org/cli/api/cloudcontroller/wrapper" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("CCTraceHeaderRequest", func() { + var ( + fakeConnection *cloudcontrollerfakes.FakeConnection + + wrapper cloudcontroller.Connection + + request *cloudcontroller.Request + response *cloudcontroller.Response + makeErr error + + traceHeader string + spanHeader string + ) + + BeforeEach(func() { + fakeConnection = new(cloudcontrollerfakes.FakeConnection) + + traceHeader = "trace-id" + spanHeader = "span-id" + + wrapper = NewCCTraceHeaderRequest(traceHeader, spanHeader).Wrap(fakeConnection) + + body := bytes.NewReader([]byte("foo")) + + req, err := http.NewRequest(http.MethodGet, "https://foo.bar.com/banana", body) + Expect(err).NotTo(HaveOccurred()) + + response = &cloudcontroller.Response{ + RawResponse: []byte("some-response-body"), + HTTPResponse: &http.Response{}, + } + request = cloudcontroller.NewRequest(req, body) + }) + + JustBeforeEach(func() { + makeErr = wrapper.Make(request, response) + }) + + Describe("Make", func() { + It("Adds the request headers", func() { + Expect(makeErr).NotTo(HaveOccurred()) + Expect(request.Header.Get("X-B3-TraceId")).To(Equal(traceHeader)) + Expect(request.Header.Get("X-B3-SpanId")).To(Equal(spanHeader)) + }) + + It("Calls the inner connection", func() { + Expect(fakeConnection.MakeCallCount()).To(Equal(1)) + req, resp := fakeConnection.MakeArgsForCall(0) + Expect(req).To(Equal(request)) + Expect(resp).To(Equal(response)) + }) + }) +}) diff --git a/api/router/wrapper/trace_request.go b/api/router/wrapper/trace_request.go new file mode 100644 index 00000000000..724cb6ee3ad --- /dev/null +++ b/api/router/wrapper/trace_request.go @@ -0,0 +1,31 @@ +package wrapper + +import ( + "code.cloudfoundry.org/cli/api/router" + "code.cloudfoundry.org/cli/api/shared" +) + +// RoutingTraceHeaderRequest is a wrapper that adds b3 trace headers to requests. +type RoutingTraceHeaderRequest struct { + headers *shared.TraceHeaders + connection router.Connection +} + +// NewRoutingTraceHeaderRequest returns a pointer to a RoutingTraceHeaderRequest wrapper. +func NewRoutingTraceHeaderRequest(trace, span string) *RoutingTraceHeaderRequest { + return &RoutingTraceHeaderRequest{ + headers: shared.NewTraceHeaders(trace, span), + } +} + +// Add tracing headers +func (t *RoutingTraceHeaderRequest) Make(request *router.Request, passedResponse *router.Response) error { + t.headers.SetHeaders(request.Request) + return t.connection.Make(request, passedResponse) +} + +// Wrap sets the connection in the RoutingTraceHeaderRequest and returns itself. +func (t *RoutingTraceHeaderRequest) Wrap(innerconnection router.Connection) router.Connection { + t.connection = innerconnection + return t +} diff --git a/api/router/wrapper/trace_request_test.go b/api/router/wrapper/trace_request_test.go new file mode 100644 index 00000000000..fd4a1bd36e8 --- /dev/null +++ b/api/router/wrapper/trace_request_test.go @@ -0,0 +1,67 @@ +package wrapper_test + +import ( + "bytes" + "net/http" + + "code.cloudfoundry.org/cli/api/router" + "code.cloudfoundry.org/cli/api/router/routerfakes" + . "code.cloudfoundry.org/cli/api/router/wrapper" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("CCTraceHeaderRequest", func() { + var ( + fakeConnection *routerfakes.FakeConnection + + wrapper router.Connection + + request *router.Request + response *router.Response + makeErr error + + traceHeader string + spanHeader string + ) + + BeforeEach(func() { + fakeConnection = new(routerfakes.FakeConnection) + + traceHeader = "trace-id" + spanHeader = "span-id" + + wrapper = NewRoutingTraceHeaderRequest(traceHeader, spanHeader).Wrap(fakeConnection) + + body := bytes.NewReader([]byte("foo")) + + req, err := http.NewRequest(http.MethodGet, "https://foo.bar.com/banana", body) + Expect(err).NotTo(HaveOccurred()) + + response = &router.Response{ + RawResponse: []byte("some-response-body"), + HTTPResponse: &http.Response{}, + } + request = router.NewRequest(req, body) + }) + + JustBeforeEach(func() { + makeErr = wrapper.Make(request, response) + }) + + Describe("Make", func() { + It("Adds the request headers", func() { + Expect(makeErr).NotTo(HaveOccurred()) + Expect(request.Header.Get("X-B3-TraceId")).To(Equal(traceHeader)) + Expect(request.Header.Get("X-B3-SpanId")).To(Equal(spanHeader)) + }) + + It("Calls the inner connection", func() { + Expect(fakeConnection.MakeCallCount()).To(Equal(1)) + req, resp := fakeConnection.MakeArgsForCall(0) + Expect(req).To(Equal(request)) + Expect(resp).To(Equal(response)) + }) + }) +}) diff --git a/api/shared/trace_headers.go b/api/shared/trace_headers.go new file mode 100644 index 00000000000..1aec94d62f2 --- /dev/null +++ b/api/shared/trace_headers.go @@ -0,0 +1,35 @@ +package shared + +import ( + "net/http" +) + +const ( + B3TraceIDHeader = "X-B3-TraceId" + B3SpanIDHeader = "X-B3-SpanId" +) + +// TraceHeaders sets b3 trace headers to requests. +type TraceHeaders struct { + b3trace string + b3span string +} + +// NewTraceHeaders returns a pointer to a TraceHeaderRequest. +func NewTraceHeaders(trace, span string) *TraceHeaders { + return &TraceHeaders{ + b3trace: trace, + b3span: span, + } +} + +// Add tracing headers if they are not already set. +func (t *TraceHeaders) SetHeaders(request *http.Request) { + // only override the trace headers if they are not already set (e.g. already explicitly set by cf curl) + if request.Header.Get(B3TraceIDHeader) == "" { + request.Header.Add(B3TraceIDHeader, t.b3trace) + } + if request.Header.Get(B3SpanIDHeader) == "" { + request.Header.Add(B3SpanIDHeader, t.b3span) + } +} diff --git a/api/shared/trace_headers_test.go b/api/shared/trace_headers_test.go new file mode 100644 index 00000000000..c5efe888b01 --- /dev/null +++ b/api/shared/trace_headers_test.go @@ -0,0 +1,42 @@ +package shared_test + +import ( + "net/http" + + . "code.cloudfoundry.org/cli/api/shared" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("B3 Trace Headers", func() { + Describe("SetHeaders", func() { + Context("when there are already headers set", func() { + It("does not add the headers", func() { + traceHeaders := NewTraceHeaders("new_trace_id", "new_span_id") + request := &http.Request{ + Header: http.Header{}, + } + request.Header.Set("X-B3-TraceId", "old_trace_id") + request.Header.Set("X-B3-SpanId", "old_span_id") + traceHeaders.SetHeaders(request) + + Expect(request.Header.Get("X-B3-TraceId")).To(Equal("old_trace_id")) + Expect(request.Header.Get("X-B3-SpanId")).To(Equal("old_span_id")) + }) + }) + + Context("when there are no headers set", func() { + It("adds the headers", func() { + traceHeaders := NewTraceHeaders("new_trace_id", "new_span_id") + request := &http.Request{ + Header: http.Header{}, + } + traceHeaders.SetHeaders(request) + + Expect(request.Header.Get("X-B3-TraceId")).To(Equal("new_trace_id")) + Expect(request.Header.Get("X-B3-SpanId")).To(Equal("new_span_id")) + }) + }) + }) +}) diff --git a/api/uaa/wrapper/trace_request.go b/api/uaa/wrapper/trace_request.go new file mode 100644 index 00000000000..635812ed826 --- /dev/null +++ b/api/uaa/wrapper/trace_request.go @@ -0,0 +1,33 @@ +package wrapper + +import ( + "net/http" + + "code.cloudfoundry.org/cli/api/shared" + "code.cloudfoundry.org/cli/api/uaa" +) + +// UAATraceHeaderRequest is a wrapper that adds b3 trace headers to requests. +type UAATraceHeaderRequest struct { + headers *shared.TraceHeaders + connection uaa.Connection +} + +// NewUAATraceHeaderRequest returns a pointer to a UAATraceHeaderRequest wrapper. +func NewUAATraceHeaderRequest(trace, span string) *UAATraceHeaderRequest { + return &UAATraceHeaderRequest{ + headers: shared.NewTraceHeaders(trace, span), + } +} + +// Add tracing headers +func (t *UAATraceHeaderRequest) Make(request *http.Request, passedResponse *uaa.Response) error { + t.headers.SetHeaders(request) + return t.connection.Make(request, passedResponse) +} + +// Wrap sets the connection in the UAATraceHeaderRequest and returns itself. +func (t *UAATraceHeaderRequest) Wrap(innerconnection uaa.Connection) uaa.Connection { + t.connection = innerconnection + return t +} diff --git a/api/uaa/wrapper/trace_request_test.go b/api/uaa/wrapper/trace_request_test.go new file mode 100644 index 00000000000..9f9aae39101 --- /dev/null +++ b/api/uaa/wrapper/trace_request_test.go @@ -0,0 +1,67 @@ +package wrapper_test + +import ( + "bytes" + "net/http" + + "code.cloudfoundry.org/cli/api/uaa" + "code.cloudfoundry.org/cli/api/uaa/uaafakes" + . "code.cloudfoundry.org/cli/api/uaa/wrapper" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("CCTraceHeaderRequest", func() { + var ( + fakeConnection *uaafakes.FakeConnection + + wrapper uaa.Connection + + request *http.Request + response *uaa.Response + makeErr error + + traceHeader string + spanHeader string + ) + + BeforeEach(func() { + fakeConnection = new(uaafakes.FakeConnection) + + traceHeader = "trace-id" + spanHeader = "span-id" + + wrapper = NewUAATraceHeaderRequest(traceHeader, spanHeader).Wrap(fakeConnection) + + body := bytes.NewReader([]byte("foo")) + + var err error + request, err = http.NewRequest(http.MethodGet, "https://foo.bar.com/banana", body) + Expect(err).NotTo(HaveOccurred()) + + response = &uaa.Response{ + RawResponse: []byte("some-response-body"), + HTTPResponse: &http.Response{}, + } + }) + + JustBeforeEach(func() { + makeErr = wrapper.Make(request, response) + }) + + Describe("Make", func() { + It("Adds the request headers", func() { + Expect(makeErr).NotTo(HaveOccurred()) + Expect(request.Header.Get("X-B3-TraceId")).To(Equal(traceHeader)) + Expect(request.Header.Get("X-B3-SpanId")).To(Equal(spanHeader)) + }) + + It("Calls the inner connection", func() { + Expect(fakeConnection.MakeCallCount()).To(Equal(1)) + req, resp := fakeConnection.MakeArgsForCall(0) + Expect(req).To(Equal(request)) + Expect(resp).To(Equal(response)) + }) + }) +}) diff --git a/command/commandfakes/fake_config.go b/command/commandfakes/fake_config.go index 2b7f1fe4014..1aa3e8e60a3 100644 --- a/command/commandfakes/fake_config.go +++ b/command/commandfakes/fake_config.go @@ -51,6 +51,26 @@ type FakeConfig struct { authorizationEndpointReturnsOnCall map[int]struct { result1 string } + B3SpanIDStub func() string + b3SpanIDMutex sync.RWMutex + b3SpanIDArgsForCall []struct { + } + b3SpanIDReturns struct { + result1 string + } + b3SpanIDReturnsOnCall map[int]struct { + result1 string + } + B3TraceIDStub func() string + b3TraceIDMutex sync.RWMutex + b3TraceIDArgsForCall []struct { + } + b3TraceIDReturns struct { + result1 string + } + b3TraceIDReturnsOnCall map[int]struct { + result1 string + } BinaryNameStub func() string binaryNameMutex sync.RWMutex binaryNameArgsForCall []struct { @@ -867,6 +887,112 @@ func (fake *FakeConfig) AuthorizationEndpointReturnsOnCall(i int, result1 string }{result1} } +func (fake *FakeConfig) B3SpanID() string { + fake.b3SpanIDMutex.Lock() + ret, specificReturn := fake.b3SpanIDReturnsOnCall[len(fake.b3SpanIDArgsForCall)] + fake.b3SpanIDArgsForCall = append(fake.b3SpanIDArgsForCall, struct { + }{}) + stub := fake.B3SpanIDStub + fakeReturns := fake.b3SpanIDReturns + fake.recordInvocation("B3SpanID", []interface{}{}) + fake.b3SpanIDMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeConfig) B3SpanIDCallCount() int { + fake.b3SpanIDMutex.RLock() + defer fake.b3SpanIDMutex.RUnlock() + return len(fake.b3SpanIDArgsForCall) +} + +func (fake *FakeConfig) B3SpanIDCalls(stub func() string) { + fake.b3SpanIDMutex.Lock() + defer fake.b3SpanIDMutex.Unlock() + fake.B3SpanIDStub = stub +} + +func (fake *FakeConfig) B3SpanIDReturns(result1 string) { + fake.b3SpanIDMutex.Lock() + defer fake.b3SpanIDMutex.Unlock() + fake.B3SpanIDStub = nil + fake.b3SpanIDReturns = struct { + result1 string + }{result1} +} + +func (fake *FakeConfig) B3SpanIDReturnsOnCall(i int, result1 string) { + fake.b3SpanIDMutex.Lock() + defer fake.b3SpanIDMutex.Unlock() + fake.B3SpanIDStub = nil + if fake.b3SpanIDReturnsOnCall == nil { + fake.b3SpanIDReturnsOnCall = make(map[int]struct { + result1 string + }) + } + fake.b3SpanIDReturnsOnCall[i] = struct { + result1 string + }{result1} +} + +func (fake *FakeConfig) B3TraceID() string { + fake.b3TraceIDMutex.Lock() + ret, specificReturn := fake.b3TraceIDReturnsOnCall[len(fake.b3TraceIDArgsForCall)] + fake.b3TraceIDArgsForCall = append(fake.b3TraceIDArgsForCall, struct { + }{}) + stub := fake.B3TraceIDStub + fakeReturns := fake.b3TraceIDReturns + fake.recordInvocation("B3TraceID", []interface{}{}) + fake.b3TraceIDMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeConfig) B3TraceIDCallCount() int { + fake.b3TraceIDMutex.RLock() + defer fake.b3TraceIDMutex.RUnlock() + return len(fake.b3TraceIDArgsForCall) +} + +func (fake *FakeConfig) B3TraceIDCalls(stub func() string) { + fake.b3TraceIDMutex.Lock() + defer fake.b3TraceIDMutex.Unlock() + fake.B3TraceIDStub = stub +} + +func (fake *FakeConfig) B3TraceIDReturns(result1 string) { + fake.b3TraceIDMutex.Lock() + defer fake.b3TraceIDMutex.Unlock() + fake.B3TraceIDStub = nil + fake.b3TraceIDReturns = struct { + result1 string + }{result1} +} + +func (fake *FakeConfig) B3TraceIDReturnsOnCall(i int, result1 string) { + fake.b3TraceIDMutex.Lock() + defer fake.b3TraceIDMutex.Unlock() + fake.B3TraceIDStub = nil + if fake.b3TraceIDReturnsOnCall == nil { + fake.b3TraceIDReturnsOnCall = make(map[int]struct { + result1 string + }) + } + fake.b3TraceIDReturnsOnCall[i] = struct { + result1 string + }{result1} +} + func (fake *FakeConfig) BinaryName() string { fake.binaryNameMutex.Lock() ret, specificReturn := fake.binaryNameReturnsOnCall[len(fake.binaryNameArgsForCall)] @@ -4028,6 +4154,10 @@ func (fake *FakeConfig) Invocations() map[string][][]interface{} { defer fake.addPluginRepositoryMutex.RUnlock() fake.authorizationEndpointMutex.RLock() defer fake.authorizationEndpointMutex.RUnlock() + fake.b3SpanIDMutex.RLock() + defer fake.b3SpanIDMutex.RUnlock() + fake.b3TraceIDMutex.RLock() + defer fake.b3TraceIDMutex.RUnlock() fake.binaryNameMutex.RLock() defer fake.binaryNameMutex.RUnlock() fake.binaryVersionMutex.RLock() diff --git a/command/config.go b/command/config.go index 676223c7923..84e7abdb8ff 100644 --- a/command/config.go +++ b/command/config.go @@ -15,6 +15,8 @@ type Config interface { AddPluginRepository(name string, url string) AuthorizationEndpoint() string APIVersion() string + B3TraceID() string + B3SpanID() string BinaryName() string BinaryVersion() string CFPassword() string diff --git a/command/v7/shared/new_clients.go b/command/v7/shared/new_clients.go index 4486bd8ffab..3e6cae4efd7 100644 --- a/command/v7/shared/new_clients.go +++ b/command/v7/shared/new_clients.go @@ -47,6 +47,7 @@ func NewWrappedCloudControllerClient(config command.Config, ui command.UI, extra } ccWrappers = append(ccWrappers, extraWrappers...) + ccWrappers = append(ccWrappers, ccWrapper.NewCCTraceHeaderRequest(config.B3TraceID(), config.B3SpanID())) ccWrappers = append(ccWrappers, ccWrapper.NewRetryRequest(config.RequestRetryCount())) return ccv3.NewClient(ccv3.Config{ @@ -85,6 +86,7 @@ func newWrappedUAAClient(config command.Config, ui command.UI) (*uaa.Client, err uaaAuthWrapper := uaaWrapper.NewUAAAuthentication(uaaClient, config) uaaClient.WrapConnection(uaaAuthWrapper) + uaaClient.WrapConnection(uaaWrapper.NewUAATraceHeaderRequest(config.B3TraceID(), config.B3SpanID())) uaaClient.WrapConnection(uaaWrapper.NewRetryRequest(config.RequestRetryCount())) err = uaaClient.SetupResources(config.UAAEndpoint(), config.AuthorizationEndpoint()) diff --git a/util/configv3/env.go b/util/configv3/env.go index 27e15504fcd..023cfb24546 100644 --- a/util/configv3/env.go +++ b/util/configv3/env.go @@ -5,6 +5,8 @@ import ( "strconv" "strings" "time" + + "code.cloudfoundry.org/cli/util/random" ) // EnvOverride represents all the environment variables read by the CF CLI @@ -20,6 +22,8 @@ type EnvOverride struct { CFStartupTimeout string CFTrace string CFUsername string + CFB3TraceID string + CFB3SpanID string DockerPassword string CNBCredentials string Experimental string @@ -160,3 +164,17 @@ func (config *Config) StartupTimeout() time.Duration { return DefaultStartupTimeout } + +func (config *Config) B3TraceID() string { + if config.ENV.CFB3TraceID == "" { + config.ENV.CFB3TraceID = random.GenerateHex(32) + } + return config.ENV.CFB3TraceID +} + +func (config *Config) B3SpanID() string { + if config.ENV.CFB3SpanID == "" { + config.ENV.CFB3SpanID = random.GenerateHex(16) + } + return config.ENV.CFB3SpanID +} diff --git a/util/configv3/load_config.go b/util/configv3/load_config.go index 883b2cd31cb..e421b9a6cc4 100644 --- a/util/configv3/load_config.go +++ b/util/configv3/load_config.go @@ -127,6 +127,8 @@ func LoadConfig(flags ...FlagOverride) (*Config, error) { CFStartupTimeout: os.Getenv("CF_STARTUP_TIMEOUT"), CFTrace: os.Getenv("CF_TRACE"), CFUsername: os.Getenv("CF_USERNAME"), + CFB3TraceID: os.Getenv("CF_B3_TRACE_ID"), + CFB3SpanID: os.Getenv("CF_B3_SPAN_ID"), DockerPassword: os.Getenv("CF_DOCKER_PASSWORD"), CNBCredentials: os.Getenv("CNB_REGISTRY_CREDS"), Experimental: os.Getenv("CF_CLI_EXPERIMENTAL"), diff --git a/util/random/hex.go b/util/random/hex.go new file mode 100644 index 00000000000..cd46ae4939b --- /dev/null +++ b/util/random/hex.go @@ -0,0 +1,15 @@ +package random + +import ( + "crypto/rand" + "encoding/hex" +) + +func GenerateHex(length int) string { + b := make([]byte, length/2) + if _, err := rand.Read(b); err != nil { + panic(err) + } + + return hex.EncodeToString(b) +} From d0352c76ca07fa5a1d8ea8edcb08b1efcb9ded7e Mon Sep 17 00:00:00 2001 From: Sam Gunaratne Date: Mon, 2 Dec 2024 12:00:46 -0700 Subject: [PATCH 2/2] Auto set span headers for each request --- api/cloudcontroller/wrapper/trace_request.go | 4 +-- .../wrapper/trace_request_test.go | 6 ++--- api/router/wrapper/trace_request.go | 4 +-- api/router/wrapper/trace_request_test.go | 7 ++---- api/shared/trace_headers.go | 10 +++++--- api/shared/trace_headers_test.go | 6 ++--- api/uaa/wrapper/trace_request.go | 4 +-- api/uaa/wrapper/trace_request_test.go | 6 ++--- command/config.go | 1 - command/v7/shared/new_clients.go | 6 +++-- go.mod | 1 + util/configv3/env.go | 12 ++------- util/configv3/load_config.go | 1 - util/random/hex.go | 15 ----------- util/trace/trace.go | 25 +++++++++++++++++++ util/trace/trace_suite_test.go | 13 ++++++++++ util/trace/trace_test.go | 19 ++++++++++++++ 17 files changed, 85 insertions(+), 55 deletions(-) delete mode 100644 util/random/hex.go create mode 100644 util/trace/trace.go create mode 100644 util/trace/trace_suite_test.go create mode 100644 util/trace/trace_test.go diff --git a/api/cloudcontroller/wrapper/trace_request.go b/api/cloudcontroller/wrapper/trace_request.go index 87d3298dae2..73f6fe47bad 100644 --- a/api/cloudcontroller/wrapper/trace_request.go +++ b/api/cloudcontroller/wrapper/trace_request.go @@ -12,9 +12,9 @@ type CCTraceHeaderRequest struct { } // NewCCTraceHeaderRequest returns a pointer to a CCTraceHeaderRequest wrapper. -func NewCCTraceHeaderRequest(trace, span string) *CCTraceHeaderRequest { +func NewCCTraceHeaderRequest(trace string) *CCTraceHeaderRequest { return &CCTraceHeaderRequest{ - headers: shared.NewTraceHeaders(trace, span), + headers: shared.NewTraceHeaders(trace), } } diff --git a/api/cloudcontroller/wrapper/trace_request_test.go b/api/cloudcontroller/wrapper/trace_request_test.go index b4719c7b0fd..e7a1c5b3b4f 100644 --- a/api/cloudcontroller/wrapper/trace_request_test.go +++ b/api/cloudcontroller/wrapper/trace_request_test.go @@ -23,16 +23,14 @@ var _ = Describe("CCTraceHeaderRequest", func() { makeErr error traceHeader string - spanHeader string ) BeforeEach(func() { fakeConnection = new(cloudcontrollerfakes.FakeConnection) traceHeader = "trace-id" - spanHeader = "span-id" - wrapper = NewCCTraceHeaderRequest(traceHeader, spanHeader).Wrap(fakeConnection) + wrapper = NewCCTraceHeaderRequest(traceHeader).Wrap(fakeConnection) body := bytes.NewReader([]byte("foo")) @@ -54,7 +52,7 @@ var _ = Describe("CCTraceHeaderRequest", func() { It("Adds the request headers", func() { Expect(makeErr).NotTo(HaveOccurred()) Expect(request.Header.Get("X-B3-TraceId")).To(Equal(traceHeader)) - Expect(request.Header.Get("X-B3-SpanId")).To(Equal(spanHeader)) + Expect(request.Header.Get("X-B3-SpanId")).ToNot(BeEmpty()) }) It("Calls the inner connection", func() { diff --git a/api/router/wrapper/trace_request.go b/api/router/wrapper/trace_request.go index 724cb6ee3ad..a96a1ce2ee6 100644 --- a/api/router/wrapper/trace_request.go +++ b/api/router/wrapper/trace_request.go @@ -12,9 +12,9 @@ type RoutingTraceHeaderRequest struct { } // NewRoutingTraceHeaderRequest returns a pointer to a RoutingTraceHeaderRequest wrapper. -func NewRoutingTraceHeaderRequest(trace, span string) *RoutingTraceHeaderRequest { +func NewRoutingTraceHeaderRequest(trace string) *RoutingTraceHeaderRequest { return &RoutingTraceHeaderRequest{ - headers: shared.NewTraceHeaders(trace, span), + headers: shared.NewTraceHeaders(trace), } } diff --git a/api/router/wrapper/trace_request_test.go b/api/router/wrapper/trace_request_test.go index fd4a1bd36e8..91a420f38ee 100644 --- a/api/router/wrapper/trace_request_test.go +++ b/api/router/wrapper/trace_request_test.go @@ -23,16 +23,13 @@ var _ = Describe("CCTraceHeaderRequest", func() { makeErr error traceHeader string - spanHeader string ) BeforeEach(func() { fakeConnection = new(routerfakes.FakeConnection) traceHeader = "trace-id" - spanHeader = "span-id" - - wrapper = NewRoutingTraceHeaderRequest(traceHeader, spanHeader).Wrap(fakeConnection) + wrapper = NewRoutingTraceHeaderRequest(traceHeader).Wrap(fakeConnection) body := bytes.NewReader([]byte("foo")) @@ -54,7 +51,7 @@ var _ = Describe("CCTraceHeaderRequest", func() { It("Adds the request headers", func() { Expect(makeErr).NotTo(HaveOccurred()) Expect(request.Header.Get("X-B3-TraceId")).To(Equal(traceHeader)) - Expect(request.Header.Get("X-B3-SpanId")).To(Equal(spanHeader)) + Expect(request.Header.Get("X-B3-SpanId")).ToNot(BeEmpty()) }) It("Calls the inner connection", func() { diff --git a/api/shared/trace_headers.go b/api/shared/trace_headers.go index 1aec94d62f2..e913b5c64cf 100644 --- a/api/shared/trace_headers.go +++ b/api/shared/trace_headers.go @@ -2,6 +2,8 @@ package shared import ( "net/http" + + "code.cloudfoundry.org/cli/util/trace" ) const ( @@ -12,14 +14,12 @@ const ( // TraceHeaders sets b3 trace headers to requests. type TraceHeaders struct { b3trace string - b3span string } // NewTraceHeaders returns a pointer to a TraceHeaderRequest. -func NewTraceHeaders(trace, span string) *TraceHeaders { +func NewTraceHeaders(trace string) *TraceHeaders { return &TraceHeaders{ b3trace: trace, - b3span: span, } } @@ -30,6 +30,8 @@ func (t *TraceHeaders) SetHeaders(request *http.Request) { request.Header.Add(B3TraceIDHeader, t.b3trace) } if request.Header.Get(B3SpanIDHeader) == "" { - request.Header.Add(B3SpanIDHeader, t.b3span) + request.Header.Add(B3SpanIDHeader, trace.GenerateRandomTraceID(16)) } + + // request.Header.Add(("B3", request.Header.Get(B3TraceIDHeader)+request.Header.Get(B3SpanIDHeader))) } diff --git a/api/shared/trace_headers_test.go b/api/shared/trace_headers_test.go index c5efe888b01..660dde0f302 100644 --- a/api/shared/trace_headers_test.go +++ b/api/shared/trace_headers_test.go @@ -13,7 +13,7 @@ var _ = Describe("B3 Trace Headers", func() { Describe("SetHeaders", func() { Context("when there are already headers set", func() { It("does not add the headers", func() { - traceHeaders := NewTraceHeaders("new_trace_id", "new_span_id") + traceHeaders := NewTraceHeaders("new_trace_id") request := &http.Request{ Header: http.Header{}, } @@ -28,14 +28,14 @@ var _ = Describe("B3 Trace Headers", func() { Context("when there are no headers set", func() { It("adds the headers", func() { - traceHeaders := NewTraceHeaders("new_trace_id", "new_span_id") + traceHeaders := NewTraceHeaders("new_trace_id") request := &http.Request{ Header: http.Header{}, } traceHeaders.SetHeaders(request) Expect(request.Header.Get("X-B3-TraceId")).To(Equal("new_trace_id")) - Expect(request.Header.Get("X-B3-SpanId")).To(Equal("new_span_id")) + Expect(request.Header.Get("X-B3-SpanId")).ToNot(BeEmpty()) }) }) }) diff --git a/api/uaa/wrapper/trace_request.go b/api/uaa/wrapper/trace_request.go index 635812ed826..dedebf6e755 100644 --- a/api/uaa/wrapper/trace_request.go +++ b/api/uaa/wrapper/trace_request.go @@ -14,9 +14,9 @@ type UAATraceHeaderRequest struct { } // NewUAATraceHeaderRequest returns a pointer to a UAATraceHeaderRequest wrapper. -func NewUAATraceHeaderRequest(trace, span string) *UAATraceHeaderRequest { +func NewUAATraceHeaderRequest(trace string) *UAATraceHeaderRequest { return &UAATraceHeaderRequest{ - headers: shared.NewTraceHeaders(trace, span), + headers: shared.NewTraceHeaders(trace), } } diff --git a/api/uaa/wrapper/trace_request_test.go b/api/uaa/wrapper/trace_request_test.go index 9f9aae39101..fc978792905 100644 --- a/api/uaa/wrapper/trace_request_test.go +++ b/api/uaa/wrapper/trace_request_test.go @@ -23,16 +23,14 @@ var _ = Describe("CCTraceHeaderRequest", func() { makeErr error traceHeader string - spanHeader string ) BeforeEach(func() { fakeConnection = new(uaafakes.FakeConnection) traceHeader = "trace-id" - spanHeader = "span-id" - wrapper = NewUAATraceHeaderRequest(traceHeader, spanHeader).Wrap(fakeConnection) + wrapper = NewUAATraceHeaderRequest(traceHeader).Wrap(fakeConnection) body := bytes.NewReader([]byte("foo")) @@ -54,7 +52,7 @@ var _ = Describe("CCTraceHeaderRequest", func() { It("Adds the request headers", func() { Expect(makeErr).NotTo(HaveOccurred()) Expect(request.Header.Get("X-B3-TraceId")).To(Equal(traceHeader)) - Expect(request.Header.Get("X-B3-SpanId")).To(Equal(spanHeader)) + Expect(request.Header.Get("X-B3-SpanId")).ToNot(BeEmpty()) }) It("Calls the inner connection", func() { diff --git a/command/config.go b/command/config.go index 84e7abdb8ff..bde3b4a754b 100644 --- a/command/config.go +++ b/command/config.go @@ -16,7 +16,6 @@ type Config interface { AuthorizationEndpoint() string APIVersion() string B3TraceID() string - B3SpanID() string BinaryName() string BinaryVersion() string CFPassword() string diff --git a/command/v7/shared/new_clients.go b/command/v7/shared/new_clients.go index 3e6cae4efd7..b1e719c5928 100644 --- a/command/v7/shared/new_clients.go +++ b/command/v7/shared/new_clients.go @@ -47,7 +47,7 @@ func NewWrappedCloudControllerClient(config command.Config, ui command.UI, extra } ccWrappers = append(ccWrappers, extraWrappers...) - ccWrappers = append(ccWrappers, ccWrapper.NewCCTraceHeaderRequest(config.B3TraceID(), config.B3SpanID())) + ccWrappers = append(ccWrappers, ccWrapper.NewCCTraceHeaderRequest(config.B3TraceID())) ccWrappers = append(ccWrappers, ccWrapper.NewRetryRequest(config.RequestRetryCount())) return ccv3.NewClient(ccv3.Config{ @@ -86,7 +86,7 @@ func newWrappedUAAClient(config command.Config, ui command.UI) (*uaa.Client, err uaaAuthWrapper := uaaWrapper.NewUAAAuthentication(uaaClient, config) uaaClient.WrapConnection(uaaAuthWrapper) - uaaClient.WrapConnection(uaaWrapper.NewUAATraceHeaderRequest(config.B3TraceID(), config.B3SpanID())) + uaaClient.WrapConnection(uaaWrapper.NewUAATraceHeaderRequest(config.B3TraceID())) uaaClient.WrapConnection(uaaWrapper.NewRetryRequest(config.RequestRetryCount())) err = uaaClient.SetupResources(config.UAAEndpoint(), config.AuthorizationEndpoint()) @@ -123,6 +123,8 @@ func newWrappedRoutingClient(config command.Config, ui command.UI, uaaClient *ua authWrapper := routingWrapper.NewUAAAuthentication(uaaClient, config) routingWrappers = append(routingWrappers, authWrapper) + routingWrappers = append(routingWrappers, routingWrapper.NewRoutingTraceHeaderRequest(config.B3TraceID())) + routingConfig.Wrappers = routingWrappers routingClient := router.NewClient(routingConfig) diff --git a/go.mod b/go.mod index 04b2f59fe21..380d0cea595 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( github.com/distribution/reference v0.6.0 github.com/fatih/color v1.18.0 github.com/google/go-querystring v1.1.0 + github.com/google/uuid v1.6.0 github.com/jessevdk/go-flags v1.6.1 github.com/lunixbochs/vtclean v1.0.0 github.com/mattn/go-colorable v0.1.13 diff --git a/util/configv3/env.go b/util/configv3/env.go index 023cfb24546..75e198cfa04 100644 --- a/util/configv3/env.go +++ b/util/configv3/env.go @@ -6,7 +6,7 @@ import ( "strings" "time" - "code.cloudfoundry.org/cli/util/random" + "code.cloudfoundry.org/cli/util/trace" ) // EnvOverride represents all the environment variables read by the CF CLI @@ -23,7 +23,6 @@ type EnvOverride struct { CFTrace string CFUsername string CFB3TraceID string - CFB3SpanID string DockerPassword string CNBCredentials string Experimental string @@ -167,14 +166,7 @@ func (config *Config) StartupTimeout() time.Duration { func (config *Config) B3TraceID() string { if config.ENV.CFB3TraceID == "" { - config.ENV.CFB3TraceID = random.GenerateHex(32) + config.ENV.CFB3TraceID = trace.GenerateUUIDTraceID() } return config.ENV.CFB3TraceID } - -func (config *Config) B3SpanID() string { - if config.ENV.CFB3SpanID == "" { - config.ENV.CFB3SpanID = random.GenerateHex(16) - } - return config.ENV.CFB3SpanID -} diff --git a/util/configv3/load_config.go b/util/configv3/load_config.go index e421b9a6cc4..228c4f3f809 100644 --- a/util/configv3/load_config.go +++ b/util/configv3/load_config.go @@ -128,7 +128,6 @@ func LoadConfig(flags ...FlagOverride) (*Config, error) { CFTrace: os.Getenv("CF_TRACE"), CFUsername: os.Getenv("CF_USERNAME"), CFB3TraceID: os.Getenv("CF_B3_TRACE_ID"), - CFB3SpanID: os.Getenv("CF_B3_SPAN_ID"), DockerPassword: os.Getenv("CF_DOCKER_PASSWORD"), CNBCredentials: os.Getenv("CNB_REGISTRY_CREDS"), Experimental: os.Getenv("CF_CLI_EXPERIMENTAL"), diff --git a/util/random/hex.go b/util/random/hex.go deleted file mode 100644 index cd46ae4939b..00000000000 --- a/util/random/hex.go +++ /dev/null @@ -1,15 +0,0 @@ -package random - -import ( - "crypto/rand" - "encoding/hex" -) - -func GenerateHex(length int) string { - b := make([]byte, length/2) - if _, err := rand.Read(b); err != nil { - panic(err) - } - - return hex.EncodeToString(b) -} diff --git a/util/trace/trace.go b/util/trace/trace.go new file mode 100644 index 00000000000..3bd530f1e24 --- /dev/null +++ b/util/trace/trace.go @@ -0,0 +1,25 @@ +package trace + +import ( + "crypto/rand" + "encoding/hex" + "strings" + + "github.com/google/uuid" +) + +// GenerateUUIDTraceID returns a UUID v4 string with the dashes removed as a 32 lower-hex encoded string. +func GenerateUUIDTraceID() string { + uuidV4 := uuid.New() + return strings.ReplaceAll(uuidV4.String(), "-", "") +} + +// GenerateRandomTraceID returns a random hex string of the given length. +func GenerateRandomTraceID(length int) string { + b := make([]byte, length/2) + if _, err := rand.Read(b); err != nil { + panic(err) + } + + return hex.EncodeToString(b) +} diff --git a/util/trace/trace_suite_test.go b/util/trace/trace_suite_test.go new file mode 100644 index 00000000000..943b695d17c --- /dev/null +++ b/util/trace/trace_suite_test.go @@ -0,0 +1,13 @@ +package trace_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "testing" +) + +func TestTrace(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Trace Suite") +} diff --git a/util/trace/trace_test.go b/util/trace/trace_test.go new file mode 100644 index 00000000000..75c18720759 --- /dev/null +++ b/util/trace/trace_test.go @@ -0,0 +1,19 @@ +package trace_test + +import ( + "code.cloudfoundry.org/cli/util/trace" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("GenerateHex", func() { + It("returns random trace id", func() { + Expect(trace.GenerateUUIDTraceID()).To(HaveLen(32)) + }) +}) + +var _ = Describe("GenerateRandomTraceID", func() { + It("returns random trace id", func() { + Expect(trace.GenerateRandomTraceID(16)).To(HaveLen(16)) + }) +})