Skip to content

Commit

Permalink
Merge branch 'main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
pantuza authored Dec 13, 2023
2 parents 8575703 + 071b983 commit c96c92c
Show file tree
Hide file tree
Showing 18 changed files with 199 additions and 62 deletions.
13 changes: 13 additions & 0 deletions .chloggen/addretrysetvalidation.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# 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. otlpreceiver)
component: "exporterhelper"

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Add RetrySettings validation function"

# One or more tracking issues or pull requests related to the change
issues: [9089]
29 changes: 29 additions & 0 deletions .chloggen/codeboten_make-otel-default.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# 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. otlpreceiver)
component: service

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Enable `telemetry.useOtelForInternalMetrics` by updating the flag to beta"

# One or more tracking issues or pull requests related to the change
issues: [7454]

# (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: |
The metrics generated should be consistent with the metrics generated
previously with OpenCensus. Users can disable the behaviour
by setting `--feature-gates -telemetry.useOtelForInternalMetrics` at
collector start.
# 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: []
10 changes: 5 additions & 5 deletions exporter/exporterhelper/obsexporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ type testParams struct {

func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt obsreporttest.TestTelemetry, useOtel bool)) {
t.Run("WithOC", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand All @@ -188,11 +193,6 @@ func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt
})

t.Run("WithOTel", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), true))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand Down
8 changes: 8 additions & 0 deletions exporter/exporterhelper/obsreport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/exporter"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/internal/obsreportconfig"
"go.opentelemetry.io/collector/obsreport/obsreporttest"
)

Expand All @@ -26,6 +28,12 @@ func TestExportEnqueueFailure(t *testing.T) {
})
require.NoError(t, err)

originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()

logRecords := int64(7)
obsrep.recordEnqueueFailureWithOC(context.Background(), component.DataTypeLogs, logRecords)
require.NoError(t, tt.CheckExporterEnqueueFailedLogs(logRecords))
Expand Down
5 changes: 2 additions & 3 deletions exporter/exporterhelper/queue_sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ func setFeatureGateForTest(t testing.TB, gate *featuregate.Gate, enabled bool) f
}

