lostluck commented on issue #37143:
URL: https://github.com/apache/beam/issues/37143#issuecomment-3676358791

   I agree with your characterization of the issue @shunping . We want a good 
experience regardless of where/how the code is run, whether that's locally 
though loopback, or on remote workers, or lost in docker containers.
   
   The goal for any of the logging within Beam is to aid in debugging issues. 
(There are broadly other uses for logging, especially structured kinds, but 
that's a separate kind).
   
   I disagree that there's value in the distinction between the packages, 
beyond that as they exist, there are expectations for how they work presently. 
TBH, the Go "log" and our "beam/log" packages didn't have much thought put into 
them. I'd put more effort into ensuring the slog behavior when a user is using 
the "default" handler (however we've manipulated it, if needed) is reasonable 
for the execution case.
   
   The Go SDK also has a tricky bit of handling for locally that the other SDKs 
do not: The prism execution is local, and it uses slog internally. I would 
start there and figure out how that needs to behave. Obviously the slog logs 
within prism should not be redirected through the Beam Logging API back to 
itself in that circumstance.
   
   Note that most users don't want/need debugging output from the Prism part of 
execution.
   
   That should be an easy enough fix by plumbing explicit loggers through prism 
when it's operating in process like that, and not having it use the default 
handler ever, which might be used or overriden by user side handling. I believe 
that was mostly done recently by Shunping anyway.
   
   ---------
   
   In my explorations for in my "hobby" Go SDK, I opted to avoid any weird 
default logger behavior by putting an explicit logger on the framework's 
context object. This avoids breaking user side defaults, and makes the user 
intent explicit about what they want that print out to do/and go to. It's 
otherwise the *slog.Logger type for however it's used.
   
   Eg. 
https://github.com/lostluck/beam-go/blob/4eac3b7dabc67e978a6934dc67f9aaf3e3b662a3/dfc.go#L372
   
   I'm concerned that unless there's a material improvement to any new approach 
you take, it's going to be difficult for users to migrate or similar.  At best 
you can further mangle the parameter injection code to understand a 
*slog.Logger parameter, and have that be the "canonical Beam logger" that 
people use for Beam logging purposes, so any of the default injection sites are 
left to whatever the user does.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to