Skip to content

Commit 7a2d8b5

Browse files
authored
Add context.Context to HTTP middleware interface constructors (#14549)
#### Description API change in extensionmiddleware, an unstable package to allow middleware access to the start context. HTTP only. gRPC is treated in #14536. This is technically a breaking change in configmiddleware and for all implementations of this extension. We believe no one is using this feature outside of this repository, see memorylimiter is fixed here. #### Link to tracking issue Part of #14523 #### Documentation N/A
1 parent d48b9fa commit 7a2d8b5

14 files changed

Lines changed: 137 additions & 87 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
change_type: bug_fix
2+
3+
component: pkg/config/configmiddleware
4+
5+
note: Add context.Context to HTTP middleware interface constructors.
6+
7+
subtext: This is a breaking API change for components that implement or use extensionmiddleware.
8+
9+
issues: [14523]

config/confighttp/client.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -205,16 +205,15 @@ func (cc *ClientConfig) ToClient(ctx context.Context, extensions map[component.I
205205
return nil, errors.New("middlewares were configured but this component or its host does not support extensions")
206206
}
207207
for i := len(cc.Middlewares) - 1; i >= 0; i-- {
208-
var wrapper func(http.RoundTripper) (http.RoundTripper, error)
209-
wrapper, err = cc.Middlewares[i].GetHTTPClientRoundTripper(ctx, extensions)
208+
getClient, rerr := cc.Middlewares[i].GetHTTPClientRoundTripper(ctx, extensions)
210209
// If we failed to get the middleware
211-
if err != nil {
212-
return nil, err
210+
if rerr != nil {
211+
return nil, rerr
213212
}
214-
clientTransport, err = wrapper(clientTransport)
213+
clientTransport, rerr = getClient(ctx, clientTransport)
215214
// If we failed to construct a wrapper
216-
if err != nil {
217-
return nil, err
215+
if rerr != nil {
216+
return nil, rerr
218217
}
219218
}
220219

config/confighttp/client_middleware_test.go

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,30 +32,32 @@ type testClientMiddleware struct {
3232
func newTestClientMiddleware(name string) component.Component {
3333
return &testClientMiddleware{
3434
Extension: extensionmiddlewaretest.NewNop(),
35-
GetHTTPRoundTripperFunc: func(transport http.RoundTripper) (http.RoundTripper, error) {
36-
return extensionmiddlewaretest.RoundTripperFunc(
37-
func(req *http.Request) (*http.Response, error) {
38-
resp, err := transport.RoundTrip(req)
39-
if err != nil {
40-
return resp, err
41-
}
42-
43-
// Read the original body
44-
body, err := io.ReadAll(resp.Body)
45-
if err != nil {
46-
return resp, err
47-
}
48-
_ = resp.Body.Close()
49-
50-
// Create a new body with the appended text
51-
newBody := string(body) + "\r\noutput by " + name
52-
53-
// Replace the response body
54-
resp.Body = io.NopCloser(strings.NewReader(newBody))
55-
resp.ContentLength = int64(len(newBody))
56-
57-
return resp, nil
58-
}), nil
35+
GetHTTPRoundTripperFunc: func(_ context.Context) (extensionmiddleware.WrapHTTPRoundTripperFunc, error) {
36+
return func(_ context.Context, transport http.RoundTripper) (http.RoundTripper, error) {
37+
return extensionmiddlewaretest.RoundTripperFunc(
38+
func(req *http.Request) (*http.Response, error) {
39+
resp, err := transport.RoundTrip(req)
40+
if err != nil {
41+
return resp, err
42+
}
43+
44+
// Read the original body
45+
body, err := io.ReadAll(resp.Body)
46+
if err != nil {
47+
return resp, err
48+
}
49+
_ = resp.Body.Close()
50+
51+
// Create a new body with the appended text
52+
newBody := string(body) + "\r\noutput by " + name
53+
54+
// Replace the response body
55+
resp.Body = io.NopCloser(strings.NewReader(newBody))
56+
resp.ContentLength = int64(len(newBody))
57+
58+
return resp, nil
59+
}), nil
60+
}, nil
5961
},
6062
}
6163
}

config/confighttp/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func (sc *ServerConfig) ToServer(ctx context.Context, extensions map[component.I
207207
if err != nil {
208208
return nil, err
209209
}
210-
handler, err = wrapper(handler)
210+
handler, err = wrapper(ctx, handler)
211211
// If we failed to construct a wrapper
212212
if err != nil {
213213
return nil, err

config/confighttp/server_middleware_test.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,19 @@ type testServerMiddleware struct {
3232
func newTestServerMiddleware(name string) component.Component {
3333
return &testServerMiddleware{
3434
Extension: extensionmiddlewaretest.NewNop(),
35-
GetHTTPHandlerFunc: func(handler http.Handler) (http.Handler, error) {
36-
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
37-
// Append middleware name to the URL path
38-
r.URL.Path += name + "/"
35+
GetHTTPHandlerFunc: func(_ context.Context) (extensionmiddleware.WrapHTTPHandlerFunc, error) {
36+
return func(_ context.Context, handler http.Handler) (http.Handler, error) {
37+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
38+
// Append middleware name to the URL path
39+
r.URL.Path += name + "/"
3940

40-
// Call the next handler in the chain
41-
handler.ServeHTTP(w, r)
41+
// Call the next handler in the chain
42+
handler.ServeHTTP(w, r)
4243

43-
// Add middleware name to the response
44-
_, _ = w.Write([]byte("\r\nserved by " + name))
45-
}), nil
44+
// Add middleware name to the response
45+
_, _ = w.Write([]byte("\r\nserved by " + name))
46+
}), nil
47+
}, nil
4648
},
4749
}
4850
}

config/configmiddleware/configmiddleware.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"context"
1010
"errors"
1111
"fmt"
12-
"net/http"
1312

1413
"google.golang.org/grpc"
1514

@@ -38,10 +37,10 @@ type Config struct {
3837
// returns the HTTP client wrapper function. If a middleware is not
3938
// found, an error is returned. This should only be used by HTTP
4039
// clients.
41-
func (m Config) GetHTTPClientRoundTripper(_ context.Context, extensions map[component.ID]component.Component) (func(http.RoundTripper) (http.RoundTripper, error), error) {
40+
func (m Config) GetHTTPClientRoundTripper(ctx context.Context, extensions map[component.ID]component.Component) (extensionmiddleware.WrapHTTPRoundTripperFunc, error) {
4241
if ext, found := extensions[m.ID]; found {
4342
if client, ok := ext.(extensionmiddleware.HTTPClient); ok {
44-
return client.GetHTTPRoundTripper, nil
43+
return client.GetHTTPRoundTripper(ctx)
4544
}
4645
return nil, errNotHTTPClient
4746
}
@@ -53,10 +52,10 @@ func (m Config) GetHTTPClientRoundTripper(_ context.Context, extensions map[comp
5352
// returns the http.Handler wrapper function. If a middleware is not
5453
// found, an error is returned. This should only be used by HTTP
5554
// servers.
56-
func (m Config) GetHTTPServerHandler(_ context.Context, extensions map[component.ID]component.Component) (func(http.Handler) (http.Handler, error), error) {
55+
func (m Config) GetHTTPServerHandler(ctx context.Context, extensions map[component.ID]component.Component) (extensionmiddleware.WrapHTTPHandlerFunc, error) {
5756
if ext, found := extensions[m.ID]; found {
5857
if server, ok := ext.(extensionmiddleware.HTTPServer); ok {
59-
return server.GetHTTPHandler, nil
58+
return server.GetHTTPHandler(ctx)
6059
}
6160
return nil, errNotHTTPServer
6261
}

extension/extensionmiddleware/client.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,12 @@ import (
1212

1313
// HTTPClient is an interface for HTTP client middleware extensions.
1414
type HTTPClient interface {
15-
// GetHTTPRoundTripper wraps the provided client RoundTripper.
16-
GetHTTPRoundTripper(http.RoundTripper) (http.RoundTripper, error)
15+
// GetHTTPRoundTripper initializes a client HTTP RoundTripper
16+
// wrapper, this typically called when the Collector
17+
// starts. If there is an error this returns a nil
18+
// function. If the error is nil, the WrapHTTPRoundTripperFunc
19+
// will never be nil.
20+
GetHTTPRoundTripper(context.Context) (WrapHTTPRoundTripperFunc, error)
1721
}
1822

1923
// GRPCClient is an interface for gRPC client middleware extensions.
@@ -25,13 +29,13 @@ type GRPCClient interface {
2529
var _ HTTPClient = (*GetHTTPRoundTripperFunc)(nil)
2630

2731
// GetHTTPRoundTripperFunc is a function that implements HTTPClient.
28-
type GetHTTPRoundTripperFunc func(base http.RoundTripper) (http.RoundTripper, error)
32+
type GetHTTPRoundTripperFunc func(context.Context) (WrapHTTPRoundTripperFunc, error)
2933

30-
func (f GetHTTPRoundTripperFunc) GetHTTPRoundTripper(base http.RoundTripper) (http.RoundTripper, error) {
34+
func (f GetHTTPRoundTripperFunc) GetHTTPRoundTripper(ctx context.Context) (WrapHTTPRoundTripperFunc, error) {
3135
if f == nil {
32-
return base, nil
36+
return func(_ context.Context, rt http.RoundTripper) (http.RoundTripper, error) { return rt, nil }, nil
3337
}
34-
return f(base)
38+
return f(ctx)
3539
}
3640

3741
var _ GRPCClient = (*GetGRPCClientOptionsFunc)(nil)
@@ -45,3 +49,7 @@ func (f GetGRPCClientOptionsFunc) GetGRPCClientOptions(ctx context.Context) ([]g
4549
}
4650
return f(ctx)
4751
}
52+
53+
// WrapHTTPRoundTripperFunc is called to initialize a new instance of
54+
// HTTP client middleware.
55+
type WrapHTTPRoundTripperFunc = func(context.Context, http.RoundTripper) (http.RoundTripper, error)

extension/extensionmiddleware/client_test.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,33 +16,36 @@ import (
1616
func TestGetHTTPRoundTripperFunc(t *testing.T) {
1717
// Create a base round tripper for testing
1818
baseRT := http.DefaultTransport
19+
testctx := context.Background()
1920

2021
t.Run("nil function", func(t *testing.T) {
2122
var nilFunc GetHTTPRoundTripperFunc
22-
rt, err := nilFunc.GetHTTPRoundTripper(baseRT)
23+
rtfunc, err := nilFunc.GetHTTPRoundTripper(testctx)
24+
require.NoError(t, err)
25+
26+
rt, err := rtfunc(testctx, baseRT)
2327
require.NoError(t, err)
2428
require.Equal(t, baseRT, rt)
2529
})
2630

2731
t.Run("identity function", func(t *testing.T) {
28-
identityFunc := GetHTTPRoundTripperFunc(func(base http.RoundTripper) (http.RoundTripper, error) {
29-
return base, nil
30-
})
31-
rt, err := identityFunc.GetHTTPRoundTripper(baseRT)
32+
identityFunc := GetHTTPRoundTripperFunc(nil)
33+
rtfunc, err := identityFunc.GetHTTPRoundTripper(testctx)
34+
require.NoError(t, err)
35+
rt, err := rtfunc(testctx, baseRT)
3236
require.NoError(t, err)
3337
require.Equal(t, baseRT, rt)
3438
})
3539

3640
t.Run("error function", func(t *testing.T) {
3741
expectedErr := errors.New("round tripper error")
38-
errorFunc := GetHTTPRoundTripperFunc(func(_ http.RoundTripper) (http.RoundTripper, error) {
42+
errorFunc := GetHTTPRoundTripperFunc(func(_ context.Context) (WrapHTTPRoundTripperFunc, error) {
3943
return nil, expectedErr
4044
})
41-
42-
rt, err := errorFunc.GetHTTPRoundTripper(baseRT)
45+
rtfunc, err := errorFunc.GetHTTPRoundTripper(testctx)
4346
require.Error(t, err)
4447
require.Equal(t, expectedErr, err)
45-
require.Nil(t, rt)
48+
require.Nil(t, rtfunc)
4649
})
4750
}
4851

extension/extensionmiddleware/extensionmiddlewaretest/err.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package extensionmiddlewaretest // import "go.opentelemetry.io/collector/extensi
55

66
import (
77
"context"
8-
"net/http"
98

109
"google.golang.org/grpc"
1110

@@ -35,13 +34,13 @@ type baseExtension struct {
3534
// extensionmiddleware interface and always returns an error.
3635
func NewErr(err error) extension.Extension {
3736
return &baseExtension{
38-
GetHTTPRoundTripperFunc: func(http.RoundTripper) (http.RoundTripper, error) {
37+
GetHTTPRoundTripperFunc: func(context.Context) (extensionmiddleware.WrapHTTPRoundTripperFunc, error) {
3938
return nil, err
4039
},
4140
GetGRPCClientOptionsFunc: func(context.Context) ([]grpc.DialOption, error) {
4241
return nil, err
4342
},
44-
GetHTTPHandlerFunc: func(http.Handler) (http.Handler, error) {
43+
GetHTTPHandlerFunc: func(context.Context) (extensionmiddleware.WrapHTTPHandlerFunc, error) {
4544
return nil, err
4645
},
4746
GetGRPCServerOptionsFunc: func(context.Context) ([]grpc.ServerOption, error) {

extension/extensionmiddleware/extensionmiddlewaretest/err_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func TestErrClient(t *testing.T) {
1818

1919
httpClient, ok := client.(extensionmiddleware.HTTPClient)
2020
require.True(t, ok)
21-
_, err := httpClient.GetHTTPRoundTripper(nil)
21+
_, err := httpClient.GetHTTPRoundTripper(context.Background())
2222
require.Error(t, err)
2323

2424
grpcClient, ok := client.(extensionmiddleware.GRPCClient)
@@ -29,10 +29,11 @@ func TestErrClient(t *testing.T) {
2929

3030
func TestErrServer(t *testing.T) {
3131
server := NewErr(errors.New("error"))
32+
testctx := context.Background()
3233

3334
httpServer, ok := server.(extensionmiddleware.HTTPServer)
3435
require.True(t, ok)
35-
_, err := httpServer.GetHTTPHandler(nil)
36+
_, err := httpServer.GetHTTPHandler(testctx)
3637
require.Error(t, err)
3738

3839
grpcServer, ok := server.(extensionmiddleware.GRPCServer)

0 commit comments

Comments
 (0)