[app] Improve HTTP Clients

We are now using our default transporter for all http clients which comes with tracing out of the box. We are now also using NewRequestWithContext instead of NewRequest in all places.
kobsio · Jul 22, 2022 · 4490c9f · 4490c9f
1 parent 3ec93d2
commit 4490c9f
Show file tree

Hide file tree

Showing 44 changed files with 124 additions and 76 deletions.
diff --git a/pkg/hub/api/applications/applications_test.go b/pkg/hub/api/applications/applications_test.go
@@ -111,7 +111,7 @@ func TestGetApplications(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient, otel.Tracer("applications")}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 
@@ -192,7 +192,7 @@ func TestGetApplicationsCount(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient, otel.Tracer("applications")}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 
@@ -246,7 +246,7 @@ func TestGetTags(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient, otel.Tracer("applications")}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 
@@ -340,7 +340,7 @@ func TestGetApplication(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient, otel.Tracer("applications")}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 
@@ -467,7 +467,7 @@ func TestGetApplicationsByTeam(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient, otel.Tracer("applications")}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 

diff --git a/pkg/hub/api/clusters/clusters_test.go b/pkg/hub/api/clusters/clusters_test.go
@@ -59,7 +59,7 @@ func TestGetClusters(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 
@@ -117,7 +117,7 @@ func TestGetNamespaces(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 
@@ -171,7 +171,7 @@ func TestGetResources(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 

diff --git a/pkg/hub/api/dashboards/dashboards_test.go b/pkg/hub/api/dashboards/dashboards_test.go
@@ -2,6 +2,7 @@ package dashboards
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -48,7 +49,7 @@ func TestGetDashboardsFromReferences(t *testing.T) {
 			router := Router{chi.NewRouter(), mockStoreClient}
 			router.Get("/dashboards", router.getDashboardsFromReferences)
 
-			req, _ := http.NewRequest(http.MethodPost, "/dashboards", bytes.NewBuffer(tt.body))
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodPost, "/dashboards", bytes.NewBuffer(tt.body))
 			w := httptest.NewRecorder()
 
 			router.getDashboardsFromReferences(w, req)
@@ -87,7 +88,7 @@ func TestGetDashboard(t *testing.T) {
 			router := Router{chi.NewRouter(), mockStoreClient}
 			router.Get("/dashboard", router.getDashboard)
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			w := httptest.NewRecorder()
 
 			router.getDashboard(w, req)

diff --git a/pkg/hub/api/navigation/navigation_test.go b/pkg/hub/api/navigation/navigation_test.go
@@ -32,7 +32,7 @@ func TestGetNavigationGroups(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			router := Router{chi.NewRouter(), tt.config}
 
-			req, _ := http.NewRequest(http.MethodGet, "/groups", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/groups", nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 

diff --git a/pkg/hub/api/notifications/notifications_test.go b/pkg/hub/api/notifications/notifications_test.go
@@ -130,7 +130,7 @@ func TestGetNotificationGroups(t *testing.T) {
 
 			router := Router{chi.NewRouter(), tt.config, mockStoreClient, otel.Tracer("notifications")}
 
-			req, _ := http.NewRequest(http.MethodGet, "/groups", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/groups", nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 

diff --git a/pkg/hub/api/teams/teams_test.go b/pkg/hub/api/teams/teams_test.go
@@ -116,7 +116,7 @@ func TestGetTeams(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 
@@ -197,7 +197,7 @@ func TestGetTeam(t *testing.T) {
 
 			router := Router{chi.NewRouter(), mockStoreClient}
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			rctx := chi.NewRouteContext()
 			req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
 

diff --git a/pkg/hub/middleware/userauth/auth_test.go b/pkg/hub/middleware/userauth/auth_test.go
@@ -231,7 +231,7 @@ func TestAuthHandler(t *testing.T) {
 			router.Use(tt.auth.Handler)
 			router.Mount(url, Mount(""))
 
-			req, _ := http.NewRequest(http.MethodGet, url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil)
 			tt.prepareRequest(req)
 
 			w := httptest.NewRecorder()

diff --git a/pkg/hub/middleware/userauth/router_test.go b/pkg/hub/middleware/userauth/router_test.go
@@ -34,7 +34,7 @@ func TestUserAuthHandler(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			router := Router{chi.NewRouter(), ""}
 
-			req, _ := http.NewRequest(http.MethodGet, "/user", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/user", nil)
 			req = req.WithContext(context.WithValue(req.Context(), authContext.UserKey, tt.user))
 
 			w := httptest.NewRecorder()
@@ -49,7 +49,7 @@ func TestUserAuthHandler(t *testing.T) {
 func TestUserLogoutHandler(t *testing.T) {
 	router := Router{chi.NewRouter(), ""}
 
-	req, _ := http.NewRequest(http.MethodGet, "/user", nil)
+	req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/user", nil)
 
 	w := httptest.NewRecorder()
 	router.userLogoutHandler(w, req)

diff --git a/pkg/hub/satellites/satellite/request.go b/pkg/hub/satellites/satellite/request.go
@@ -9,6 +9,7 @@ import (
 	authContext "github.com/kobsio/kobs/pkg/hub/middleware/userauth/context"
 	"github.com/kobsio/kobs/pkg/middleware/errresponse"
 
+	"github.com/go-chi/chi/v5/middleware"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/propagation"
 )
@@ -24,13 +25,15 @@ func doRequest[T any](ctx context.Context, user *authContext.User, client *http.
 	}
 
 	otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(req.Header))
-
 	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
 	if user != nil {
 		req.Header.Set("x-kobs-user", user.ToString())
 	} else {
 		req.Header.Set("x-kobs-user", "{\"email\": \"\"}")
 	}
+	if requestID := middleware.GetReqID(ctx); requestID != "" {
+		req.Header.Set("requestID", requestID)
+	}
 
 	resp, err := client.Do(req)
 	if err != nil {

diff --git a/pkg/hub/satellites/satellite/satellite.go b/pkg/hub/satellites/satellite/satellite.go
@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"net/http/httputil"
 	"net/url"
-	"time"
 
 	authContext "github.com/kobsio/kobs/pkg/hub/middleware/userauth/context"
 	applicationv1 "github.com/kobsio/kobs/pkg/kube/apis/application/v1"
@@ -16,9 +15,9 @@ import (
 	"github.com/kobsio/kobs/pkg/kube/clusters/cluster"
 	"github.com/kobsio/kobs/pkg/log"
 	"github.com/kobsio/kobs/pkg/middleware/errresponse"
+	"github.com/kobsio/kobs/pkg/middleware/roundtripper"
 	"github.com/kobsio/kobs/pkg/satellite/plugins/plugin"
 
-	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
@@ -221,8 +220,7 @@ func NewClient(config Config) (Client, error) {
 	return &client{
 		config: config,
 		httpClient: &http.Client{
-			Transport: otelhttp.NewTransport(http.DefaultTransport),
-			Timeout:   60 * time.Second,
+			Transport: roundtripper.DefaultRoundTripper,
 		},
 		proxyURL: proxyURL,
 		tracer:   otel.Tracer("satellite"),

diff --git a/pkg/middleware/roundtripper/roundtripper.go b/pkg/middleware/roundtripper/roundtripper.go
@@ -4,17 +4,19 @@ import (
 	"net"
 	"net/http"
 	"time"
+
+	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 )
 
 // DefaultRoundTripper is our default RoundTripper.
-var DefaultRoundTripper http.RoundTripper = &http.Transport{
+var DefaultRoundTripper http.RoundTripper = otelhttp.NewTransport(&http.Transport{
 	Proxy: http.ProxyFromEnvironment,
 	DialContext: (&net.Dialer{
 		Timeout:   30 * time.Second,
 		KeepAlive: 30 * time.Second,
 	}).DialContext,
 	TLSHandshakeTimeout: 10 * time.Second,
-}
+})
 
 // BasicAuthTransport is the struct to add basic auth to a RoundTripper.
 type BasicAuthTransport struct {

diff --git a/pkg/middleware/roundtripper/roundtripper_test.go b/pkg/middleware/roundtripper/roundtripper_test.go
@@ -1,6 +1,7 @@
 package roundtripper
 
 import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -31,7 +32,7 @@ func TestBasicAuthTransport(t *testing.T) {
 		Password:  "admin",
 	}
 
-	req, _ := http.NewRequest(http.MethodGet, "/", nil)
+	req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil)
 	roundTripper.RoundTrip(req)
 
 	w := httptest.NewRecorder()
@@ -49,7 +50,7 @@ func TestTokenAuthTransporter(t *testing.T) {
 		Token:     "admin",
 	}
 
-	req, _ := http.NewRequest(http.MethodGet, "/", nil)
+	req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil)
 	roundTripper.RoundTrip(req)
 
 	w := httptest.NewRecorder()

diff --git a/pkg/satellite/api/applications/applications_test.go b/pkg/satellite/api/applications/applications_test.go
@@ -1,6 +1,7 @@
 package applications
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -57,7 +58,7 @@ func TestGetApplications(t *testing.T) {
 			router := Router{chi.NewRouter(), Config{}, mockClustersClient}
 			router.Get("/applications", router.getApplications)
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			w := httptest.NewRecorder()
 
 			router.getApplications(w, req)

diff --git a/pkg/satellite/api/clusters/clusters_test.go b/pkg/satellite/api/clusters/clusters_test.go
@@ -1,6 +1,7 @@
 package clusters
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -40,7 +41,7 @@ func TestGetClusters(t *testing.T) {
 			router := Router{chi.NewRouter(), Config{}, mockClustersClient}
 			router.Get("/clusters", router.getClusters)
 
-			req, _ := http.NewRequest(http.MethodGet, tt.url, nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil)
 			w := httptest.NewRecorder()
 
 			router.getClusters(w, req)
@@ -93,7 +94,7 @@ func TestGetNamespaces(t *testing.T) {
 			}
 			router.Get("/namespaces", router.getNamespaces)
 
-			req, _ := http.NewRequest(http.MethodGet, "/namespaces", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/namespaces", nil)
 			w := httptest.NewRecorder()
 
 			router.getNamespaces(w, req)
@@ -121,7 +122,7 @@ func TestGetCRDs(t *testing.T) {
 	}
 	router.Get("/crds", router.getCRDs)
 
-	req, _ := http.NewRequest(http.MethodGet, "/crds", nil)
+	req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/crds", nil)
 	w := httptest.NewRecorder()
 
 	router.getCRDs(w, req)

diff --git a/pkg/satellite/api/dashboards/dashboards_test.go b/pkg/satellite/api/dashboards/dashboards_test.go
@@ -1,6 +1,7 @@
 package dashboards
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -59,7 +60,7 @@ func TestGetDashboards(t *testing.T) {
 			}
 			router.Get("/dashboards", router.getDashboards)
 
-			req, _ := http.NewRequest(http.MethodGet, "/dashboards", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/dashboards", nil)
 			w := httptest.NewRecorder()
 
 			router.getDashboards(w, req)

diff --git a/pkg/satellite/api/teams/teams_test.go b/pkg/satellite/api/teams/teams_test.go
@@ -1,6 +1,7 @@
 package teams
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -52,7 +53,7 @@ func TestGetTeams(t *testing.T) {
 			router := Router{Mux: chi.NewRouter(), clustersClient: mockClustersClient, config: Config{}}
 			router.Get("/teams", router.getTeams)
 
-			req, _ := http.NewRequest(http.MethodGet, "/teams", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/teams", nil)
 			w := httptest.NewRecorder()
 
 			router.getTeams(w, req)

diff --git a/pkg/satellite/api/users/users_test.go b/pkg/satellite/api/users/users_test.go
@@ -1,6 +1,7 @@
 package users
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -51,7 +52,7 @@ func TestGetUsers(t *testing.T) {
 			router := Router{Mux: chi.NewRouter(), clustersClient: mockClustersClient, config: Config{}}
 			router.Get("/users", router.getUsers)
 
-			req, _ := http.NewRequest(http.MethodGet, "/users", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/users", nil)
 			w := httptest.NewRecorder()
 
 			router.getUsers(w, req)

diff --git a/pkg/satellite/middleware/tokenauth/tokenauth_test.go b/pkg/satellite/middleware/tokenauth/tokenauth_test.go
@@ -1,6 +1,7 @@
 package tokenauth
 
 import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -47,7 +48,7 @@ func TestHandler(t *testing.T) {
 				render.JSON(w, r, nil)
 			})
 
-			req, _ := http.NewRequest(http.MethodGet, "/", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil)
 			tt.prepareRequest(req)
 
 			w := httptest.NewRecorder()

diff --git a/pkg/satellite/middleware/user/user_test.go b/pkg/satellite/middleware/user/user_test.go
@@ -1,6 +1,7 @@
 package user
 
 import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -47,7 +48,7 @@ func TestHandler(t *testing.T) {
 				render.JSON(w, r, nil)
 			})
 
-			req, _ := http.NewRequest(http.MethodGet, "/", nil)
+			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "/", nil)
 			tt.prepareRequest(req)
 
 			w := httptest.NewRecorder()