Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .chloggen/grpc-extension-context.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
change_type: bug_fix

component: pkg/extensionmiddleware

note: Add context.Context to gRPC middleware interface constructors.

subtext: This is a breaking API change for components that implement or use extensionmiddleware.

issues: [14523]

change_logs: [api]
9 changes: 9 additions & 0 deletions .chloggen/grpc-middlware-context.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
change_type: bug_fix
Comment thread
evan-bradley marked this conversation as resolved.

component: pkg/config/configmiddleware

note: Add context.Context to gRPC middleware interface constructors.

issues: [14523]

change_logs: [api]
2 changes: 1 addition & 1 deletion config/configgrpc/client_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func newTestMiddlewareConfig(name string) configmiddleware.Config {
func newTestClientMiddleware(name string) extension.Extension {
return &testClientMiddleware{
Extension: extensionmiddlewaretest.NewNop(),
GetGRPCClientOptionsFunc: func() ([]grpc.DialOption, error) {
GetGRPCClientOptionsFunc: func(_ context.Context) ([]grpc.DialOption, error) {
return []grpc.DialOption{
grpc.WithChainUnaryInterceptor(
func(
Expand Down
2 changes: 1 addition & 1 deletion config/configgrpc/server_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type testServerMiddleware struct {
func newTestServerMiddleware(name string) extension.Extension {
return &testServerMiddleware{
Extension: extensionmiddlewaretest.NewNop(),
GetGRPCServerOptionsFunc: func() ([]grpc.ServerOption, error) {
GetGRPCServerOptionsFunc: func(_ context.Context) ([]grpc.ServerOption, error) {
return []grpc.ServerOption{grpc.ChainUnaryInterceptor(
func(
ctx context.Context,
Expand Down
8 changes: 4 additions & 4 deletions config/configmiddleware/configmiddleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ func (m Config) GetHTTPServerHandler(_ context.Context, extensions map[component
// extensionmiddleware.GRPCClient from the map of extensions, and
// returns the gRPC dial options. If a middleware is not found, an
// error is returned. This should only be used by gRPC clients.
func (m Config) GetGRPCClientOptions(_ context.Context, extensions map[component.ID]component.Component) ([]grpc.DialOption, error) {
func (m Config) GetGRPCClientOptions(ctx context.Context, extensions map[component.ID]component.Component) ([]grpc.DialOption, error) {
if ext, found := extensions[m.ID]; found {
if client, ok := ext.(extensionmiddleware.GRPCClient); ok {
return client.GetGRPCClientOptions()
return client.GetGRPCClientOptions(ctx)
}
return nil, errNotGRPCClient
}
Expand All @@ -82,10 +82,10 @@ func (m Config) GetGRPCClientOptions(_ context.Context, extensions map[component
// extensionmiddleware.GRPCServer from the map of extensions, and
// returns the gRPC server options. If a middleware is not found, an
// error is returned. This should only be used by gRPC servers.
func (m Config) GetGRPCServerOptions(_ context.Context, extensions map[component.ID]component.Component) ([]grpc.ServerOption, error) {
func (m Config) GetGRPCServerOptions(ctx context.Context, extensions map[component.ID]component.Component) ([]grpc.ServerOption, error) {
if ext, found := extensions[m.ID]; found {
if server, ok := ext.(extensionmiddleware.GRPCServer); ok {
return server.GetGRPCServerOptions()
return server.GetGRPCServerOptions(ctx)
}
return nil, errNotGRPCServer
}
Expand Down
4 changes: 2 additions & 2 deletions config/configmiddleware/configmiddleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func TestConfig_GetGRPCServerOptions(t *testing.T) {
extensionmiddleware.GetGRPCServerOptionsFunc
}{
Extension: extensionmiddlewaretest.NewNop(),
GetGRPCServerOptionsFunc: func() ([]grpc.ServerOption, error) {
GetGRPCServerOptionsFunc: func(context.Context) ([]grpc.ServerOption, error) {
return []grpc.ServerOption{
grpc.EmptyServerOption{},
}, nil
Expand Down Expand Up @@ -212,7 +212,7 @@ func TestConfig_GetGRPCClientOptions(t *testing.T) {
extensionmiddleware.GetGRPCClientOptionsFunc
}{
Extension: extensionmiddlewaretest.NewNop(),
GetGRPCClientOptionsFunc: func() ([]grpc.DialOption, error) {
GetGRPCClientOptionsFunc: func(_ context.Context) ([]grpc.DialOption, error) {
return []grpc.DialOption{
grpc.EmptyDialOption{},
}, nil
Expand Down
9 changes: 5 additions & 4 deletions extension/extensionmiddleware/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package extensionmiddleware // import "go.opentelemetry.io/collector/extension/extensionmiddleware"

import (
"context"
"net/http"

"google.golang.org/grpc"
Expand All @@ -18,7 +19,7 @@ type HTTPClient interface {
// GRPCClient is an interface for gRPC client middleware extensions.
type GRPCClient interface {
// GetGRPCClientOptions returns the gRPC dial options to use for client connections.
GetGRPCClientOptions() ([]grpc.DialOption, error)
GetGRPCClientOptions(context.Context) ([]grpc.DialOption, error)
Comment thread
jmacd marked this conversation as resolved.
}

var _ HTTPClient = (*GetHTTPRoundTripperFunc)(nil)
Expand All @@ -36,11 +37,11 @@ func (f GetHTTPRoundTripperFunc) GetHTTPRoundTripper(base http.RoundTripper) (ht
var _ GRPCClient = (*GetGRPCClientOptionsFunc)(nil)

// GetGRPCClientOptionsFunc is a function that implements GRPCClient.
type GetGRPCClientOptionsFunc func() ([]grpc.DialOption, error)
type GetGRPCClientOptionsFunc func(context.Context) ([]grpc.DialOption, error)

func (f GetGRPCClientOptionsFunc) GetGRPCClientOptions() ([]grpc.DialOption, error) {
func (f GetGRPCClientOptionsFunc) GetGRPCClientOptions(ctx context.Context) ([]grpc.DialOption, error) {
if f == nil {
return nil, nil
}
return f()
return f(ctx)
}
31 changes: 15 additions & 16 deletions extension/extensionmiddleware/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package extensionmiddleware

import (
"context"
"errors"
"net/http"
"testing"
Expand Down Expand Up @@ -46,44 +47,42 @@ func TestGetHTTPRoundTripperFunc(t *testing.T) {
}

func TestGetGRPCClientOptionsFunc(t *testing.T) {
type testCtx struct{}
var (
key = testCtx{}
value = "testval"
)
testctx := context.WithValue(context.Background(), key, value)

t.Run("nil function", func(t *testing.T) {
var nilFunc GetGRPCClientOptionsFunc
options, err := nilFunc.GetGRPCClientOptions()
options, err := nilFunc.GetGRPCClientOptions(testctx)
require.NoError(t, err)
require.Nil(t, options)
})

t.Run("empty options function", func(t *testing.T) {
emptyFunc := GetGRPCClientOptionsFunc(func() ([]grpc.DialOption, error) {
return []grpc.DialOption{}, nil
})

options, err := emptyFunc.GetGRPCClientOptions()
require.NoError(t, err)
require.Empty(t, options)
})

t.Run("options function", func(t *testing.T) {
// Create some test dial options
dialOpt1 := grpc.WithAuthority("test-authority")
dialOpt2 := grpc.WithDisableRetry()

optionsFunc := GetGRPCClientOptionsFunc(func() ([]grpc.DialOption, error) {
optionsFunc := GetGRPCClientOptionsFunc(func(ctx context.Context) ([]grpc.DialOption, error) {
require.Equal(t, ctx.Value(key), value)
return []grpc.DialOption{dialOpt1, dialOpt2}, nil
})

options, err := optionsFunc.GetGRPCClientOptions()
options, err := optionsFunc.GetGRPCClientOptions(testctx)
require.NoError(t, err)
require.Len(t, options, 2)
})

t.Run("error function", func(t *testing.T) {
expectedErr := errors.New("grpc options error")
errorFunc := GetGRPCClientOptionsFunc(func() ([]grpc.DialOption, error) {
errorFunc := GetGRPCClientOptionsFunc(func(ctx context.Context) ([]grpc.DialOption, error) {
require.Equal(t, ctx.Value(key), value)
return nil, expectedErr
})

options, err := errorFunc.GetGRPCClientOptions()
options, err := errorFunc.GetGRPCClientOptions(testctx)
require.Error(t, err)
require.Equal(t, expectedErr, err)
require.Nil(t, options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package extensionmiddlewaretest // import "go.opentelemetry.io/collector/extension/extensionmiddleware/extensionmiddlewaretest"

import (
"context"
"net/http"

"google.golang.org/grpc"
Expand Down Expand Up @@ -37,13 +38,13 @@ func NewErr(err error) extension.Extension {
GetHTTPRoundTripperFunc: func(http.RoundTripper) (http.RoundTripper, error) {
return nil, err
},
GetGRPCClientOptionsFunc: func() ([]grpc.DialOption, error) {
GetGRPCClientOptionsFunc: func(context.Context) ([]grpc.DialOption, error) {
return nil, err
},
GetHTTPHandlerFunc: func(http.Handler) (http.Handler, error) {
return nil, err
},
GetGRPCServerOptionsFunc: func() ([]grpc.ServerOption, error) {
GetGRPCServerOptionsFunc: func(context.Context) ([]grpc.ServerOption, error) {
return nil, err
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package extensionmiddlewaretest

import (
"context"
"errors"
"testing"

Expand All @@ -22,7 +23,7 @@ func TestErrClient(t *testing.T) {

grpcClient, ok := client.(extensionmiddleware.GRPCClient)
require.True(t, ok)
_, err = grpcClient.GetGRPCClientOptions()
_, err = grpcClient.GetGRPCClientOptions(context.Background())
require.Error(t, err)
}

Expand All @@ -36,6 +37,6 @@ func TestErrServer(t *testing.T) {

grpcServer, ok := server.(extensionmiddleware.GRPCServer)
require.True(t, ok)
_, err = grpcServer.GetGRPCServerOptions()
_, err = grpcServer.GetGRPCServerOptions(context.Background())
require.Error(t, err)
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package extensionmiddlewaretest

import (
"context"
"net/http"
"testing"

Expand All @@ -23,7 +24,7 @@ func TestNopClient(t *testing.T) {

grpcClient, ok := client.(extensionmiddleware.GRPCClient)
require.True(t, ok)
grpcOpts, err := grpcClient.GetGRPCClientOptions()
grpcOpts, err := grpcClient.GetGRPCClientOptions(context.Background())
require.NoError(t, err)
require.Nil(t, grpcOpts)
}
Expand All @@ -39,7 +40,7 @@ func TestNopServer(t *testing.T) {

grpcServer, ok := client.(extensionmiddleware.GRPCServer)
require.True(t, ok)
grpcOpts, err := grpcServer.GetGRPCServerOptions()
grpcOpts, err := grpcServer.GetGRPCServerOptions(context.Background())
require.NoError(t, err)
require.Nil(t, grpcOpts)
}
Expand Down
9 changes: 5 additions & 4 deletions extension/extensionmiddleware/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package extensionmiddleware // import "go.opentelemetry.io/collector/extension/extensionmiddleware"

import (
"context"
"net/http"

"google.golang.org/grpc"
Expand All @@ -18,7 +19,7 @@ type HTTPServer interface {
// GRPCServer defines the interface for gRPC server middleware extensions.
type GRPCServer interface {
// GetGRPCServerOptions returns options for a gRPC server.
GetGRPCServerOptions() ([]grpc.ServerOption, error)
GetGRPCServerOptions(context.Context) ([]grpc.ServerOption, error)
}

var _ HTTPServer = (*GetHTTPHandlerFunc)(nil)
Expand All @@ -36,11 +37,11 @@ func (f GetHTTPHandlerFunc) GetHTTPHandler(base http.Handler) (http.Handler, err
var _ GRPCServer = (*GetGRPCServerOptionsFunc)(nil)

// GetGRPCServerOptionsFunc is a function that implements GRPCServer.
type GetGRPCServerOptionsFunc func() ([]grpc.ServerOption, error)
type GetGRPCServerOptionsFunc func(context.Context) ([]grpc.ServerOption, error)

func (f GetGRPCServerOptionsFunc) GetGRPCServerOptions() ([]grpc.ServerOption, error) {
func (f GetGRPCServerOptionsFunc) GetGRPCServerOptions(ctx context.Context) ([]grpc.ServerOption, error) {
if f == nil {
return nil, nil
}
return f()
return f(ctx)
}
19 changes: 14 additions & 5 deletions extension/extensionmiddleware/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,16 @@ func TestGetHTTPHandlerFunc(t *testing.T) {
}

func TestGetGRPCServerOptionsFunc(t *testing.T) {
type testCtx struct{}
var (
key = testCtx{}
value = "testval"
)
testctx := context.WithValue(context.Background(), key, value)

t.Run("nil_function", func(t *testing.T) {
var f GetGRPCServerOptionsFunc
opts, err := f.GetGRPCServerOptions()
opts, err := f.GetGRPCServerOptions(testctx)
require.NoError(t, err)
require.Nil(t, opts)
})
Expand All @@ -84,22 +91,24 @@ func TestGetGRPCServerOptionsFunc(t *testing.T) {
}
expectedOpts := []grpc.ServerOption{grpc.UnaryInterceptor(interceptor)}

f := GetGRPCServerOptionsFunc(func() ([]grpc.ServerOption, error) {
f := GetGRPCServerOptionsFunc(func(ctx context.Context) ([]grpc.ServerOption, error) {
require.Equal(t, ctx.Value(key), value)
return expectedOpts, nil
})

opts, err := f.GetGRPCServerOptions()
opts, err := f.GetGRPCServerOptions(testctx)
require.NoError(t, err)
require.Equal(t, expectedOpts, opts)
})

t.Run("returns_error", func(t *testing.T) {
expectedErr := errors.New("test error")
f := GetGRPCServerOptionsFunc(func() ([]grpc.ServerOption, error) {
f := GetGRPCServerOptionsFunc(func(ctx context.Context) ([]grpc.ServerOption, error) {
require.Equal(t, ctx.Value(key), value)
return nil, expectedErr
})

opts, err := f.GetGRPCServerOptions()
opts, err := f.GetGRPCServerOptions(testctx)
require.Equal(t, expectedErr, err)
require.Nil(t, opts)
})
Expand Down
2 changes: 1 addition & 1 deletion extension/memorylimiterextension/memorylimiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (ml *memoryLimiterExtension) GetHTTPHandler(base http.Handler) (http.Handle
}), nil
}

func (ml *memoryLimiterExtension) GetGRPCServerOptions() ([]grpc.ServerOption, error) {
func (ml *memoryLimiterExtension) GetGRPCServerOptions(_ context.Context) ([]grpc.ServerOption, error) {
return []grpc.ServerOption{
grpc.ChainUnaryInterceptor(
func(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp any, err error) {
Expand Down
Loading