From f6156a91004559934f8e7550167c9142bb858b99 Mon Sep 17 00:00:00 2001 From: Matej Gera <38492574+matej-g@users.noreply.github.com> Date: Tue, 3 Sep 2024 22:39:50 +0200 Subject: [PATCH] [extension/opamp] Add support for polling interval in HTTP client (#34811) **Description:** Adds support for configuring polling interval for the OpAMP HTTP client. **Link to tracking Issue:** #34749 **Testing:** Adjusted unit test, manually tested **Documentation:** Added to README --------- Signed-off-by: Matej Gera Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- ...mpextension_add_http_polling_interval.yaml | 27 +++++++++ extension/opampextension/README.md | 13 ++++- extension/opampextension/config.go | 47 +++++++++++++--- extension/opampextension/config_test.go | 56 ++++++++++++++----- .../opampextension/testdata/config_http.yaml | 1 + 5 files changed, 121 insertions(+), 23 deletions(-) create mode 100644 .chloggen/opampextension_add_http_polling_interval.yaml diff --git a/.chloggen/opampextension_add_http_polling_interval.yaml b/.chloggen/opampextension_add_http_polling_interval.yaml new file mode 100644 index 000000000000..c1549b8c4ed3 --- /dev/null +++ b/.chloggen/opampextension_add_http_polling_interval.yaml @@ -0,0 +1,27 @@ +# 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. filelogreceiver) +component: opampextension + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Adds the ability to configure the polling interval for the HTTP client. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [34749] + +# (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: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# 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. +# Default: '[user]' +change_logs: [] diff --git a/extension/opampextension/README.md b/extension/opampextension/README.md index 920d60ab91d3..b12f1869d949 100644 --- a/extension/opampextension/README.md +++ b/extension/opampextension/README.md @@ -19,12 +19,23 @@ The following settings are required: - `ws`: The OpAMP websocket transport settings. - `endpoint` (no default): The OpAMP server websocket endpoint (URL). -The following settings are optional: +The following settings are optional for the websocket client: - `server`: The OpAMP server connection settings. - `ws`: The OpAMP websocket transport settings. - `tls`: TLS settings. - `headers`: HTTP headers to set. + +The following settings are optional for the HTTP client: + +- `server`: The OpAMP server connection settings. + - `http`: The OpAMP websocket transport settings. + - `tls`: TLS settings. + - `headers`: HTTP headers to set. + - `polling_interval`: The interval at which the extension will poll the server. Defaults to 30s. + +The following settings are optional for both transports: + - `instance_uid`: A UUIDv7 formatted as a 36 character string in canonical representation. Auto-generated on start if missing. Setting this ensures the instance UID remains constant across process restarts. diff --git a/extension/opampextension/config.go b/extension/opampextension/config.go index be16b8e94f81..202bfdbb6d8a 100644 --- a/extension/opampextension/config.go +++ b/extension/opampextension/config.go @@ -15,6 +15,10 @@ import ( "go.uber.org/zap" ) +// Default value for HTTP client's polling interval, set to 30 seconds in +// accordance with the OpAMP spec. +const httpPollingIntervalDefault = 30 * time.Second + // Config contains the configuration for the opamp extension. Trying to mirror // the OpAMP supervisor config for some consistency. type Config struct { @@ -68,12 +72,6 @@ type commonFields struct { Headers map[string]configopaque.String `mapstructure:"headers,omitempty"` } -// OpAMPServer contains the OpAMP transport configuration. -type OpAMPServer struct { - WS *commonFields `mapstructure:"ws,omitempty"` - HTTP *commonFields `mapstructure:"http,omitempty"` -} - func (c *commonFields) Scheme() string { uri, err := url.ParseRequestURI(c.Endpoint) if err != nil { @@ -89,11 +87,38 @@ func (c *commonFields) Validate() error { return nil } +type httpFields struct { + commonFields `mapstructure:",squash"` + + PollingInterval time.Duration `mapstructure:"polling_interval"` +} + +func (h *httpFields) Validate() error { + if err := h.commonFields.Validate(); err != nil { + return err + } + + if h.PollingInterval < 0 { + return errors.New("polling interval must be 0 or greater") + } + + return nil +} + +// OpAMPServer contains the OpAMP transport configuration. +type OpAMPServer struct { + WS *commonFields `mapstructure:"ws,omitempty"` + HTTP *httpFields `mapstructure:"http,omitempty"` +} + func (s OpAMPServer) GetClient(logger *zap.Logger) client.OpAMPClient { if s.WS != nil { return client.NewWebSocket(newLoggerFromZap(logger.With(zap.String("client", "ws")))) } - return client.NewHTTP(newLoggerFromZap(logger.With(zap.String("client", "http")))) + + httpClient := client.NewHTTP(newLoggerFromZap(logger.With(zap.String("client", "http")))) + httpClient.SetPollingInterval(s.GetPollingInterval()) + return httpClient } func (s OpAMPServer) GetHeaders() map[string]configopaque.String { @@ -123,6 +148,14 @@ func (s OpAMPServer) GetEndpoint() string { return "" } +func (s OpAMPServer) GetPollingInterval() time.Duration { + if s.HTTP != nil && s.HTTP.PollingInterval > 0 { + return s.HTTP.PollingInterval + } + + return httpPollingIntervalDefault +} + // Validate checks if the extension configuration is valid func (cfg *Config) Validate() error { switch { diff --git a/extension/opampextension/config_test.go b/extension/opampextension/config_test.go index 7c996f53b7e8..bbccff4d91aa 100644 --- a/extension/opampextension/config_test.go +++ b/extension/opampextension/config_test.go @@ -53,8 +53,11 @@ func TestUnmarshalHttpConfig(t *testing.T) { assert.Equal(t, &Config{ Server: &OpAMPServer{ - HTTP: &commonFields{ - Endpoint: "https://127.0.0.1:4320/v1/opamp", + HTTP: &httpFields{ + commonFields: commonFields{ + Endpoint: "https://127.0.0.1:4320/v1/opamp", + }, + PollingInterval: 1 * time.Minute, }, }, InstanceUID: "01BX5ZZKBKACTAV9WEVGEMMVRZ", @@ -115,13 +118,15 @@ func TestConfig_Getters(t *testing.T) { name: "HTTP valid endpoint and valid instance id", fields: fields{ Server: &OpAMPServer{ - HTTP: &commonFields{ - Endpoint: "https://127.0.0.1:4320/v1/opamp", - Headers: map[string]configopaque.String{ - "test": configopaque.String("test"), - }, - TLSSetting: configtls.ClientConfig{ - Insecure: true, + HTTP: &httpFields{ + commonFields: commonFields{ + Endpoint: "https://127.0.0.1:4320/v1/opamp", + Headers: map[string]configopaque.String{ + "test": configopaque.String("test"), + }, + TLSSetting: configtls.ClientConfig{ + Insecure: true, + }, }, }, }, @@ -194,7 +199,7 @@ func TestConfig_Validate(t *testing.T) { name: "HTTP must have endpoint", fields: fields{ Server: &OpAMPServer{ - HTTP: &commonFields{}, + HTTP: &httpFields{}, }, }, wantErr: func(t assert.TestingT, err error, _ ...any) bool { @@ -205,8 +210,10 @@ func TestConfig_Validate(t *testing.T) { name: "HTTP valid endpoint and invalid instance id", fields: fields{ Server: &OpAMPServer{ - HTTP: &commonFields{ - Endpoint: "https://127.0.0.1:4320/v1/opamp", + HTTP: &httpFields{ + commonFields: commonFields{ + Endpoint: "https://127.0.0.1:4320/v1/opamp", + }, }, }, InstanceUID: "01BX5ZZKBKACTAV9WEVGEMMVRZFAIL", @@ -219,14 +226,33 @@ func TestConfig_Validate(t *testing.T) { name: "HTTP valid endpoint and valid instance id", fields: fields{ Server: &OpAMPServer{ - HTTP: &commonFields{ - Endpoint: "https://127.0.0.1:4320/v1/opamp", + HTTP: &httpFields{ + commonFields: commonFields{ + Endpoint: "https://127.0.0.1:4320/v1/opamp", + }, }, }, InstanceUID: "01BX5ZZKBKACTAV9WEVGEMMVRZ", }, wantErr: assert.NoError, }, + { + name: "HTTP invalid polling interval", + fields: fields{ + Server: &OpAMPServer{ + HTTP: &httpFields{ + commonFields: commonFields{ + Endpoint: "https://127.0.0.1:4320/v1/opamp", + }, + PollingInterval: -1, + }, + }, + InstanceUID: "01BX5ZZKBKACTAV9WEVGEMMVRZ", + }, + wantErr: func(t assert.TestingT, err error, _ ...any) bool { + return assert.Equal(t, "polling interval must be 0 or greater", err.Error()) + }, + }, { name: "neither config set", fields: fields{ @@ -241,7 +267,7 @@ func TestConfig_Validate(t *testing.T) { fields: fields{ Server: &OpAMPServer{ WS: &commonFields{}, - HTTP: &commonFields{}, + HTTP: &httpFields{}, }, }, wantErr: func(t assert.TestingT, err error, _ ...any) bool { diff --git a/extension/opampextension/testdata/config_http.yaml b/extension/opampextension/testdata/config_http.yaml index c97da589a392..cfbfffddef30 100644 --- a/extension/opampextension/testdata/config_http.yaml +++ b/extension/opampextension/testdata/config_http.yaml @@ -1,4 +1,5 @@ server: http: endpoint: https://127.0.0.1:4320/v1/opamp + polling_interval: 1m instance_uid: 01BX5ZZKBKACTAV9WEVGEMMVRZ