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

[telemetrySettting] Create sampled Logger #8134

Merged
merged 35 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
4394496
Capability to configure sampledLogger in the exporterHelper
antonjim-te Jul 26, 2023
3192d88
Capability to configure sampledLogger in the exporterHelper
antonjim-te Jul 26, 2023
39f95e4
Improve documentation
antonjim-te Jul 26, 2023
5295e11
Improve documentation
antonjim-te Jul 26, 2023
9534fbb
Apply PR feedback
antonjim-te Jul 27, 2023
56b71f3
Apply PR feedback
antonjim-te Jul 27, 2023
5aec726
Apply PR feedback
antonjim-te Jul 27, 2023
5d2cdeb
Merge main
antonjim-te Aug 21, 2023
7ab0227
Fix linter
antonjim-te Aug 28, 2023
9778c88
Add chloggen
antonjim-te Aug 28, 2023
b6ac5ac
Apply feedback. Describe sampled_logger config
antonjim-te Aug 28, 2023
6c2051a
Merge master
antonjim-te Sep 6, 2023
f53561a
Merge branch 'open-telemetry:main' into develop/sampledLoggedCfg
antonjim-te Sep 6, 2023
310b747
Merge master
antonjim-te Sep 6, 2023
9540c55
Create a extra sampled logger as part of the telemetry settings
antonjim-te Sep 6, 2023
20536a1
Review PR
antonjim-te Sep 6, 2023
0b60ef8
Apply feedback from @mx-psi . Create the sampledLogger the first time…
antonjim-te Sep 7, 2023
0add644
Add telemetry test
antonjim-te Sep 7, 2023
37fcef7
Update variable name
antonjim-te Sep 7, 2023
70f05d8
Update variable name
antonjim-te Sep 7, 2023
a5d054b
Merge branch 'main' into develop/sampledLoggedCfg
antonjim-te Sep 11, 2023
868041f
Fix merge commit
antonjim-te Sep 11, 2023
8a321ac
Apply @dmitryax feedback. Keep Sampling config in general logger
antonjim-te Sep 11, 2023
0cb309c
review PR
antonjim-te Sep 11, 2023
bc250fb
Review PR
antonjim-te Sep 12, 2023
7180926
Update default sampling logger configuration in the telemetry configu…
antonjim-te Sep 19, 2023
15b3b42
Merge branch 'main' into develop/sampledLoggedCfg
antonjim-te Sep 19, 2023
409f6ff
Fix merge
antonjim-te Sep 19, 2023
66fa58e
Fix linter
antonjim-te Sep 19, 2023
4b84a00
Apply @dmitryax feedback
antonjim-te Sep 22, 2023
fcd2b83
Apply @dmitryax feedback
antonjim-te Sep 22, 2023
ce3776a
Apply @dmitryax feedback
antonjim-te Sep 25, 2023
bce885d
apply @jpkrohling feedback
antonjim-te Sep 26, 2023
f9ca8a7
apply @jpkrohling feedback
antonjim-te Sep 26, 2023
8c3cddd
apply feedback
antonjim-te Sep 27, 2023
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
26 changes: 26 additions & 0 deletions .chloggen/SampledLoggerTelemetry.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# 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/telemetry exporter/exporterhelper

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Update default sampling logger configuration in the telemetry configuration."
antonjim-te marked this conversation as resolved.
Show resolved Hide resolved

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

# (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 sampled logger configuration can be disabled easily by setting the 'enabled' field to 'false'.
antonjim-te marked this conversation as resolved.
Show resolved Hide resolved


# 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: [user]
37 changes: 6 additions & 31 deletions exporter/exporterhelper/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ package exporterhelper // import "go.opentelemetry.io/collector/exporter/exporte

