Skip to content

Commit 6e13d00

Browse files
authored
feat: Add optional HTTP Middleware function to StartSettings for serverimpl (#263)
Problem --------------------- Traces for HTTP requests to the opamp-go server break, see #253 Solution --------------------- - Add an HTTP Handler middleware function to `StartSettings` - If this function is configured, apply it in serverimpl's `Start` where the HTTP Handler is set - (add unit tests) Code review notes --------------------- - This is a step in addressing #253 but mostly just for HTTP clients and requests. There is likely more to do for maintaining trace linkage through requests that come over websocket connections - I figured if users are using `Attach` instead of `Start`, they might have their own middleware configured for their HTTP server, so it makes more sense to hook this into `StartSettings` and the `Start` function
1 parent d1d6081 commit 6e13d00

File tree

6 files changed

+162
-2
lines changed

6 files changed

+162
-2
lines changed

internal/examples/go.mod

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ require (
99
github.com/open-telemetry/opamp-go v0.1.0
1010
github.com/shirou/gopsutil v3.21.11+incompatible
1111
github.com/stretchr/testify v1.8.4
12+
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0
1213
go.opentelemetry.io/otel v1.24.0
1314
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.24.0
1415
go.opentelemetry.io/otel/metric v1.24.0
@@ -19,12 +20,13 @@ require (
1920

2021
require (
2122
github.com/davecgh/go-spew v1.1.1 // indirect
23+
github.com/felixge/httpsnoop v1.0.4 // indirect
2224
github.com/fsnotify/fsnotify v1.4.9 // indirect
2325
github.com/go-logr/logr v1.4.1 // indirect
2426
github.com/go-logr/stdr v1.2.2 // indirect
2527
github.com/go-ole/go-ole v1.2.6 // indirect
26-
github.com/gorilla/websocket v1.5.1 // indirect
2728
github.com/golang/protobuf v1.5.3 // indirect
29+
github.com/gorilla/websocket v1.5.1 // indirect
2830
github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 // indirect
2931
github.com/mitchellh/copystructure v1.2.0 // indirect
3032
github.com/mitchellh/mapstructure v1.4.1 // indirect

internal/examples/go.sum

+4
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
2121
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
2222
github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
2323
github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M=
24+
github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg=
25+
github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
2426
github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4=
2527
github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
2628
github.com/go-ldap/ldap v3.0.2+incompatible/go.mod h1:qfd9rJvER9Q0/D/Sqn1DfHRoBp40uXYvFoEVrNEPqRc=
@@ -127,6 +129,8 @@ github.com/tklauser/numcpus v0.3.0 h1:ILuRUQBtssgnxw0XXIjKUC56fgnOrFoQQ/4+DeU2bi
127129
github.com/tklauser/numcpus v0.3.0/go.mod h1:yFGUr7TUHQRAhyqBcEg0Ge34zDBAsIvJJcyE6boqnA8=
128130
github.com/yusufpapurcu/wmi v1.2.2 h1:KBNDSne4vP5mbSWnJbO+51IMOXJB67QiYCSBrubbPRg=
129131
github.com/yusufpapurcu/wmi v1.2.2/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0=
132+
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 h1:jq9TW8u3so/bN+JPT166wjOI6/vQPF6Xe7nMNIltagk=
133+
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0/go.mod h1:p8pYQP+m5XfbZm9fxtSKAbM6oIllS7s2AfxrChvc7iw=
130134
go.opentelemetry.io/otel v1.24.0 h1:0LAOdjNmQeSTzGBzduGe/rU4tZhMwL5rWgtp9Ku5Jfo=
131135
go.opentelemetry.io/otel v1.24.0/go.mod h1:W7b9Ozg4nkF5tWI5zsXkaKKDjdVjpD4oAt9Qi/MArHo=
132136
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.24.0 h1:mM8nKi6/iFQ0iqst80wDHU2ge198Ye/TfN0WBS5U24Y=

internal/examples/server/opampsrv/opampsrv.go

+3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"net/http"
77
"os"
88

9+
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
10+
911
"github.com/open-telemetry/opamp-go/internal"
1012
"github.com/open-telemetry/opamp-go/internal/examples/server/data"
1113
"github.com/open-telemetry/opamp-go/protobufs"
@@ -54,6 +56,7 @@ func (srv *Server) Start() {
5456
},
5557
},
5658
ListenEndpoint: "127.0.0.1:4320",
59+
HTTPMiddleware: otelhttp.NewMiddleware("/v1/opamp"),
5760
}
5861
tlsConfig, err := internal.CreateServerTLSConfig(
5962
"../../certs/certs/ca.cert.pem",

server/server.go

+5
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ type StartSettings struct {
4242

4343
// Server's TLS configuration.
4444
TLSConfig *tls.Config
45+
46+
// HTTPMiddleware specifies middleware for HTTP messages received by the server.
47+
// Note that the function will be called once for websockets upon connecting and will
48+
// be called for every HTTP request. This function is optional to set.
49+
HTTPMiddleware func(handler http.Handler) http.Handler
4550
}
4651

4752
type HTTPHandlerFunc func(http.ResponseWriter, *http.Request)

server/serverimpl.go

+17-1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,16 @@ type server struct {
4747

4848
var _ OpAMPServer = (*server)(nil)
4949

50+
// innerHTTPHandler implements the http.Handler interface so it can be used by functions
51+
// that require the type (like Middleware) without exposing ServeHTTP directly on server.
52+
type innerHTTPHander struct {
53+
httpHandlerFunc http.HandlerFunc
54+
}
55+
56+
func (i innerHTTPHander) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
57+
i.httpHandlerFunc(writer, request)
58+
}
59+
5060
// New creates a new OpAMP Server.
5161
func New(logger types.Logger) *server {
5262
if logger == nil {
@@ -82,7 +92,13 @@ func (s *server) Start(settings StartSettings) error {
8292
path = defaultOpAMPPath
8393
}
8494

85-
mux.HandleFunc(path, s.httpHandler)
95+
handler := innerHTTPHander{s.httpHandler}
96+
97+
if settings.HTTPMiddleware != nil {
98+
mux.Handle(path, settings.HTTPMiddleware(handler))
99+
} else {
100+
mux.Handle(path, handler)
101+
}
86102

87103
hs := &http.Server{
88104
Handler: mux,

server/serverimpl_test.go

+130
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,33 @@ func TestServerStartStop(t *testing.T) {
5757
assert.NoError(t, err)
5858
}
5959

60+
func TestServerStartStopWithMiddleware(t *testing.T) {
61+
var addedMiddleware atomic.Bool
62+
assert.False(t, addedMiddleware.Load())
63+
64+
testHTTPMiddleware := func(handler http.Handler) http.Handler {
65+
addedMiddleware.Store(true)
66+
return http.HandlerFunc(
67+
func(w http.ResponseWriter, r *http.Request) {
68+
handler.ServeHTTP(w, r)
69+
},
70+
)
71+
}
72+
73+
startSettings := &StartSettings{
74+
HTTPMiddleware: testHTTPMiddleware,
75+
}
76+
77+
srv := startServer(t, startSettings)
78+
assert.True(t, addedMiddleware.Load())
79+
80+
err := srv.Start(*startSettings)
81+
assert.ErrorIs(t, err, errAlreadyStarted)
82+
83+
err = srv.Stop(context.Background())
84+
assert.NoError(t, err)
85+
}
86+
6087
func TestServerAddrWithNonZeroPort(t *testing.T) {
6188
srv := New(&sharedinternal.NopLogger{})
6289
require.NotNil(t, srv)
@@ -830,6 +857,109 @@ func TestConnectionAllowsConcurrentWrites(t *testing.T) {
830857
}
831858
}
832859

860+
func TestServerCallsHTTPMiddlewareOverWebsocket(t *testing.T) {
861+
middlewareCalled := int32(0)
862+
863+
testHTTPMiddleware := func(handler http.Handler) http.Handler {
864+
return http.HandlerFunc(
865+
func(w http.ResponseWriter, r *http.Request) {
866+
atomic.AddInt32(&middlewareCalled, 1)
867+
handler.ServeHTTP(w, r)
868+
},
869+
)
870+
}
871+
872+
callbacks := CallbacksStruct{
873+
OnConnectingFunc: func(request *http.Request) types.ConnectionResponse {
874+
return types.ConnectionResponse{
875+
Accept: true,
876+
ConnectionCallbacks: ConnectionCallbacksStruct{},
877+
}
878+
},
879+
}
880+
881+
// Start a Server
882+
settings := &StartSettings{
883+
HTTPMiddleware: testHTTPMiddleware,
884+
Settings: Settings{Callbacks: callbacks},
885+
}
886+
srv := startServer(t, settings)
887+
defer func() {
888+
err := srv.Stop(context.Background())
889+
assert.NoError(t, err)
890+
}()
891+
892+
// Connect to the server, ensuring successful connection
893+
conn, resp, err := dialClient(settings)
894+
assert.NoError(t, err)
895+
assert.NotNil(t, conn)
896+
require.NotNil(t, resp)
897+
assert.EqualValues(t, 101, resp.StatusCode)
898+
899+
// Verify middleware was called once for the websocket connection
900+
eventually(t, func() bool { return atomic.LoadInt32(&middlewareCalled) == int32(1) })
901+
assert.Equal(t, int32(1), atomic.LoadInt32(&middlewareCalled))
902+
}
903+
904+
func TestServerCallsHTTPMiddlewareOverHTTP(t *testing.T) {
905+
middlewareCalled := int32(0)
906+
907+
testHTTPMiddleware := func(handler http.Handler) http.Handler {
908+
return http.HandlerFunc(
909+
func(w http.ResponseWriter, r *http.Request) {
910+
atomic.AddInt32(&middlewareCalled, 1)
911+
handler.ServeHTTP(w, r)
912+
},
913+
)
914+
}
915+
916+
callbacks := CallbacksStruct{
917+
OnConnectingFunc: func(request *http.Request) types.ConnectionResponse {
918+
return types.ConnectionResponse{
919+
Accept: true,
920+
ConnectionCallbacks: ConnectionCallbacksStruct{},
921+
}
922+
},
923+
}
924+
925+
// Start a Server
926+
settings := &StartSettings{
927+
HTTPMiddleware: testHTTPMiddleware,
928+
Settings: Settings{Callbacks: callbacks},
929+
}
930+
srv := startServer(t, settings)
931+
defer func() {
932+
err := srv.Stop(context.Background())
933+
assert.NoError(t, err)
934+
}()
935+
936+
// Send an AgentToServer message to the Server
937+
sendMsg1 := protobufs.AgentToServer{InstanceUid: "01BX5ZZKBKACTAV9WEVGEMMVS1"}
938+
serializedProtoBytes1, err := proto.Marshal(&sendMsg1)
939+
require.NoError(t, err)
940+
_, err = http.Post(
941+
"http://"+settings.ListenEndpoint+settings.ListenPath,
942+
contentTypeProtobuf,
943+
bytes.NewReader(serializedProtoBytes1),
944+
)
945+
require.NoError(t, err)
946+
947+
// Send another AgentToServer message to the Server
948+
sendMsg2 := protobufs.AgentToServer{InstanceUid: "01BX5ZZKBKACTAV9WEVGEMMVRZ"}
949+
serializedProtoBytes2, err := proto.Marshal(&sendMsg2)
950+
require.NoError(t, err)
951+
_, err = http.Post(
952+
"http://"+settings.ListenEndpoint+settings.ListenPath,
953+
contentTypeProtobuf,
954+
bytes.NewReader(serializedProtoBytes2),
955+
)
956+
require.NoError(t, err)
957+
958+
// Verify middleware was triggered for each HTTP call
959+
eventually(t, func() bool { return atomic.LoadInt32(&middlewareCalled) == int32(2) })
960+
assert.Equal(t, int32(2), atomic.LoadInt32(&middlewareCalled))
961+
}
962+
833963
func BenchmarkSendToClient(b *testing.B) {
834964
clientConnections := []*websocket.Conn{}
835965
serverConnections := []types.Connection{}

0 commit comments

Comments
 (0)