fix: Buildkite API calls now included in trace

The context was not being used in calls, leading to disconnected traces.

Using the context means creating the client each time, as the client does not support per-request context.

internal/buildkite/pipeline.go

@@ -2,39 +2,42 @@ package buildkite
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"log"
+	"net/http"
 	"net/url"
 
 	"github.com/buildkite/go-buildkite/v3/buildkite"
 	"github.com/jamestelfer/chinmina-bridge/internal/config"
 )
 
 type PipelineLookup struct {
-	client *buildkite.Client
+	token  string
+	apiURL *url.URL
 }
 
-func New(cfg config.BuildkiteConfig) PipelineLookup {
-	transport, err := buildkite.NewTokenConfig(cfg.Token, false)
-	if err != nil {
-		log.Fatalf("client config failed: %s", err)
+func New(cfg config.BuildkiteConfig) (p PipelineLookup, err error) {
+	if cfg.Token == "" {
+		err = errors.New("token must be configured for Buildkite API access")
+		return
 	}
-
-	client := buildkite.NewClient(transport.Client())
+	p.token = cfg.Token
 
 	if cfg.ApiURL != "" {
-		url, _ := url.Parse(cfg.ApiURL)
-		transport.APIHost = url.Host
-		client.BaseURL, _ = url.Parse(cfg.ApiURL)
+		u, perr := url.Parse(cfg.ApiURL)
+		if perr != nil {
+			err = fmt.Errorf("could not parse Buildkite API URL: %w", perr)
+			return
+		}
+		p.apiURL = u
 	}
 
-	return PipelineLookup{
-		client,
-	}
+	return
 }
 
 func (p PipelineLookup) RepositoryLookup(ctx context.Context, organizationSlug, pipelineSlug string) (string, error) {
-	pipeline, _, err := p.client.Pipelines.Get(organizationSlug, pipelineSlug)
+	client := p.createClient(ctx)
+	pipeline, _, err := client.Pipelines.Get(organizationSlug, pipelineSlug)
 	if err != nil {
 		return "", fmt.Errorf("failed to get pipeline called %s/%s: %w", organizationSlug, pipelineSlug, err)
 
@@ -47,3 +50,37 @@ func (p PipelineLookup) RepositoryLookup(ctx context.Context, organizationSlug,
 
 	return *repo, nil
 }
+
+// createClient creates a new Buildkite API client. A client is required for
+// every invocation, so the current context can be included in the request.
+// Without this, HTTP client traces are not attached to their parent request.
+func (p PipelineLookup) createClient(ctx context.Context) *buildkite.Client {
+	def := http.DefaultTransport
+
+	rt := roundTripperFunc(func(req *http.Request) (*http.Response, error) {
+		req = req.WithContext(ctx)
+		return def.RoundTrip(req)
+	})
+
+	transport := buildkite.TokenAuthTransport{
+		APIToken:  p.token,
+		Transport: rt,
+	}
+
+	client := buildkite.NewClient(
+		transport.Client(),
+	)
+
+	if p.apiURL != nil {
+		transport.APIHost = p.apiURL.Host
+		client.BaseURL = p.apiURL
+	}
+
+	return client
+}
+
+type roundTripperFunc func(req *http.Request) (*http.Response, error)
+
+func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) {
+	return f(req)
+}

internal/buildkite/pipeline_test.go

@@ -14,6 +14,26 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestConfigFails(t *testing.T) {
+	t.Run("NoToken", func(t *testing.T) {
+		_, err := buildkite.New(config.BuildkiteConfig{
+			Token:  "",
+			ApiURL: "",
+		})
+		require.Error(t, err)
+		assert.ErrorContains(t, err, "token must be configured for Buildkite API access")
+	})
+
+	t.Run("InvalidURL", func(t *testing.T) {
+		_, err := buildkite.New(config.BuildkiteConfig{
+			Token:  "asdbv",
+			ApiURL: "://",
+		})
+		require.Error(t, err)
+		assert.ErrorContains(t, err, "could not parse Buildkite API URL")
+	})
+}
+
 func TestRepositoryLookup_Succeeds(t *testing.T) {
 	router := http.NewServeMux()
 
@@ -38,10 +58,11 @@ func TestRepositoryLookup_Succeeds(t *testing.T) {
 	svr := httptest.NewServer(router)
 	defer svr.Close()
 
-	bk := buildkite.New(config.BuildkiteConfig{
+	bk, err := buildkite.New(config.BuildkiteConfig{
 		Token:  "expected-token",
 		ApiURL: svr.URL,
 	})
+	require.NoError(t, err)
 
 	repo, err := bk.RepositoryLookup(context.Background(), "expected-organization", "expected-pipeline")
 
@@ -78,12 +99,13 @@ func TestRepositoryLookup_SendsAuthToken(t *testing.T) {
 	svr := httptest.NewServer(router)
 	defer svr.Close()
 
-	bk := buildkite.New(config.BuildkiteConfig{
+	bk, err := buildkite.New(config.BuildkiteConfig{
 		Token:  "expected-token",
 		ApiURL: svr.URL,
 	})
+	require.NoError(t, err)
 
-	_, err := bk.RepositoryLookup(context.Background(), "expected-organization", "expected-pipeline")
+	_, err = bk.RepositoryLookup(context.Background(), "expected-organization", "expected-pipeline")
 
 	require.NoError(t, err)
 	assert.Equal(t, "Bearer expected-token", actualToken)
@@ -108,12 +130,13 @@ func TestRepositoryLookup_FailsWhenRepoNotConfigured(t *testing.T) {
 	svr := httptest.NewServer(router)
 	defer svr.Close()
 
-	bk := buildkite.New(config.BuildkiteConfig{
+	bk, err := buildkite.New(config.BuildkiteConfig{
 		Token:  "expected-token",
 		ApiURL: svr.URL,
 	})
+	require.NoError(t, err)
 
-	_, err := bk.RepositoryLookup(context.Background(), "expected-organization", "expected-pipeline")
+	_, err = bk.RepositoryLookup(context.Background(), "expected-organization", "expected-pipeline")
 
 	require.Error(t, err)
 	assert.ErrorContains(t, err, "no configured repository for pipeline expected-organization/expected-pipeline")
@@ -129,12 +152,13 @@ func TestRepositoryLookup_Fails(t *testing.T) {
 	svr := httptest.NewServer(router)
 	defer svr.Close()
 
-	bk := buildkite.New(config.BuildkiteConfig{
+	bk, err := buildkite.New(config.BuildkiteConfig{
 		Token:  "expected-token",
 		ApiURL: svr.URL,
 	})
+	require.NoError(t, err)
 
-	_, err := bk.RepositoryLookup(context.Background(), "expected-organization", "expected-pipeline")
+	_, err = bk.RepositoryLookup(context.Background(), "expected-organization", "expected-pipeline")
 
 	require.Error(t, err)
 	assert.ErrorContains(t, err, ": 418")

main.go

@@ -39,7 +39,11 @@ func configureServerRoutes(cfg config.Config) (http.Handler, error) {
 	authorized := alice.New(maxRequestSize(requestLimitBytes), authorizer)
 
 	// setup token handler and dependencies
-	bk := buildkite.New(cfg.Buildkite)
+	bk, err := buildkite.New(cfg.Buildkite)
+	if err != nil {
+		return nil, fmt.Errorf("buildkite configuration failed: %w", err)
+	}
+
 	gh, err := github.New(cfg.Github)
 	if err != nil {
 		return nil, fmt.Errorf("github configuration failed: %w", err)