lukecwik commented on a change in pull request #17094:
URL: https://github.com/apache/beam/pull/17094#discussion_r830316956



##########
File path: 
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtil.java
##########
@@ -466,7 +467,8 @@ SeekableByteChannel open(GcsPath path, 
GoogleCloudStorageReadOptions readOptions
         MonitoringInfoConstants.Labels.RESOURCE,
         GcpResourceIdentifiers.cloudStorageBucket(path.getBucket()));
     baseLabels.put(
-        MonitoringInfoConstants.Labels.GCS_PROJECT_ID, 
googleCloudStorageOptions.getProjectId());
+        MonitoringInfoConstants.Labels.GCS_PROJECT_ID,
+        MoreObjects.firstNonNull(googleCloudStorageOptions.getProjectId(), 
""));

Review comment:
       It looks like project id will not be set for the default case since the 
GcsUtil does not set it at all.
   
   I found 
https://docs.google.com/document/d/1K_qsi1TMtt0fLxEjkM1taHIZX5V1csGAjh1pMALr4pI/edit#
 but it does not seem to cover unknown values and what to do there. I did see 
that in gcsio.py we try to get the project number so that would mean that we 
would want to use String.valueOf(...) here which will convert `null` to 
`"null"` similar to how unknown project numbers in Python will turn into 
`"None"`. `"null"` will be more obvious that this isn't implemented.




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