feat(router): more intuitive subgraph traffic shaping config inheritance

router-tests/circuit_breaker_test.go

@@ -2,13 +2,14 @@ package integration
 
 import (
 	"context"
-	"github.com/prometheus/client_golang/prometheus"
-	io_prometheus_client "github.com/prometheus/client_model/go"
 	"net/http"
 	"sort"
 	"sync/atomic"
 	"testing"
 
+	"github.com/prometheus/client_golang/prometheus"
+	io_prometheus_client "github.com/prometheus/client_model/go"
+
 	"github.com/stretchr/testify/require"
 	"github.com/wundergraph/cosmo/router-tests/testenv"
 	"github.com/wundergraph/cosmo/router/core"

router-tests/lifecycle/shutdown_test.go

@@ -27,7 +27,7 @@ func TestShutdownGoroutineLeaks(t *testing.T) {
 
 		RouterOptions: []core.Option{
 			core.WithSubgraphTransportOptions(core.NewSubgraphTransportOptions(config.TrafficShapingRules{
-				Subgraphs: map[string]*config.GlobalSubgraphRequestRule{
+				Subgraphs: map[string]config.GlobalSubgraphRequestRule{
 					"employees": {
 						MaxIdleConns: integration.ToPtr(10),
 					},

router-tests/prometheus_test.go

@@ -4,15 +4,16 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	rmetric "github.com/wundergraph/cosmo/router/pkg/metric"
-	"github.com/wundergraph/cosmo/router/pkg/otel"
-	"go.opentelemetry.io/otel/sdk/metric/metricdata"
 	"net/http"
 	"regexp"
 	"strings"
 	"testing"
 	"time"
 
+	rmetric "github.com/wundergraph/cosmo/router/pkg/metric"
+	"github.com/wundergraph/cosmo/router/pkg/otel"
+	"go.opentelemetry.io/otel/sdk/metric/metricdata"
+
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/collectors"
 	io_prometheus_client "github.com/prometheus/client_model/go"
@@ -4645,7 +4646,7 @@ func TestFlakyPrometheusRouterConnectionMetrics(t *testing.T) {
 			All: config.GlobalSubgraphRequestRule{
 				RequestTimeout: PointerOf(200 * time.Millisecond),
 			},
-			Subgraphs: map[string]*config.GlobalSubgraphRequestRule{
+			Subgraphs: map[string]config.GlobalSubgraphRequestRule{
 				"availability": {
 					RequestTimeout: PointerOf(300 * time.Millisecond),
 				},

router-tests/telemetry/connection_metrics_test.go

@@ -161,7 +161,7 @@ func TestConnectionMetrics(t *testing.T) {
 			All: config.GlobalSubgraphRequestRule{
 				RequestTimeout: integration.ToPtr(200 * time.Millisecond),
 			},
-			Subgraphs: map[string]*config.GlobalSubgraphRequestRule{
+			Subgraphs: map[string]config.GlobalSubgraphRequestRule{
 				"availability": {
 					RequestTimeout: integration.ToPtr(300 * time.Millisecond),
 				},

router-tests/telemetry/telemetry_test.go

@@ -3639,7 +3639,7 @@ func TestFlakyTelemetry(t *testing.T) {
 						All: config.GlobalSubgraphRequestRule{
 							RequestTimeout: integration.ToPtr(10 * time.Second),
 						},
-						Subgraphs: map[string]*config.GlobalSubgraphRequestRule{
+						Subgraphs: map[string]config.GlobalSubgraphRequestRule{
 							"hobbies": {
 								RequestTimeout: integration.ToPtr(3 * time.Second),
 							},

router-tests/timeout_test.go

@@ -47,7 +47,7 @@ func TestFlakyTimeouts(t *testing.T) {
 			All: config.GlobalSubgraphRequestRule{
 				RequestTimeout: ToPtr(500 * time.Millisecond),
 			},
-			Subgraphs: map[string]*config.GlobalSubgraphRequestRule{
+			Subgraphs: map[string]config.GlobalSubgraphRequestRule{
 				"test1": {
 					ResponseHeaderTimeout: ToPtr(100 * time.Millisecond),
 				},
@@ -181,7 +181,7 @@ func TestFlakyTimeouts(t *testing.T) {
 			All: config.GlobalSubgraphRequestRule{
 				RequestTimeout: ToPtr(200 * time.Millisecond),
 			},
-			Subgraphs: map[string]*config.GlobalSubgraphRequestRule{
+			Subgraphs: map[string]config.GlobalSubgraphRequestRule{
 				"hobbies": {
 					RequestTimeout: ToPtr(300 * time.Millisecond),
 				},
@@ -297,7 +297,7 @@ func TestFlakyTimeouts(t *testing.T) {
 			All: config.GlobalSubgraphRequestRule{
 				RequestTimeout: ToPtr(500 * time.Millisecond),
 			},
-			Subgraphs: map[string]*config.GlobalSubgraphRequestRule{
+			Subgraphs: map[string]config.GlobalSubgraphRequestRule{
 				"hobbies": {
 					ResponseHeaderTimeout: ToPtr(100 * time.Millisecond),
 				},

router/core/router.go

@@ -1816,8 +1816,12 @@ func DefaultFileUploadConfig() *config.FileUpload {
 	}
 }
 
-func NewTransportRequestOptions(cfg config.GlobalSubgraphRequestRule) *TransportRequestOptions {
-	defaults := DefaultTransportRequestOptions()
+// NewTransportRequestOptions creates a new TransportRequestOptions instance with the given configuration and defaults.
+// If defaults is nil, it uses the global default values.
+func NewTransportRequestOptions(cfg config.GlobalSubgraphRequestRule, defaults *TransportRequestOptions) *TransportRequestOptions {
+	if defaults == nil {
+		defaults = DefaultTransportRequestOptions()
+	}
 
 	return &TransportRequestOptions{
 		RequestTimeout:         or(cfg.RequestTimeout, defaults.RequestTimeout),
@@ -1850,13 +1854,15 @@ func DefaultTransportRequestOptions() *TransportRequestOptions {
 }
 
 func NewSubgraphTransportOptions(cfg config.TrafficShapingRules) *SubgraphTransportOptions {
+	allRequestOptions := NewTransportRequestOptions(cfg.All, nil)
+
 	base := &SubgraphTransportOptions{
-		TransportRequestOptions: NewTransportRequestOptions(cfg.All),
+		TransportRequestOptions: allRequestOptions,
 		SubgraphMap:             map[string]*TransportRequestOptions{},
 	}
 
 	for k, v := range cfg.Subgraphs {
-		base.SubgraphMap[k] = NewTransportRequestOptions(*v)
+		base.SubgraphMap[k] = NewTransportRequestOptions(v, allRequestOptions)
 	}
 
 	return base
@@ -1872,9 +1878,8 @@ func NewSubgraphCircuitBreakerOptions(cfg config.TrafficShapingRules) *SubgraphC
 	}
 	// Subgraph specific circuit breakers
 	for k, v := range cfg.Subgraphs {
-		if v != nil {
-			entry.SubgraphMap[k] = newCircuitBreakerConfig(v.CircuitBreaker)
-		}
+		entry.SubgraphMap[k] = newCircuitBreakerConfig(v.CircuitBreaker)
+
 	}
 
 	return entry

router/core/router_test.go

@@ -230,40 +230,159 @@ func TestOverridesPriority(t *testing.T) {
 }
 
 func TestTrafficShapingRules(t *testing.T) {
-	allRequestTimeout := 10 * time.Second
-	allDialTimeout := 0 * time.Second
-	subgraphRequestTimeout := 15 * time.Second
-	subgraphDialTimeout := 0 * time.Second
+	t.Run("loads defaults correctly when empty", func(t *testing.T) {
+		config := config.TrafficShapingRules{
+			All: config.GlobalSubgraphRequestRule{},
+		}
 
-	defaults := DefaultTransportRequestOptions()
+		defaults := DefaultTransportRequestOptions()
 
-	config := config.TrafficShapingRules{
-		All: config.GlobalSubgraphRequestRule{
-			RequestTimeout: &allRequestTimeout,
-			DialTimeout:    &allDialTimeout,
-		},
-		Subgraphs: map[string]*config.GlobalSubgraphRequestRule{
-			"some-subgraph": {
-				RequestTimeout: &subgraphRequestTimeout,
-				DialTimeout:    &subgraphDialTimeout,
+		options := []Option{
+			WithSubgraphTransportOptions(NewSubgraphTransportOptions(config)),
+		}
+		router, err := NewRouter(options...)
+		assert.Nil(t, err)
+
+		// Assert that configs are properly loaded from defaults when empty
+		assert.Equal(t, defaults.RequestTimeout, router.subgraphTransportOptions.RequestTimeout)
+		assert.Equal(t, defaults.TLSHandshakeTimeout, router.subgraphTransportOptions.TLSHandshakeTimeout)
+		assert.Equal(t, defaults.ResponseHeaderTimeout, router.subgraphTransportOptions.ResponseHeaderTimeout)
+		assert.Equal(t, defaults.ExpectContinueTimeout, router.subgraphTransportOptions.ExpectContinueTimeout)
+		assert.Equal(t, defaults.KeepAliveProbeInterval, router.subgraphTransportOptions.KeepAliveProbeInterval)
+		assert.Equal(t, defaults.KeepAliveIdleTimeout, router.subgraphTransportOptions.KeepAliveIdleTimeout)
+		assert.Equal(t, defaults.DialTimeout, router.subgraphTransportOptions.DialTimeout)
+		assert.Equal(t, defaults.MaxConnsPerHost, router.subgraphTransportOptions.MaxConnsPerHost)
+		assert.Equal(t, defaults.MaxIdleConns, router.subgraphTransportOptions.MaxIdleConns)
+		assert.Equal(t, defaults.MaxIdleConnsPerHost, router.subgraphTransportOptions.MaxIdleConnsPerHost)
+	})
+
+	t.Run("loads set values over defaults when populated", func(t *testing.T) {
+		allRequestTimeout := 60 * time.Second
+		allTLSHandshakeTimeout := 10 * time.Second
+		allResponseHeaderTimeout := 0 * time.Second
+		allExpectContinueTimeout := 0 * time.Second
+		allKeepAliveProbeInterval := 30 * time.Second
+		allKeepAliveIdleTimeout := 90 * time.Second
+		allDialTimeout := 30 * time.Second
+		allMaxConnsPerHost := 100
+		allMaxIdleConns := 1024
+		allMaxIdleConnsPerHost := 20
+
+		config := config.TrafficShapingRules{
+			All: config.GlobalSubgraphRequestRule{
+				RequestTimeout:         &allRequestTimeout,
+				TLSHandshakeTimeout:    &allTLSHandshakeTimeout,
+				ResponseHeaderTimeout:  &allResponseHeaderTimeout,
+				ExpectContinueTimeout:  &allExpectContinueTimeout,
+				KeepAliveProbeInterval: &allKeepAliveProbeInterval,
+				KeepAliveIdleTimeout:   &allKeepAliveIdleTimeout,
+				DialTimeout:            &allDialTimeout,
+				MaxConnsPerHost:        &allMaxConnsPerHost,
+				MaxIdleConns:           &allMaxIdleConns,
+				MaxIdleConnsPerHost:    &allMaxIdleConnsPerHost,
 			},
-		},
-	}
+		}
 
-	options := []Option{
-		WithSubgraphTransportOptions(NewSubgraphTransportOptions(config)),
-	}
-	router, err := NewRouter(options...)
-	assert.Nil(t, err)
+		options := []Option{
+			WithSubgraphTransportOptions(NewSubgraphTransportOptions(config)),
+		}
+
+		router, err := NewRouter(options...)
+		assert.Nil(t, err)
+
+		// Assert that configs are properly loaded over defaults when populated
+		assert.Equal(t, allRequestTimeout, router.subgraphTransportOptions.RequestTimeout)
+		assert.Equal(t, allDialTimeout, router.subgraphTransportOptions.DialTimeout)
+		assert.Equal(t, allMaxConnsPerHost, router.subgraphTransportOptions.MaxConnsPerHost)
+		assert.Equal(t, allTLSHandshakeTimeout, router.subgraphTransportOptions.TLSHandshakeTimeout)
+		assert.Equal(t, allResponseHeaderTimeout, router.subgraphTransportOptions.ResponseHeaderTimeout)
+		assert.Equal(t, allExpectContinueTimeout, router.subgraphTransportOptions.ExpectContinueTimeout)
+		assert.Equal(t, allKeepAliveProbeInterval, router.subgraphTransportOptions.KeepAliveProbeInterval)
+		assert.Equal(t, allKeepAliveIdleTimeout, router.subgraphTransportOptions.KeepAliveIdleTimeout)
+		assert.Equal(t, allMaxIdleConns, router.subgraphTransportOptions.MaxIdleConns)
+		assert.Equal(t, allMaxIdleConnsPerHost, router.subgraphTransportOptions.MaxIdleConnsPerHost)
+	})
+
+	t.Run("falls through to defaults when partially populated", func(t *testing.T) {
+		allRequestTimeout := 60 * time.Second
+
+		config := config.TrafficShapingRules{
+			All: config.GlobalSubgraphRequestRule{
+				RequestTimeout: &allRequestTimeout,
+			},
+		}
+
+		defaults := DefaultTransportRequestOptions()
+
+		options := []Option{
+			WithSubgraphTransportOptions(NewSubgraphTransportOptions(config)),
+		}
+		router, err := NewRouter(options...)
+		assert.Nil(t, err)
+
+		// Loads the populated value
+		assert.Equal(t, allRequestTimeout, router.subgraphTransportOptions.RequestTimeout)
+
+		// Falls through to defaults when not set
+		assert.Equal(t, defaults.TLSHandshakeTimeout, router.subgraphTransportOptions.TLSHandshakeTimeout)
+		assert.Equal(t, defaults.ResponseHeaderTimeout, router.subgraphTransportOptions.ResponseHeaderTimeout)
+		assert.Equal(t, defaults.ExpectContinueTimeout, router.subgraphTransportOptions.ExpectContinueTimeout)
+		assert.Equal(t, defaults.KeepAliveProbeInterval, router.subgraphTransportOptions.KeepAliveProbeInterval)
+		assert.Equal(t, defaults.KeepAliveIdleTimeout, router.subgraphTransportOptions.KeepAliveIdleTimeout)
+		assert.Equal(t, defaults.DialTimeout, router.subgraphTransportOptions.DialTimeout)
+		assert.Equal(t, defaults.MaxConnsPerHost, router.subgraphTransportOptions.MaxConnsPerHost)
+		assert.Equal(t, defaults.MaxIdleConns, router.subgraphTransportOptions.MaxIdleConns)
+		assert.Equal(t, defaults.MaxIdleConnsPerHost, router.subgraphTransportOptions.MaxIdleConnsPerHost)
+	})
+
+	t.Run("loads subgraph specific options with fallback to all and defaults", func(t *testing.T) {
+		allRequestTimeout := 10 * time.Second
+		allDialTimeout := 0 * time.Second
+		allMaxConnsPerHost := 1024
+
+		subgraphRequestTimeout := 15 * time.Second
+		subgraphDialTimeout := 0 * time.Second
+
+		defaults := DefaultTransportRequestOptions()
+
+		config := config.TrafficShapingRules{
+			All: config.GlobalSubgraphRequestRule{
+				RequestTimeout:  &allRequestTimeout,
+				DialTimeout:     &allDialTimeout,
+				MaxConnsPerHost: &allMaxConnsPerHost,
+			},
+			Subgraphs: map[string]config.GlobalSubgraphRequestRule{
+				"some-subgraph": {
+					RequestTimeout: &subgraphRequestTimeout,
+					DialTimeout:    &subgraphDialTimeout,
+				},
+			},
+		}
+
+		options := []Option{
+			WithSubgraphTransportOptions(NewSubgraphTransportOptions(config)),
+		}
+		router, err := NewRouter(options...)
+		assert.Nil(t, err)
+
+		// Assert that configs are loaded for real, zero and absent values.
+		assert.Equal(t, allRequestTimeout, router.subgraphTransportOptions.RequestTimeout)
+		assert.Equal(t, allDialTimeout, router.subgraphTransportOptions.DialTimeout)
+		assert.Equal(t, allMaxConnsPerHost, router.subgraphTransportOptions.MaxConnsPerHost)
+		assert.Equal(t, defaults.MaxIdleConns, router.subgraphTransportOptions.MaxIdleConns)
+
+		subgraphRequestOptions := router.subgraphTransportOptions.SubgraphMap["some-subgraph"]
+
+		// Subgraph specific configurations
+		assert.Equal(t, subgraphRequestTimeout, subgraphRequestOptions.RequestTimeout)
+		assert.Equal(t, subgraphDialTimeout, subgraphRequestOptions.DialTimeout)
 
-	// Assert that configs are loaded for real, zero and absent values.
-	assert.Equal(t, allRequestTimeout, router.subgraphTransportOptions.RequestTimeout)
-	assert.Equal(t, allDialTimeout, router.subgraphTransportOptions.DialTimeout)
-	assert.Equal(t, defaults.MaxIdleConns, router.subgraphTransportOptions.MaxIdleConns)
+		// Inherit from `all`
+		assert.Equal(t, allMaxConnsPerHost, subgraphRequestOptions.MaxConnsPerHost)
 
-	assert.Equal(t, subgraphRequestTimeout, router.subgraphTransportOptions.SubgraphMap["some-subgraph"].RequestTimeout)
-	assert.Equal(t, subgraphDialTimeout, router.subgraphTransportOptions.SubgraphMap["some-subgraph"].DialTimeout)
-	assert.Equal(t, defaults.MaxIdleConns, router.subgraphTransportOptions.SubgraphMap["some-subgraph"].MaxIdleConns)
+		// Inherit from global defaults
+		assert.Equal(t, defaults.MaxIdleConns, subgraphRequestOptions.MaxIdleConns)
+	})
 }
 
 // Confirms that defaults and fallthrough works properly
@@ -272,13 +391,13 @@ func TestNewTransportRequestOptions(t *testing.T) {
 
 	subgraphRequestTimeout := 10 * time.Second
 	subgraphDialTimeout := 0 * time.Second
-	subgraphConfig := &config.GlobalSubgraphRequestRule{
+	subgraphConfig := config.GlobalSubgraphRequestRule{
 		RequestTimeout: &subgraphRequestTimeout,
 		DialTimeout:    &subgraphDialTimeout,
 	}
 
 	// Test that the defaults are set properly
-	transportCfg := NewTransportRequestOptions(*subgraphConfig)
+	transportCfg := NewTransportRequestOptions(subgraphConfig, nil)
 
 	// The two set values are preserved, including the manually specified zero
 	assert.Equal(t, subgraphRequestTimeout, transportCfg.RequestTimeout)

router/pkg/config/config.go

@@ -167,7 +167,7 @@ type TrafficShapingRules struct {
 	// Apply to requests from clients to the router
 	Router RouterTrafficConfiguration `yaml:"router"`
 	// Subgraphs is a set of rules that apply to requests from the router to subgraphs. The key is the subgraph name.
-	Subgraphs map[string]*GlobalSubgraphRequestRule `yaml:"subgraphs,omitempty"`
+	Subgraphs map[string]GlobalSubgraphRequestRule `yaml:"subgraphs,omitempty"`
 }
 
 type FileUpload struct {