import (
"context"
"time"

"go.uber.org/zap"
"go.uber.org/zap/zapcore"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer"
Expand Down Expand Up @@ -102,7 +98,7 @@ func WithTimeout(timeoutSettings TimeoutSettings) Option {
// The default RetrySettings is to disable retries.
func WithRetry(retrySettings RetrySettings) Option {
return func(o *baseExporter) {
o.retrySender = newRetrySender(o.set.ID, retrySettings, o.sampledLogger, o.onTemporaryFailure)
o.retrySender = newRetrySender(o.set.ID, retrySettings, o.set.Logger, o.onTemporaryFailure)
}
}

Expand All @@ -122,7 +118,7 @@ func WithQueue(config QueueSettings) Option {
queue = internal.NewPersistentQueue(config.QueueSize, config.NumConsumers, *config.StorageID, o.marshaler, o.unmarshaler)
}
}
qs := newQueueSender(o.set.ID, o.signal, queue, o.sampledLogger)
qs := newQueueSender(o.set.ID, o.signal, queue, o.set.Logger)
o.queueSender = qs
o.setOnTemporaryFailure(qs.onTemporaryFailure)
}
Expand All @@ -147,9 +143,8 @@ type baseExporter struct {
unmarshaler internal.RequestUnmarshaler
signal component.DataType

set exporter.CreateSettings
obsrep *obsExporter
sampledLogger *zap.Logger
set exporter.CreateSettings
obsrep *obsExporter

// Chain of senders that the exporter helper applies before passing the data to the actual exporter.
// The data is handled by each sender in the respective order starting from the queueSender.
Expand Down Expand Up @@ -185,9 +180,8 @@ func newBaseExporter(set exporter.CreateSettings, signal component.DataType, req
retrySender: &baseRequestSender{},
timeoutSender: &timeoutSender{cfg: NewDefaultTimeoutSettings()},

set: set,
obsrep: obsrep,
sampledLogger: createSampledLogger(set.Logger),
set: set,
obsrep: obsrep,
}

for _, op := range options {
Expand Down Expand Up @@ -237,22 +231,3 @@ func (be *baseExporter) setOnTemporaryFailure(onTemporaryFailure onRequestHandli
rs.onTemporaryFailure = onTemporaryFailure
}
}

func createSampledLogger(logger *zap.Logger) *zap.Logger {
if logger.Core().Enabled(zapcore.DebugLevel) {
// Debugging is enabled. Don't do any sampling.
return logger
}

// Create a logger that samples all messages to 1 per 10 seconds initially,
// and 1/100 of messages after that.
opts := zap.WrapCore(func(core zapcore.Core) zapcore.Core {
return zapcore.NewSamplerWithOptions(
core,
10*time.Second,
1,
100,
)
})
return logger.WithOptions(opts)
}
6 changes: 5 additions & 1 deletion otelcol/unmarshaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package otelcol // import "go.opentelemetry.io/collector/otelcol"

import (
"time"

"go.uber.org/zap/zapcore"

"go.opentelemetry.io/collector/config/configtelemetry"
Expand Down Expand Up @@ -45,7 +47,9 @@ func unmarshal(v *confmap.Conf, factories Factories) (*configSettings, error) {
Development: false,
Encoding: "console",
Sampling: &telemetry.LogsSamplingConfig{
Initial: 100,
Enabled: true,
Tick: 1 * time.Second,
antonjim-te marked this conversation as resolved.
Show resolved Hide resolved
Initial: 10,
jpkrohling marked this conversation as resolved.
Show resolved Hide resolved
Thereafter: 100,
},
OutputPaths: []string{"stderr"},
Expand Down
5 changes: 4 additions & 1 deletion otelcol/unmarshaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package otelcol

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -45,7 +46,9 @@ func TestUnmarshalEmptyAllSections(t *testing.T) {
Development: zapProdCfg.Development,
Encoding: "console",
Sampling: &telemetry.LogsSamplingConfig{
Initial: 100,
Enabled: true,
Tick: 1 * time.Second,
Initial: 10,
Thereafter: 100,
},
DisableCaller: zapProdCfg.DisableCaller,
Expand Down
11 changes: 11 additions & 0 deletions service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,17 @@ func TestNilCollectorEffectiveConfig(t *testing.T) {
require.NoError(t, srv.Shutdown(context.Background()))
}

func TestServiceTelemetryLogger(t *testing.T) {
srv, err := New(context.Background(), newNopSettings(), newNopConfig())
require.NoError(t, err)

assert.NoError(t, srv.Start(context.Background()))
t.Cleanup(func() {
assert.NoError(t, srv.Shutdown(context.Background()))
})
assert.NotNil(t, srv.telemetrySettings.Logger)
}

func assertResourceLabels(t *testing.T, res pcommon.Resource, expectedLabels map[string]labelValue) {
for key, labelValue := range expectedLabels {
lookupKey, ok := prometheusToOtelConv[key]
Expand Down
16 changes: 13 additions & 3 deletions service/telemetry/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry"

import (
"fmt"
"time"

"go.uber.org/zap/zapcore"

Expand Down Expand Up @@ -54,7 +55,14 @@ type LogsConfig struct {
// (default = false)
DisableStacktrace bool `mapstructure:"disable_stacktrace"`

// Sampling sets a sampling policy. A nil SamplingConfig disables sampling.
// Sampling sets a sampling policy.
// Default:
// sampling:
// enabled: true
// tick: 1s
// initial: 10
// thereafter: 100
// Sampling can be disabled by setting 'enabled' to false
Sampling *LogsSamplingConfig `mapstructure:"sampling"`

// OutputPaths is a list of URLs or file paths to write logging output to.
Expand Down Expand Up @@ -91,8 +99,10 @@ type LogsConfig struct {
// global CPU and I/O load that logging puts on your process while attempting
// to preserve a representative subset of your logs.
type LogsSamplingConfig struct {
Initial int `mapstructure:"initial"`
Thereafter int `mapstructure:"thereafter"`
Enabled bool `mapstructure:"enabled"`
Tick time.Duration `mapstructure:"tick"`
Initial int `mapstructure:"initial"`
Thereafter int `mapstructure:"thereafter"`
antonjim-te marked this conversation as resolved.
Show resolved Hide resolved
}

// MetricsConfig exposes the common Telemetry configuration for one component.
Expand Down
24 changes: 15 additions & 9 deletions service/telemetry/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ func newLogger(cfg LogsConfig, options []zap.Option) (*zap.Logger, error) {
zapCfg := &zap.Config{
Level: zap.NewAtomicLevelAt(cfg.Level),
Development: cfg.Development,
Sampling: toSamplingConfig(cfg.Sampling),
antonjim-te marked this conversation as resolved.
Show resolved Hide resolved
Encoding: cfg.Encoding,
EncoderConfig: zap.NewProductionEncoderConfig(),
OutputPaths: cfg.OutputPaths,
Expand All @@ -78,16 +77,23 @@ func newLogger(cfg LogsConfig, options []zap.Option) (*zap.Logger, error) {
if err != nil {
return nil, err
}
if cfg.Sampling != nil && cfg.Sampling.Enabled {
logger = newSampledLogger(logger, cfg.Sampling)
}

return logger, nil
}

func toSamplingConfig(sc *LogsSamplingConfig) *zap.SamplingConfig {
if sc == nil {
return nil
}
return &zap.SamplingConfig{
Initial: sc.Initial,
Thereafter: sc.Thereafter,
}
func newSampledLogger(logger *zap.Logger, sc *LogsSamplingConfig) *zap.Logger {
// Create a logger that samples all messages to sc.Tick per second initially,
// and sc.Initial/sc.Thereafter of messages after that.
antonjim-te marked this conversation as resolved.
Show resolved Hide resolved
opts := zap.WrapCore(func(core zapcore.Core) zapcore.Core {
return zapcore.NewSamplerWithOptions(
core,
sc.Tick,
sc.Initial,
sc.Thereafter,
)
})
return logger.WithOptions(opts)
}
117 changes: 117 additions & 0 deletions service/telemetry/telemetry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package telemetry

import (
"context"
"testing"
"time"

"github.com/stretchr/testify/assert"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"

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

func TestTelemetryConfiguration(t *testing.T) {
dmitryax marked this conversation as resolved.
Show resolved Hide resolved
tests := []struct {
name string
cfg *Config
success bool
}{
{
name: "Valid config",
cfg: &Config{
Logs: LogsConfig{
Level: zapcore.DebugLevel,
Encoding: "console",
},
Metrics: MetricsConfig{
Level: configtelemetry.LevelBasic,
Address: "127.0.0.1:3333",
},
},
success: true,
},
{
name: "Invalid config",
cfg: &Config{
Logs: LogsConfig{
Level: zapcore.DebugLevel,
},
Metrics: MetricsConfig{
Level: configtelemetry.LevelBasic,
Address: "127.0.0.1:3333",
},
},
success: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
telemetry, err := New(context.Background(), Settings{ZapOptions: []zap.Option{}}, *tt.cfg)
if tt.success {
assert.NoError(t, err)
assert.NotNil(t, telemetry)
} else {
assert.Error(t, err)
assert.Nil(t, telemetry)
}
})
}
}

func TestSampledLogger(t *testing.T) {
tests := []struct {
name string
cfg *Config
}{
{
name: "Default sampling",
cfg: &Config{
Logs: LogsConfig{
Encoding: "console",
},
},
},
{
name: "Custom sampling",
cfg: &Config{
Logs: LogsConfig{
Level: zapcore.DebugLevel,
Encoding: "console",
Sampling: &LogsSamplingConfig{
Enabled: true,
Tick: 1 * time.Second,
Initial: 100,
Thereafter: 100,
},
},
},
},
{
name: "Disable sampling",
cfg: &Config{
Logs: LogsConfig{
Level: zapcore.DebugLevel,
Encoding: "console",
Sampling: &LogsSamplingConfig{
Enabled: false,
},
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
telemetry, err := New(context.Background(), Settings{ZapOptions: []zap.Option{}}, *tt.cfg)
assert.NoError(t, err)
assert.NotNil(t, telemetry)
assert.NotNil(t, telemetry.Logger())
})
}
}
Loading