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

Exporter: WithLogger uses the github.com/go-logr/logr interface to replace the standard library logger #3497

Closed
aniaan opened this issue Nov 29, 2022 · 2 comments · Fixed by #3500
Labels
enhancement New feature or request pkg:exporter:zipkin Related to the Zipkin exporter package

Comments

@aniaan
Copy link
Contributor

aniaan commented Nov 29, 2022

Problem Statement

Currently otel api and sdk use globalLogger to print logs. globalLogger uses the github.com/go-logr/logr library, which provides the interface, and the specific log implementation can be determined by the user, which is designed to be flexible.

var globalLogger logr.Logger = stdr.New(log.New(os.Stderr, "", log.LstdFlags|log.Lshortfile))
var globalLoggerLock = &sync.RWMutex{}
// SetLogger overrides the globalLogger with l.
//
// To see Info messages use a logger with `l.V(1).Enabled() == true`
// To see Debug messages use a logger with `l.V(5).Enabled() == true`.
func SetLogger(l logr.Logger) {
globalLoggerLock.Lock()
defer globalLoggerLock.Unlock()
globalLogger = l
}

But the Exporter uses golang's standard log library, so the printing of logs in a project will be fragmented, which is not conducive to unified log processing and collection.

// WithLogger configures the exporter to use the passed logger.
func WithLogger(logger *log.Logger) Option {
return optionFunc(func(cfg config) config {
cfg.logger = logger
return cfg
})
}

I have a project where my logging library of choice is zap. When I use the otel api and sdk, I can use github.com/go-logr/zapr to wrap my logger and set the globalLogger, but when I use Exporter, I can't do anything about it, because I can only use golang's standard logging library.

Proposed Solution

Exporter uses the github.com/go-logr/logr library, which is consistent throughout the project

If the community agrees with the proposal, I can submit a PR to resolve the issue

@aniaan aniaan added the enhancement New feature or request label Nov 29, 2022
@MrAlias MrAlias added the pkg:exporter:zipkin Related to the Zipkin exporter package label Nov 29, 2022
@MrAlias
Copy link
Contributor

MrAlias commented Nov 29, 2022

The go.opentelemetry.io/otel/exporters/zipkin package is released as stable. That function signature cannot be changed in a backwards compatible way to the proposal.

@MadVikingGod
Copy link
Contributor

I would be open to this addition. We can't remove or change the current WithLogger, but we can add another option and use logr under the hood.

My only real concern is compatibility between logr and the proposed structured logging.

MrAlias added a commit to MrAlias/opentelemetry-go that referenced this issue Jan 28, 2023
MrAlias added a commit that referenced this issue Jan 29, 2023
* Update module versions

* Prepare stable-v1 for version v1.12.0

* Prepare experimental-metrics for version v0.35.0

* Prepare experimental-schema for version v0.0.4

* Update the CHANGELOG

* Undo bump to experimental-schema

Revert to original version as nothing has changed.

* Fix PR number in changelog

* Move change from #3497 into current release
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pkg:exporter:zipkin Related to the Zipkin exporter package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants