ajamato commented on a change in pull request #15394:
URL: https://github.com/apache/beam/pull/15394#discussion_r697775806
##########
File path:
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtil.java
##########
@@ -454,8 +458,31 @@ public SeekableByteChannel open(GcsPath path) throws
IOException {
@VisibleForTesting
SeekableByteChannel open(GcsPath path, GoogleCloudStorageReadOptions
readOptions)
throws IOException {
- return googleCloudStorage.open(
- new StorageResourceId(path.getBucket(), path.getObject()),
readOptions);
+ HashMap<String, String> baseLabels = new HashMap<>();
+ baseLabels.put(MonitoringInfoConstants.Labels.PTRANSFORM, "");
+ baseLabels.put(MonitoringInfoConstants.Labels.SERVICE, "Storage");
+ baseLabels.put(MonitoringInfoConstants.Labels.METHOD, "Objects.get");
+ baseLabels.put(
+ MonitoringInfoConstants.Labels.RESOURCE,
+ GcpResourceIdentifiers.cloudStorageBucket(path.getBucket()));
+ baseLabels.put(
+ MonitoringInfoConstants.Labels.GCS_PROJECT_ID,
googleCloudStorageOptions.getProjectId());
+ baseLabels.put(MonitoringInfoConstants.Labels.GCS_BUCKET,
path.getBucket());
+
+ ServiceCallMetric serviceCallMetric =
+ new ServiceCallMetric(MonitoringInfoConstants.Urns.API_REQUEST_COUNT,
baseLabels);
+ try {
+ SeekableByteChannel channel =
+ googleCloudStorage.open(
+ new StorageResourceId(path.getBucket(), path.getObject()),
readOptions);
+ serviceCallMetric.call("ok");
+ return channel;
+ } catch (IOException e) {
+ if (e.getCause() instanceof GoogleJsonResponseException) {
+ serviceCallMetric.call(((GoogleJsonResponseException)
e.getCause()).getDetails().getCode());
Review comment:
Has this been verified to be the proper way to catch this using an
integration test?
##########
File path:
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtil.java
##########
@@ -487,9 +514,35 @@ public WritableByteChannel create(GcsPath path, String
type, Integer uploadBuffe
GoogleCloudStorage gcpStorage =
new GoogleCloudStorageImpl(
newGoogleCloudStorageOptions, this.storageClient,
this.credentials);
- return gcpStorage.create(
- new StorageResourceId(path.getBucket(), path.getObject()),
-
CreateObjectOptions.builder().setOverwriteExisting(true).setContentType(type).build());
+ HashMap<String, String> baseLabels = new HashMap<>();
+ baseLabels.put(MonitoringInfoConstants.Labels.PTRANSFORM, "");
+ baseLabels.put(MonitoringInfoConstants.Labels.SERVICE, "Storage");
+ baseLabels.put(MonitoringInfoConstants.Labels.METHOD, "Objects.insert");
Review comment:
Lets call this something more general, incase the underlying API changes.
Then upstream code handling the MonitoringInfo won't need to change, as the
Method label will not need to change.
METHOD = "GcsInsert"
##########
File path:
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtil.java
##########
@@ -454,8 +458,31 @@ public SeekableByteChannel open(GcsPath path) throws
IOException {
@VisibleForTesting
SeekableByteChannel open(GcsPath path, GoogleCloudStorageReadOptions
readOptions)
throws IOException {
- return googleCloudStorage.open(
- new StorageResourceId(path.getBucket(), path.getObject()),
readOptions);
+ HashMap<String, String> baseLabels = new HashMap<>();
+ baseLabels.put(MonitoringInfoConstants.Labels.PTRANSFORM, "");
+ baseLabels.put(MonitoringInfoConstants.Labels.SERVICE, "Storage");
+ baseLabels.put(MonitoringInfoConstants.Labels.METHOD, "Objects.get");
Review comment:
Lets call this something more general, incase the underlying API changes.
Then upstream code handling the MonitoringInfo won't need to change, as the
Method label will not need to change.
METHOD = "GcsGet"
--
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]
