Skip to content

Commit

Permalink
[bugfix] temporary fix around grpclogger (#5272)
Browse files Browse the repository at this point in the history
* [bugfix] temporary fix around grpclogger

Calling SetLoggerV2 isn't thread safe and should only be called before
any grpc functions. See more details: https://pkg.go.dev/github.com/grpc-ecosystem/[email protected]/logging/settable#pkg-overview

* add SkipSettingGRPCLogger flag

This allows the InProcessCollector to prevent entering a race condition.
  • Loading branch information
Alex Boten authored Apr 28, 2022
1 parent 150c1ed commit b34df3a
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
5 changes: 4 additions & 1 deletion service/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,14 @@ func (col *Collector) setupConfigurationComponents(ctx context.Context) error {
if err != nil {
return fmt.Errorf("failed to get config: %w", err)
}

if col.logger, err = telemetrylogs.NewLogger(cfg.Service.Telemetry.Logs, col.set.LoggingOptions); err != nil {
return fmt.Errorf("failed to get logger: %w", err)
}

telemetrylogs.SetColGRPCLogger(col.logger, cfg.Service.Telemetry.Logs.Level)
if !col.set.SkipSettingGRPCLogger {
telemetrylogs.SetColGRPCLogger(col.logger, cfg.Service.Telemetry.Logs.Level)
}

col.service, err = newService(&svcSettings{
BuildInfo: col.set.BuildInfo,
Expand Down
3 changes: 3 additions & 0 deletions service/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ type CollectorSettings struct {
// LoggingOptions provides a way to change behavior of zap logging.
LoggingOptions []zap.Option

// SkipSettingGRPCLogger avoids setting the grpc logger
SkipSettingGRPCLogger bool

// For testing purpose only.
telemetry collectorTelemetryExporter
}

0 comments on commit b34df3a

Please sign in to comment.