lostluck commented on a change in pull request #15713:
URL: https://github.com/apache/beam/pull/15713#discussion_r728253180
##########
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:
Yes. An alternative would be to move the remote logging set up to the
harness/init package, so the flag is plumbed as a flag instead of as the
context. That would require exporting some of the logging set up so it would be
accessible though, as we don't want to copy the GRPC code out of the harness
package.
Overall, this was the least obtrusive. At worst, the context is carrying
around an extra value, but this is fine.
--
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]
