youngoli commented on a change in pull request #15713:
URL: https://github.com/apache/beam/pull/15713#discussion_r727678118



##########
File path: sdks/go/pkg/beam/core/runtime/harness/harness.go
##########
@@ -47,8 +47,10 @@ const cacheSize = 20
 func Main(ctx context.Context, loggingEndpoint, controlEndpoint string) error {
        hooks.DeserializeHooksFromOptions(ctx)
 
+       // Pass in the logging endpoint for use w/the default remote logging 
hook.
+       ctx = context.WithValue(ctx, loggingEndpointCtxKey, loggingEndpoint)

Review comment:
       Just a question: If a runner disables the default logging endpoint, then 
this does nothing, right? Just want to make sure I'm not missing anything.

##########
File path: sdks/go/pkg/beam/core/runtime/harness/logging.go
##########
@@ -95,6 +96,28 @@ func convertSeverity(sev log.Severity) 
fnpb.LogEntry_Severity_Enum {
        }
 }
 
+type remoteLoggingKey string
+
+// DefaultRemoteLoggingHook is the key used for the default remote logging 
hook.
+// If a runner wants to use an alternative logging solution, it can be
+// disabled with hooks.DisableHook(harness.DefaultRemoteLoggingHook).
+const DefaultRemoteLoggingHook = "default_remote_logging"
+
+var loggingEndpointCtxKey = remoteLoggingKey(DefaultRemoteLoggingHook)
+
+func init() {

Review comment:
       So I'm trying to understand how hooks work correctly, and how this 
init() function interacts with hooks.
   
   So this remote logging hook is meant to send SDK harness logs back to the 
runner (in this comment called FnHarness), right? So looking at its lifecycle:
   1. It gets registered and enabled in this init function in the user's 
pipeline code (i.e. before beam.Init)
   2. It gets serialized in options and deserialized in the SDK harness.
   3. Init gets called on the hook, which connects the SDK harness to the 
remote logging endpoint (i.e the runner).
   
   Another thing I want to double check: Doesn't this init() function, and by 
extension it's registering and enabling the hook, also get called in the SDK 
harness? I'm guessing DeserializeHooksFromOptions replaces any registered hooks 
when it gets called in harness.go?




-- 
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