From 538abdd524305f1c1de4b497ced4874f829d7461 Mon Sep 17 00:00:00 2001 From: Tania Pham Date: Tue, 4 Jun 2024 11:56:26 -0700 Subject: [PATCH 1/7] change to rr --- config/configgrpc/configgrpc.go | 33 ++++++++++++++++------------ config/configgrpc/configgrpc_test.go | 22 +++++++++---------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 98d428857ce..370832e4f74 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -13,17 +13,6 @@ import ( "github.com/mostynb/go-grpc-compression/nonclobbering/snappy" "github.com/mostynb/go-grpc-compression/nonclobbering/zstd" - "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" - "go.opentelemetry.io/otel" - "google.golang.org/grpc" - "google.golang.org/grpc/balancer" - "google.golang.org/grpc/credentials" - "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/encoding/gzip" - "google.golang.org/grpc/keepalive" - "google.golang.org/grpc/metadata" - "google.golang.org/grpc/peer" - "go.opentelemetry.io/collector/client" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configauth" @@ -34,6 +23,16 @@ import ( "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/config/internal" "go.opentelemetry.io/collector/extension/auth" + "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" + "go.opentelemetry.io/otel" + "google.golang.org/grpc" + "google.golang.org/grpc/balancer" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/encoding/gzip" + "google.golang.org/grpc/keepalive" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" ) var errMetadataNotFound = errors.New("no request metadata found") @@ -55,6 +54,11 @@ func NewDefaultKeepaliveClientConfig() *KeepaliveClientConfig { } } +// NewDefaultBalancernameClientConfig returns a new instance of BalancernameClientConfig with default values. +func NewDefaultBalancernameClientConfig() string { + return `{"loadBalancingConfig": [{"round_robin":{}}]}` +} + // ClientConfig defines common settings for a gRPC client configuration. type ClientConfig struct { // The target to which the exporter is going to send traces or metrics, @@ -102,9 +106,10 @@ type ClientConfig struct { // NewDefaultClientConfig returns a new instance of ClientConfig with default values. func NewDefaultClientConfig() *ClientConfig { return &ClientConfig{ - TLSSetting: configtls.NewDefaultClientConfig(), - Keepalive: NewDefaultKeepaliveClientConfig(), - Auth: configauth.NewDefaultAuthentication(), + TLSSetting: configtls.NewDefaultClientConfig(), + Keepalive: NewDefaultKeepaliveClientConfig(), + Auth: configauth.NewDefaultAuthentication(), + BalancerName: NewDefaultBalancernameClientConfig(), } } diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index 6f0a98aa382..0b3fee755ea 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -15,13 +15,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.uber.org/zap" - "go.uber.org/zap/zaptest/observer" - "google.golang.org/grpc" - "google.golang.org/grpc/balancer" - "google.golang.org/grpc/metadata" - "google.golang.org/grpc/peer" - "go.opentelemetry.io/collector/client" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/component/componenttest" @@ -33,6 +26,12 @@ import ( "go.opentelemetry.io/collector/extension/auth" "go.opentelemetry.io/collector/extension/auth/authtest" "go.opentelemetry.io/collector/pdata/ptrace/ptraceotlp" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" + "google.golang.org/grpc" + "google.golang.org/grpc/balancer" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" ) func TestNewDefaultKeepaliveClientConfig(t *testing.T) { @@ -46,9 +45,10 @@ func TestNewDefaultKeepaliveClientConfig(t *testing.T) { func TestNewDefaultClientConfig(t *testing.T) { expected := &ClientConfig{ - TLSSetting: configtls.NewDefaultClientConfig(), - Keepalive: NewDefaultKeepaliveClientConfig(), - Auth: configauth.NewDefaultAuthentication(), + TLSSetting: configtls.NewDefaultClientConfig(), + Keepalive: NewDefaultKeepaliveClientConfig(), + Auth: configauth.NewDefaultAuthentication(), + BalancerName: NewDefaultBalancernameClientConfig(), } result := NewDefaultClientConfig() @@ -213,7 +213,7 @@ func TestAllGrpcClientSettings(t *testing.T) { ReadBufferSize: 1024, WriteBufferSize: 1024, WaitForReady: true, - BalancerName: "configgrpc_balancer_test", + BalancerName: "round_robin", Authority: "pseudo-authority", Auth: &configauth.Authentication{AuthenticatorID: testAuthID}, }, From c5e9acb1b544978f47fa5b888b886d9bce6ee37e Mon Sep 17 00:00:00 2001 From: Tania Pham Date: Tue, 4 Jun 2024 15:01:59 -0700 Subject: [PATCH 2/7] add log --- .chloggen/load-balancer-round-robin.yaml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .chloggen/load-balancer-round-robin.yaml diff --git a/.chloggen/load-balancer-round-robin.yaml b/.chloggen/load-balancer-round-robin.yaml new file mode 100644 index 00000000000..f59b098a2ff --- /dev/null +++ b/.chloggen/load-balancer-round-robin.yaml @@ -0,0 +1,19 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: grpcconfig + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Update default pick_first to round_robin load balancer + +# One or more tracking issues or pull requests related to the change +issues: [10319] + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +change_logs: [user] \ No newline at end of file From 587fa179c069c82dbb67e4e49684d955671b2596 Mon Sep 17 00:00:00 2001 From: Tania Pham Date: Wed, 5 Jun 2024 08:53:50 -0700 Subject: [PATCH 3/7] linting and fixes --- config/configgrpc/configgrpc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 370832e4f74..f7b9e48ab0e 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -56,7 +56,7 @@ func NewDefaultKeepaliveClientConfig() *KeepaliveClientConfig { // NewDefaultBalancernameClientConfig returns a new instance of BalancernameClientConfig with default values. func NewDefaultBalancernameClientConfig() string { - return `{"loadBalancingConfig": [{"round_robin":{}}]}` + return "round_robin" } // ClientConfig defines common settings for a gRPC client configuration. From 787c1e75ffeec8f19af8c6d9408b1dad4c15f36f Mon Sep 17 00:00:00 2001 From: Tania Pham Date: Wed, 5 Jun 2024 12:28:59 -0700 Subject: [PATCH 4/7] fix imports --- config/configgrpc/configgrpc.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 09e26ffca9c..016be04a2e3 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -34,16 +34,6 @@ import ( "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/config/internal" "go.opentelemetry.io/collector/extension/auth" - "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" - "go.opentelemetry.io/otel" - "google.golang.org/grpc" - "google.golang.org/grpc/balancer" - "google.golang.org/grpc/credentials" - "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/encoding/gzip" - "google.golang.org/grpc/keepalive" - "google.golang.org/grpc/metadata" - "google.golang.org/grpc/peer" ) var errMetadataNotFound = errors.New("no request metadata found") From 7ea24a62d0e8bc6789a7cb6dc8f194d53c95cde4 Mon Sep 17 00:00:00 2001 From: Tania Pham Date: Wed, 5 Jun 2024 12:31:23 -0700 Subject: [PATCH 5/7] fix imports test --- config/configgrpc/configgrpc_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index b372cae57d9..fdda9e2d9ee 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -15,6 +15,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" + "google.golang.org/grpc" + "google.golang.org/grpc/balancer" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" + "go.opentelemetry.io/collector/client" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/component/componenttest" @@ -26,12 +33,6 @@ import ( "go.opentelemetry.io/collector/extension/auth" "go.opentelemetry.io/collector/extension/auth/authtest" "go.opentelemetry.io/collector/pdata/ptrace/ptraceotlp" - "go.uber.org/zap" - "go.uber.org/zap/zaptest/observer" - "google.golang.org/grpc" - "google.golang.org/grpc/balancer" - "google.golang.org/grpc/metadata" - "google.golang.org/grpc/peer" ) func TestNewDefaultKeepaliveClientConfig(t *testing.T) { From de4392276ba792519783488e57f389a37ffb098d Mon Sep 17 00:00:00 2001 From: Tania Pham Date: Wed, 12 Jun 2024 12:07:37 -0700 Subject: [PATCH 6/7] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Juraci Paixão Kröhling --- .chloggen/load-balancer-round-robin.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.chloggen/load-balancer-round-robin.yaml b/.chloggen/load-balancer-round-robin.yaml index f59b098a2ff..44405549588 100644 --- a/.chloggen/load-balancer-round-robin.yaml +++ b/.chloggen/load-balancer-round-robin.yaml @@ -4,10 +4,10 @@ change_type: enhancement # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) -component: grpcconfig +component: configgrpc # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Update default pick_first to round_robin load balancer +note: Update the default load balancer strategy to round_robin # One or more tracking issues or pull requests related to the change issues: [10319] From a8ed9559f6e745e8929d27fb71ec0e3dd1b97db0 Mon Sep 17 00:00:00 2001 From: Tania Pham Date: Thu, 13 Jun 2024 10:01:53 -0700 Subject: [PATCH 7/7] fix names and updated documentation --- .chloggen/load-balancer-round-robin.yaml | 5 +++++ config/configgrpc/README.md | 4 ++-- config/configgrpc/configgrpc.go | 6 +++--- config/configgrpc/configgrpc_test.go | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.chloggen/load-balancer-round-robin.yaml b/.chloggen/load-balancer-round-robin.yaml index 44405549588..1b9a711b7de 100644 --- a/.chloggen/load-balancer-round-robin.yaml +++ b/.chloggen/load-balancer-round-robin.yaml @@ -12,6 +12,11 @@ note: Update the default load balancer strategy to round_robin # One or more tracking issues or pull requests related to the change issues: [10319] +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: To restore the behavior that was previously the default, set `balancer_name` to `pick_first`. + # Optional: The change log or logs in which this entry should be included. # e.g. '[user]' or '[user, api]' # Include 'user' if the change is relevant to end users. diff --git a/config/configgrpc/README.md b/config/configgrpc/README.md index b9cf2f01be2..e0db0421e7e 100644 --- a/config/configgrpc/README.md +++ b/config/configgrpc/README.md @@ -15,8 +15,8 @@ configuration parameters are also defined under `tls` like server configuration. For more information, see [configtls README](../configtls/README.md). -- [`balancer_name`](https://github.com/grpc/grpc-go/blob/master/examples/features/load_balancing/README.md) -- `compression` Compression type to use among `gzip`, `snappy`, `zstd`, and `none`. +- [`balancer_name`](https://github.com/grpc/grpc-go/blob/master/examples/features/load_balancing/README.md): Default before v0.103.0 is `pick_first`, default for v0.103.0 is `round_robin`. See [issue](https://github.com/open-telemetry/opentelemetry-collector/issues/10298). To restore the previous behavior, set `balancer_name` to `pick_first`. +- `compression`: Compression type to use among `gzip`, `snappy`, `zstd`, and `none`. - `endpoint`: Valid value syntax available [here](https://github.com/grpc/grpc/blob/master/doc/naming.md) - [`tls`](../configtls/README.md) - `headers`: name/value pairs added to the request diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index cdc13591cd8..73176c9dec9 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -55,8 +55,8 @@ func NewDefaultKeepaliveClientConfig() *KeepaliveClientConfig { } } -// NewDefaultBalancernameClientConfig returns a new instance of BalancernameClientConfig with default values. -func NewDefaultBalancernameClientConfig() string { +// BalancerName returns a string with default load balancer value +func BalancerName() string { return "round_robin" } @@ -110,7 +110,7 @@ func NewDefaultClientConfig() *ClientConfig { TLSSetting: configtls.NewDefaultClientConfig(), Keepalive: NewDefaultKeepaliveClientConfig(), Auth: configauth.NewDefaultAuthentication(), - BalancerName: NewDefaultBalancernameClientConfig(), + BalancerName: BalancerName(), } } diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index fdda9e2d9ee..11f1eaab689 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -49,7 +49,7 @@ func TestNewDefaultClientConfig(t *testing.T) { TLSSetting: configtls.NewDefaultClientConfig(), Keepalive: NewDefaultKeepaliveClientConfig(), Auth: configauth.NewDefaultAuthentication(), - BalancerName: NewDefaultBalancernameClientConfig(), + BalancerName: BalancerName(), } result := NewDefaultClientConfig()