Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remotecfg: set lower poll_frequency limit #937

Merged
merged 7 commits into from
May 31, 2024
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ Main (unreleased)

- Allow override debug metrics level for `otelcol.*` components. (@hainenber)

- Add an initial lower limit of 10 seconds for the the `poll_frequency`
argument in the `remotecfg` block. (@tpaschalis)

- Added support for NS records to `discovery.dns`. (@djcode)

### Bugfixes
Expand Down
2 changes: 2 additions & 0 deletions docs/sources/reference/config-blocks/remotecfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ If not set, the self-reported `id` that {{< param "PRODUCT_NAME" >}} uses is a r
The `id` and `metadata` fields are used in the periodic request sent to the
remote endpoint so that the API can decide what configuration to serve.

The `poll_frequency` must be set to at least `"10s"`.

## Blocks

The following blocks are supported inside the definition of `remotecfg`:
Expand Down
4 changes: 4 additions & 0 deletions internal/service/remotecfg/remotecfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ func (a *Arguments) SetToDefault() {

// Validate implements syntax.Validator.
func (a *Arguments) Validate() error {
if a.PollFrequency < 10*time.Second {
return fmt.Errorf("poll_frequency must be at least \"10s\", got %q", a.PollFrequency)
}

// We must explicitly Validate because HTTPClientConfig is squashed and it
// won't run otherwise
if a.HTTPClientConfig != nil {
Expand Down
8 changes: 6 additions & 2 deletions internal/service/remotecfg/remotecfg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestAPIResponse(t *testing.T) {
env := newTestEnvironment(t)
require.NoError(t, env.ApplyConfig(fmt.Sprintf(`
url = "%s"
poll_frequency = "10ms"
poll_frequency = "10s"
`, url)))

client := &collectorClient{}
Expand Down Expand Up @@ -100,7 +100,7 @@ func TestAPIResponse(t *testing.T) {
// Verify that the service has loaded the updated response.
require.EventuallyWithT(t, func(c *assert.CollectT) {
assert.Equal(c, getHash([]byte(cfg2)), env.svc.getCfgHash())
}, time.Second, 10*time.Millisecond)
}, 1*time.Second, 10*time.Millisecond)
}

func buildGetConfigHandler(in string) func(context.Context, *connect.Request[collectorv1.GetConfigRequest]) (*connect.Response[collectorv1.GetConfigResponse], error) {
Expand Down Expand Up @@ -138,6 +138,10 @@ func (env *testEnvironment) ApplyConfig(config string) error {
if err := syntax.Unmarshal([]byte(config), &args); err != nil {
return err
}
// The lower limit of the poll_frequency argument would slow our tests
// considerably; let's artificially lower it after the initial validation
// has taken place.
args.PollFrequency /= 100
return env.svc.Update(args)
}

Expand Down
Loading