lostluck commented on a change in pull request #15713:
URL: https://github.com/apache/beam/pull/15713#discussion_r728267033
##########
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:
That's right, this sets up remote logging to the FnAPI Logging Service,
for the runner to do with as it likes.
That's the correct flow.
Since all beam Go workers depend on the harness, this init happens pretty
early in the package initialization process, and likely before any user code.
Certainly before any runner code (the harness dependency happens through the
runners). That means the runners can disable it at their leisure.
The alternative would be to register the hook and have runners enable it
explicitly, however, this is more likely to be missed by newer runner packages.
Simpler to have the default work as expected, and have exceptional runners that
want to make different choices explicitly disable the normal log handling.
That's correct, DeserilaizeHooksFromOptions overwrites all the existing
hooks in the registry. The function is only happening at worker side anyway at
harness main, and at that point we need the hooks to be relayed through the
options.
--
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]
