diff --git a/router-tests/circuit_breaker_test.go b/router-tests/circuit_breaker_test.go index d16ea91232..7e7f1b60eb 100644 --- a/router-tests/circuit_breaker_test.go +++ b/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" diff --git a/router-tests/lifecycle/shutdown_test.go b/router-tests/lifecycle/shutdown_test.go index 03e0312917..be4576e12d 100644 --- a/router-tests/lifecycle/shutdown_test.go +++ b/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), }, diff --git a/router-tests/prometheus_test.go b/router-tests/prometheus_test.go index e6200bac1f..69e03f8cc8 100644 --- a/router-tests/prometheus_test.go +++ b/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), }, diff --git a/router-tests/telemetry/connection_metrics_test.go b/router-tests/telemetry/connection_metrics_test.go index f1270f6292..b40bedae2f 100644 --- a/router-tests/telemetry/connection_metrics_test.go +++ b/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), }, diff --git a/router-tests/telemetry/telemetry_test.go b/router-tests/telemetry/telemetry_test.go index f8058d4aef..58dc2be217 100644 --- a/router-tests/telemetry/telemetry_test.go +++ b/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), }, diff --git a/router-tests/timeout_test.go b/router-tests/timeout_test.go index a295906095..128a4ef540 100644 --- a/router-tests/timeout_test.go +++ b/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), }, diff --git a/router/core/router.go b/router/core/router.go index 99ce3944c9..4bb1d7935e 100644 --- a/router/core/router.go +++ b/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 diff --git a/router/core/router_test.go b/router/core/router_test.go index 429581a15a..812d327cbd 100644 --- a/router/core/router_test.go +++ b/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) diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index cfd10de0c6..8253737b6e 100644 --- a/router/pkg/config/config.go +++ b/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 {