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

ddtrace: make UseLogger public #1466

Merged
merged 4 commits into from
Sep 21, 2022
Merged

ddtrace: make UseLogger public #1466

merged 4 commits into from
Sep 21, 2022

Conversation

nsrip-dd
Copy link
Contributor

Make the internal UseLogger function public so that users can specify a
single logger for all tracer and profiler logs. Right now the profiler
doesn't have a configurable logger, and the tracer has the WithLogger
option, which sets the global logger anyway.

Fixes #1331.

Make the internal UseLogger function public so that users can specify a
single logger for all tracer and profiler logs. Right now the profiler
doesn't have a configurable logger, and the tracer has the WithLogger
option, which sets the global logger anyway.

Fixes #1331.
@nsrip-dd nsrip-dd requested a review from a team September 14, 2022 13:00
@nsrip-dd nsrip-dd added this to the 1.43.0 milestone Sep 14, 2022
@knusbaum knusbaum merged commit dd30c5c into main Sep 21, 2022
@knusbaum knusbaum deleted the feature-global-use-logger branch September 21, 2022 16:31
@KaibutsuX
Copy link

Is there a way to get the old Logger interface so it can be restored? The new public UseLogger function does not return anything compared to the internal's UseLogger.

Without having access to the original logger, I can't perform the original functionality without copy-pasting the code from internal

@nsrip-dd
Copy link
Contributor Author

I suppose we could consider adding a SwapLogger(new Logger) (old Logger) interface or something. I'm curious, what's your use case? We have the undo functionality internally for testing this library but I'm not sure how useful it would be outside of developing this library.

@KaibutsuX
Copy link

KaibutsuX commented Jul 12, 2023

I'm trying to silence the internal diagnostics like INFO: Profiler configuration because they are printed to stderr, they are interpreted as errors in the DD webui. So I have a custom logger that basically says:

if !ignoring {
   originalLoggger(msg)
} else {
   // do nothing
}

Basically, in addition to implementing my own logger, I also want the option to invoke the old one based on any logic I choose.

@nsrip-dd
Copy link
Contributor Author

I see, so what you're looking for is something like a GetLogger() Logger function, so you could do

orig := ddtrace.GetLogger()
ddtrace.UseLogger(Wrap(orig))

where Wrap wraps the original logger. I just want to clarify: would the original logger even be useful? If the original logger is problematic because it logs to stderr, and your logger delegates to the original one, you would still have that problem, right? Or are some logs okay to print to stderr? Either way, the logging implementation our library uses internally is somewhat arbitrary. It's just the standard library logger. Outside of testing, the destination of our log messages doesn't affect the library. Is there any benefit to having access to the original logger vs entirely using your own? Like:

if !ignoring {
    log.Println(msg) // or whichever logging library you use elsewhere in the program
}

@KaibutsuX
Copy link

KaibutsuX commented Jul 12, 2023

Correct, I want a ddtrace.GetLogger()

Just from a design standpoint, it seems unusual that UseLogger is a permanent one-way operation that I can never revert, unless I have the original before that.

Yeah I looked through (I got a bit lost in the separation between log and ddtrace/internal/log) and it looks like the internal log is actually directly opening FD2 (stderr) and writing the logs to it. Which means if my custom logger needed to emulate that behavior, I would also need to open 2 or just call the original handler, something like this is what I want to do:

type struct mylogger {
    original ddtrace.Logger
    ignoring bool
}
(l *mylogger) Log(m string) {
    if !l.ignoring {
       // Just do whatever DD does normally
       l.original.Log(m)    // what I want to do
       /* what i actually need to do to emulate what dd is currently doing
       fd := os.Open(2) // or whatever
       fd.Write(m)
       */
    } else {
        fmt.Printf("* This message was ignored: %s\n", m) // or don't do anything here or some other external messaging channel
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

question: can you make log.UseLogger public API?
3 participants