AtharvUrunkar commented on code in PR #38246:
URL: https://github.com/apache/beam/pull/38246#discussion_r3115685420
##########
sdks/python/container/boot.go:
##########
@@ -314,23 +314,47 @@ func launchSDKProcess() error {
var wg sync.WaitGroup
wg.Add(len(workerIds))
- for _, workerId := range workerIds {
- go func(workerId string) {
- defer wg.Done()
-
- bufLogger := tools.NewBufferedLogger(logger)
- errorCount := 0
- for {
- childPids.mu.Lock()
- if childPids.canceled {
- childPids.mu.Unlock()
- return
- }
- logger.Printf(ctx, "Executing Python (worker
%v): python %v", workerId, strings.Join(args, " "))
- cmd :=
StartCommandEnv(map[string]string{"WORKER_ID": workerId}, os.Stdin, bufLogger,
bufLogger, "python", args...)
- childPids.v = append(childPids.v,
cmd.Process.Pid)
- childPids.mu.Unlock()
-
+for _, workerId := range workerIds {
+ go func(workerId string) {
+ defer wg.Done()
+
+ workerCtx := grpcx.WriteWorkerID(ctx, workerId)
+
+ // Create a separate logger per worker so that each worker initializes
+ // its own Fn logging stream with the correct worker_id metadata.
+ // Shared loggers would reuse the first stream and cause incorrect
+ // portability_worker_id attribution across workers.
+ workerLogger := &tools.Logger{
+ Endpoint: *loggingEndpoint,
+ }
+
+ bufLogger := tools.NewBufferedLogger(workerLogger)
Review Comment:
Good observation — thanks for pointing this out.
Currently, stdout/stderr logs are emitted via `FlushAtDebug`, which can make
them harder to discover unless debug logging is enabled.
I agree that making this configurable (rather than changing the default)
would likely be a safer approach, since altering the default log level could
have broader impact on existing users.
For this PR, I’d prefer to keep the current behavior unchanged and focus on
ensuring correct worker context propagation. But I think exposing
configurability for log level could be a valuable follow-up improvement.
--
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]
