Abacn commented on code in PR #31466:
URL: https://github.com/apache/beam/pull/31466#discussion_r1635013221


##########
sdks/python/apache_beam/io/gcp/gcsio.py:
##########
@@ -530,19 +541,63 @@ def _updated_to_seconds(updated):
         updated.microsecond / 1000000.0)
 
 
+def increment_bucket_read_counter_metric(read):
+  def inner(self, *args, **kwargs):
+    bytesRead = read(self, *args, **kwargs)
+    if self.enable_read_bucket_metric:
+      Metrics.counter(
+          self.__class__,
+          "GCS_read_bytes_counter_" + self._blob.bucket.name).inc(
+              len(bytesRead))
+    return bytesRead
+
+  return inner
+
+
 class BeamBlobReader(BlobReader):
-  def __init__(self, blob, chunk_size=DEFAULT_READ_BUFFER_SIZE):
+  def __init__(
+      self,
+      blob,
+      chunk_size=DEFAULT_READ_BUFFER_SIZE,
+      enable_read_bucket_metric=False):
     super().__init__(blob, chunk_size=chunk_size)
+    self.enable_read_bucket_metric = enable_read_bucket_metric
     self.mode = "r"
 
+  @increment_bucket_read_counter_metric

Review Comment:
   Decorator here seems unnecessary. Already overwrite read() and write() 
method and the counter increment could just happen there
   
   The refactor should also consider minimize the overhead when bucket_metric 
is not enabled.



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