func TestQueuedRetry_QueueMetricsReported(t *testing.T) {
resetFlag := setFeatureGateForTest(t, obsreportconfig.UseOtelForInternalMetricsfeatureGate, false)
defer resetFlag()
tt, err := obsreporttest.SetupTelemetry(defaultID)
require.NoError(t, err)

Expand All @@ -166,9 +168,6 @@ func TestQueuedRetry_QueueMetricsReported(t *testing.T) {
}

func TestQueuedRetry_QueueMetricsReportedUsingOTel(t *testing.T) {
resetFlag := setFeatureGateForTest(t, obsreportconfig.UseOtelForInternalMetricsfeatureGate, true)
defer resetFlag()

tt, err := obsreporttest.SetupTelemetry(defaultID)
require.NoError(t, err)

Expand Down
22 changes: 22 additions & 0 deletions exporter/exporterhelper/retry_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,28 @@ type RetrySettings struct {
MaxElapsedTime time.Duration `mapstructure:"max_elapsed_time"`
}

func (cfg *RetrySettings) Validate() error {
if !cfg.Enabled {
return nil
}
if cfg.InitialInterval < 0 {
return errors.New("'initial_interval' must be non-negative")
}
if cfg.RandomizationFactor < 0 || cfg.RandomizationFactor > 1 {
return errors.New("'randomization_factor' must be within [0, 1]")
}
if cfg.Multiplier <= 0 {
return errors.New("'multiplier' must be positive")
}
if cfg.MaxInterval < 0 {
return errors.New("'max_interval' must be non-negative")
}
if cfg.MaxElapsedTime < 0 {
return errors.New("'max_elapsed' time must be non-negative")
}
return nil
}

// NewDefaultRetrySettings returns the default settings for RetrySettings.
func NewDefaultRetrySettings() RetrySettings {
return RetrySettings{
Expand Down
63 changes: 63 additions & 0 deletions exporter/exporterhelper/retry_sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,3 +404,66 @@ func tagsMatchLabelKeys(tags []tag.Tag, keys []metricdata.LabelKey, labels []met
}
return true
}

func TestNewDefaultRetrySettings(t *testing.T) {
cfg := NewDefaultRetrySettings()
assert.NoError(t, cfg.Validate())
assert.Equal(t,
RetrySettings{
Enabled: true,
InitialInterval: 5 * time.Second,
RandomizationFactor: 0.5,
Multiplier: 1.5,
MaxInterval: 30 * time.Second,
MaxElapsedTime: 5 * time.Minute,
}, cfg)
}

func TestInvalidInitialInterval(t *testing.T) {
cfg := NewDefaultRetrySettings()
assert.NoError(t, cfg.Validate())
cfg.InitialInterval = -1
assert.Error(t, cfg.Validate())
}

func TestInvalidRandomizationFactor(t *testing.T) {
cfg := NewDefaultRetrySettings()
assert.NoError(t, cfg.Validate())
cfg.RandomizationFactor = -1
assert.Error(t, cfg.Validate())
cfg.RandomizationFactor = 2
assert.Error(t, cfg.Validate())
}

func TestInvalidMultiplier(t *testing.T) {
cfg := NewDefaultRetrySettings()
assert.NoError(t, cfg.Validate())
cfg.Multiplier = 0
assert.Error(t, cfg.Validate())
}

func TestInvalidMaxInterval(t *testing.T) {
cfg := NewDefaultRetrySettings()
assert.NoError(t, cfg.Validate())
cfg.MaxInterval = -1
assert.Error(t, cfg.Validate())
}

func TestInvalidMaxElapsedTime(t *testing.T) {
cfg := NewDefaultRetrySettings()
assert.NoError(t, cfg.Validate())
cfg.MaxElapsedTime = -1
assert.Error(t, cfg.Validate())
}

func TestDisabledWithInvalidValues(t *testing.T) {
cfg := RetrySettings{
Enabled: false,
InitialInterval: -1,
RandomizationFactor: -1,
Multiplier: 0,
MaxInterval: -1,
MaxElapsedTime: -1,
}
assert.NoError(t, cfg.Validate())
}
2 changes: 1 addition & 1 deletion internal/obsreportconfig/obsreportconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
// telemetrySettings for internal metrics.
var UseOtelForInternalMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister(
"telemetry.useOtelForInternalMetrics",
featuregate.StageAlpha,
featuregate.StageBeta,
featuregate.WithRegisterDescription("controls whether the collector uses OpenTelemetry for internal metrics"))

// DisableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable
Expand Down
7 changes: 7 additions & 0 deletions internal/obsreportconfig/obsreportconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/config/configtelemetry"
"go.opentelemetry.io/collector/featuregate"
)

func TestConfigure(t *testing.T) {
originalValue := UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tests := []struct {
name string
level configtelemetry.Level
Expand Down
2 changes: 1 addition & 1 deletion internal/tools/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
go.opentelemetry.io/build-tools/multimod v0.12.0
go.opentelemetry.io/build-tools/semconvgen v0.12.0
golang.org/x/exp v0.0.0-20230905200255-921286631fa9
golang.org/x/tools v0.16.0
golang.org/x/tools v0.16.1
golang.org/x/vuln v1.0.1
)

Expand Down
4 changes: 2 additions & 2 deletions internal/tools/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 13 additions & 3 deletions processor/memorylimiterprocessor/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,21 @@
package memorylimiterprocessor // import "go.opentelemetry.io/collector/processor/memorylimiterprocessor"

import (
"errors"
"time"

"go.opentelemetry.io/collector/component"
)

var (
errCheckIntervalOutOfRange = errors.New("'check_interval' must be greater than zero")
errLimitOutOfRange = errors.New("'limit_mib' or 'limit_percentage' must be greater than zero")
errSpikeLimitOutOfRange = errors.New("'spike_limit_mib' must be smaller than 'limit_mib'")
errSpikeLimitPercentageOutOfRange = errors.New("'spike_limit_percentage' must be smaller than 'limit_percentage'")
errLimitPercentageOutOfRange = errors.New(
"'limit_percentage' and 'spike_limit_percentage' must be greater than zero and less than or equal to hundred")
)

// Config defines configuration for memory memoryLimiter processor.
type Config struct {
// CheckInterval is the time between measurements of memory usage for the
Expand Down Expand Up @@ -46,13 +56,13 @@ func (cfg *Config) Validate() error {
return errLimitOutOfRange
}
if cfg.MemoryLimitPercentage > 100 || cfg.MemorySpikePercentage > 100 {
return errPercentageLimitOutOfRange
return errLimitPercentageOutOfRange
}
if cfg.MemoryLimitMiB > 0 && cfg.MemoryLimitMiB <= cfg.MemorySpikeLimitMiB {
return errMemSpikeLimitOutOfRange
return errSpikeLimitOutOfRange
}
if cfg.MemoryLimitPercentage > 0 && cfg.MemoryLimitPercentage <= cfg.MemorySpikePercentage {
return errMemSpikePercentageLimitOutOfRange
return errSpikeLimitPercentageOutOfRange
}
return nil
}
6 changes: 3 additions & 3 deletions processor/memorylimiterprocessor/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ func TestConfigValidate(t *testing.T) {
MemoryLimitMiB: 10,
MemorySpikeLimitMiB: 10,
},
err: errMemSpikeLimitOutOfRange,
err: errSpikeLimitOutOfRange,
},
{
name: "invalid memory percentage limit",
cfg: &Config{
CheckInterval: 1 * time.Second,
MemoryLimitPercentage: 101,
},
err: errPercentageLimitOutOfRange,
err: errLimitPercentageOutOfRange,
},
{
name: "invalid memory spike percentage limit",
Expand All @@ -94,7 +94,7 @@ func TestConfigValidate(t *testing.T) {
MemoryLimitPercentage: 50,
MemorySpikePercentage: 60,
},
err: errMemSpikePercentageLimitOutOfRange,
err: errSpikeLimitPercentageOutOfRange,
},
}
for _, tt := range tests {
Expand Down
13 changes: 0 additions & 13 deletions processor/memorylimiterprocessor/memorylimiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,6 @@ var (
// that data is being refused due to high memory usage.
errDataRefused = errors.New("data refused due to high memory usage")

// Construction errors

errCheckIntervalOutOfRange = errors.New("check_interval must be greater than zero")

errLimitOutOfRange = errors.New("limit_mib or limit_percentage must be greater than zero")

errMemSpikeLimitOutOfRange = errors.New("spike_limit_mib must be smaller than limit_mib")

errMemSpikePercentageLimitOutOfRange = errors.New("spike_limit_percentage must be smaller than limit_percentage")

errPercentageLimitOutOfRange = errors.New(
"limit_percentage and spike_limit_percentage must be greater than zero and less than or equal to hundred")

errShutdownNotStarted = errors.New("no existing monitoring routine is running")
)

Expand Down
10 changes: 5 additions & 5 deletions processor/processorhelper/obsreport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ func TestProcessorLogRecords(t *testing.T) {

func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt obsreporttest.TestTelemetry, useOtel bool)) {
t.Run("WithOC", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand All @@ -109,11 +114,6 @@ func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt
})

t.Run("WithOTel", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), true))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand Down
Loading

0 comments on commit c96c92c

Please sign in to comment.