fixed lint

Author: daniel-sanche

google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py

@@ -53,10 +53,11 @@
 # avoid reformatting into individual lines
 # fmt: off
 MILLIS_AGGREGATION = view.ExplicitBucketHistogramAggregation(
-    [     0, 1, 2, 3, 4, 5, 6, 8, 10, 13, 16, 20, 25, 30, 40,
-          50, 65, 80, 100, 130, 160, 200, 250, 300, 400, 500, 650,
-          800, 1_000, 2_000, 5_000, 10_000, 20_000, 50_000, 100_000, 
-          200_000, 400_000, 800_000, 1_600_000, 3_200_000
+    [
+        0, 1, 2, 3, 4, 5, 6, 8, 10, 13, 16, 20, 25, 30, 40,
+        50, 65, 80, 100, 130, 160, 200, 250, 300, 400, 500, 650,
+        800, 1_000, 2_000, 5_000, 10_000, 20_000, 50_000, 100_000,
+        200_000, 400_000, 800_000, 1_600_000, 3_200_000
     ]
 )
 # fmt: on

google/cloud/bigtable/data/_sync_autogen/client.py

@@ -776,6 +776,7 @@ def __init__(
                     instance_id=instance_id,
                     table_id=table_id,
                     app_profile_id=app_profile_id,
+                    client_version=client._client_version(),
                 )
             ]
         )

tests/system/data/setup_fixtures.py

@@ -19,7 +19,6 @@
 import pytest
 import os
 import uuid
-import datetime
 
 
 @pytest.fixture(scope="session")

tests/system/data/test_metrics_async.py

@@ -15,7 +15,6 @@
 import os
 import pytest
 import uuid
-import datetime
 
 from grpc import StatusCode
 
@@ -26,11 +25,9 @@
 from google.cloud.bigtable.data._metrics.data_model import (
     CompletedOperationMetric,
     CompletedAttemptMetric,
-    OperationType,
 )
 from google.cloud.bigtable.data.read_rows_query import ReadRowsQuery
 from google.cloud.bigtable_v2.types import ResponseParams
-from google.cloud.bigtable import __version__ as CLIENT_VERSION
 
 from google.cloud.bigtable.data._cross_sync import CrossSync
 

tests/system/data/test_system_autogen.py

@@ -1081,62 +1081,43 @@ def test_execute_metadata_on_empty_response(
             SqlType.Bytes(), SqlType.Int64()
         )
 
+    @pytest.fixture(scope="session")
+    def metrics_client(self, client):
+        yield client._gcp_metrics_exporter.client
+
     @pytest.mark.order("last")
-    class TestExportedMetrics(SystemTestRunner):
-        """
-        Checks to make sure metrics were exported by tests
-
-        Runs at the end of test suite, to allow other tests to write metrics
-        """
-
-        @pytest.fixture(scope="session")
-        def metrics_client(self, client):
-            yield client._gcp_metrics_exporter.client
-
-        @pytest.fixture(scope="session")
-        def time_interval(self, start_timestamp):
-            """Build a time interval between when system tests started, and the exported metric tests
-
-            Optionally adds LOOKBACK_MINUTES value for testing"""
-            end_time = datetime.datetime.now(datetime.timezone.utc)
-            LOOKBACK_MINUTES = os.getenv("LOOKBACK_MINUTES")
-            if LOOKBACK_MINUTES is not None:
-                print(f"running with LOOKBACK_MINUTES={LOOKBACK_MINUTES}")
-                start_timestamp = start_timestamp - datetime.timedelta(
-                    minutes=int(LOOKBACK_MINUTES)
-                )
-            return {"start_time": start_timestamp, "end_time": end_time}
-
-        @pytest.mark.parametrize(
-            "metric,methods",
-            [
-                ("attempt_latencies", [m.value for m in OperationType]),
-                ("operation_latencies", [m.value for m in OperationType]),
-                ("retry_count", [m.value for m in OperationType]),
-                ("first_response_latencies", [OperationType.READ_ROWS]),
-                ("server_latencies", [m.value for m in OperationType]),
-                ("connectivity_error_count", [m.value for m in OperationType]),
-                ("application_blocking_latencies", [OperationType.READ_ROWS]),
-            ],
-        )
-        @retry.Retry(predicate=retry.if_exception_type(AssertionError))
-        def test_metric_existence(
-            self, client, table_id, metrics_client, time_interval, metric, methods
-        ):
-            """Checks existence of each metric in Cloud Monitoring"""
-            from google.cloud.bigtable import __version__ as CLIENT_VERSION
-
-            for m in methods:
-                metric_filter = (
-                    f'metric.type = "bigtable.googleapis.com/client/{metric}" '
-                    + f'AND metric.labels.client_name = "python-bigtable/{CLIENT_VERSION}" AND resource.labels.table = "{table_id}" '
-                )
-                results = list(
-                    metrics_client.list_time_series(
-                        name=f"projects/{client.project}",
-                        filter=metric_filter,
-                        interval=time_interval,
-                        view=0,
-                    )
+    @pytest.mark.parametrize(
+        "metric,methods",
+        [
+            ("attempt_latencies", [m.value for m in OperationType]),
+            ("operation_latencies", [m.value for m in OperationType]),
+            ("retry_count", [m.value for m in OperationType]),
+            ("first_response_latencies", [OperationType.READ_ROWS]),
+            ("server_latencies", [m.value for m in OperationType]),
+            ("connectivity_error_count", [m.value for m in OperationType]),
+            ("application_blocking_latencies", [OperationType.READ_ROWS]),
+        ],
+    )
+    @retry.Retry(predicate=retry.if_exception_type(AssertionError))
+    def test_metric_existence(
+        self, client, table_id, metrics_client, start_timestamp, metric, methods
+    ):
+        """Checks to make sure metrics were exported by tests
+
+        Runs at the end of test suite, to let other tests write metrics"""
+        end_timestamp = datetime.datetime.now(datetime.timezone.utc)
+        for m in methods:
+            metric_filter = (
+                f'metric.type = "bigtable.googleapis.com/client/{metric}" '
+                + f'AND metric.labels.client_name = "python-bigtable/{client._client_version()}" '
+                + f'AND resource.labels.table = "{table_id}" '
+            )
+            results = list(
+                metrics_client.list_time_series(
+                    name=f"projects/{client.project}",
+                    filter=metric_filter,
+                    interval={"start_time": start_timestamp, "end_time": end_timestamp},
+                    view=0,
                 )
-                assert len(results) > 0, f"No data found for {metric} {m}"
+            )
+            assert len(results) > 0, f"No data found for {metric} {m}"

tests/unit/data/_metrics/test_gcp_exporter_handler.py

@@ -24,7 +24,6 @@
     ScopeMetrics,
     Metric,
     Sum,
-    Histogram,
     AggregationTemporality,
 )
 from google.cloud.monitoring_v3 import (
@@ -46,7 +45,6 @@
 
 
 class TestGoogleCloudMetricsHandler:
-
     def _make_one(self, *args, **kwargs):
         return GoogleCloudMetricsHandler(*args, **kwargs)
 
@@ -62,7 +60,7 @@ def test_ctor_defaults(self):
             handler = self._make_one(
                 expected_exporter,
                 instance_id=expected_instance,
-                table_id=expected_table
+                table_id=expected_table,
             )
         assert isinstance(handler.meter_provider, MeterProvider)
         assert isinstance(handler.otel, _OpenTelemetryInstruments)
@@ -80,7 +78,6 @@ def test_ctor_explicit(self):
         expected_version = "my_version"
         expected_uid = "my_uid"
         expected_app_profile = "my_profile"
-        expected_instruments = object()
         expected_exporter = BigtableMetricsExporter("project")
         handler = self._make_one(
             expected_exporter,
@@ -99,8 +96,6 @@ def test_ctor_explicit(self):
         )
         assert handler.shared_labels["client_uid"] == expected_uid
 
-
-
     @mock.patch(
         "google.cloud.bigtable.data._metrics.handlers.gcp_exporter.PeriodicExportingMetricReader"
     )
@@ -188,13 +183,20 @@ def test__to_point_w_number(self, value, expected_field):
         expected_start_time_nanos = 100
         expected_end_time_nanos = 200
         dp = NumberDataPoint(
-            attributes={}, start_time_unix_nano=expected_start_time_nanos, time_unix_nano=expected_end_time_nanos, value=value
+            attributes={},
+            start_time_unix_nano=expected_start_time_nanos,
+            time_unix_nano=expected_end_time_nanos,
+            value=value,
         )
         point = instance._to_point(dp)
         assert isinstance(point, Point)
         assert getattr(point.value, expected_field) == value
-        assert (point.interval.start_time.second * 10**9) + point.interval.start_time.nanosecond == expected_start_time_nanos
-        assert (point.interval.end_time.second * 10**9) + point.interval.end_time.nanosecond == expected_end_time_nanos
+        assert (
+            point.interval.start_time.second * 10**9
+        ) + point.interval.start_time.nanosecond == expected_start_time_nanos
+        assert (
+            point.interval.end_time.second * 10**9
+        ) + point.interval.end_time.nanosecond == expected_end_time_nanos
 
     def test__to_point_w_histogram(self):
         """Test that HistogramDataPoint is converted to a Point correctly."""
@@ -223,9 +225,16 @@ def test__to_point_w_histogram(self):
         assert dist.count == expected_count
         assert dist.mean == expected_sum / expected_count
         assert list(dist.bucket_counts) == expected_bucket_counts
-        assert list(dist.bucket_options.explicit_buckets.bounds) == expected_explicit_bounds
-        assert (point.interval.start_time.second * 10**9) + point.interval.start_time.nanosecond == expected_start_time_nanos
-        assert (point.interval.end_time.second * 10**9) + point.interval.end_time.nanosecond == expected_end_time_nanos
+        assert (
+            list(dist.bucket_options.explicit_buckets.bounds)
+            == expected_explicit_bounds
+        )
+        assert (
+            point.interval.start_time.second * 10**9
+        ) + point.interval.start_time.nanosecond == expected_start_time_nanos
+        assert (
+            point.interval.end_time.second * 10**9
+        ) + point.interval.end_time.nanosecond == expected_end_time_nanos
 
     def test__to_point_w_histogram_zero_count(self):
         """Test that HistogramDataPoint with zero count is converted to a Point correctly."""
@@ -329,7 +338,9 @@ def test_export(self):
                 is_monotonic=False,
             ),
         )
-        scope_metric = ScopeMetrics(scope=mock.Mock(), metrics=[metric], schema_url=None)
+        scope_metric = ScopeMetrics(
+            scope=mock.Mock(), metrics=[metric], schema_url=None
+        )
         resource_metric = ResourceMetrics(
             resource=mock.Mock(), scope_metrics=[scope_metric], schema_url=None
         )
@@ -372,7 +383,9 @@ def test_export_no_attributes(self):
                 is_monotonic=False,
             ),
         )
-        scope_metric = ScopeMetrics(scope=mock.Mock(), metrics=[metric], schema_url=None)
+        scope_metric = ScopeMetrics(
+            scope=mock.Mock(), metrics=[metric], schema_url=None
+        )
         resource_metric = ResourceMetrics(
             resource=mock.Mock(), scope_metrics=[scope_metric], schema_url=None
         )
@@ -412,7 +425,9 @@ def test_exception_in_export(self):
                 is_monotonic=False,
             ),
         )
-        scope_metric = ScopeMetrics(scope=mock.Mock(), metrics=[metric], schema_url=None)
+        scope_metric = ScopeMetrics(
+            scope=mock.Mock(), metrics=[metric], schema_url=None
+        )
         resource_metric = ResourceMetrics(
             resource=mock.Mock(), scope_metrics=[scope_metric], schema_url=None
         )

tests/unit/data/_metrics/test_opentelemetry_handler.py

@@ -17,8 +17,6 @@
 from grpc import StatusCode
 
 from google.cloud.bigtable.data._metrics.data_model import (
-    DEFAULT_CLUSTER_ID,
-    DEFAULT_ZONE,
     ActiveOperationMetric,
     CompletedAttemptMetric,
     CompletedOperationMetric,

tests/unit/data/_sync_autogen/test_client.py

@@ -79,6 +79,10 @@ def _make_client(cls, *args, use_emulator=True, **kwargs):
             return cls._get_target_class()(*args, **kwargs)
 
     def test_ctor(self):
+        from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import (
+            BigtableMetricsExporter,
+        )
+
         expected_project = "project-id"
         expected_credentials = AnonymousCredentials()
         client = self._make_client(
@@ -92,6 +96,8 @@ def test_ctor(self):
         assert isinstance(
             client._metrics_interceptor, CrossSync._Sync_Impl.MetricsInterceptor
         )
+        assert client._gcp_metrics_exporter is not None
+        assert isinstance(client._gcp_metrics_exporter, BigtableMetricsExporter)
         client.close()
 
     def test_ctor_super_inits(self):
@@ -154,6 +160,31 @@ def test_ctor_dict_options(self):
             start_background_refresh.assert_called_once()
             client.close()
 
+    def test_metrics_exporter_init_shares_arguments(self):
+        expected_credentials = AnonymousCredentials()
+        expected_project = "custom_project"
+        expected_options = client_options.ClientOptions()
+        expected_options.credentials_file = None
+        expected_options.quota_project_id = None
+        with mock.patch(
+            "google.cloud.bigtable.data._metrics.handlers.gcp_exporter.BigtableMetricsExporter.__init__",
+            return_value=None,
+        ) as exporter_mock:
+            with self._make_client(
+                project=expected_project,
+                credentials=expected_credentials,
+                client_options=expected_options,
+            ):
+                exporter_mock.assert_called_once_with(
+                    project_id=expected_project,
+                    credentials=expected_credentials,
+                    client_options=expected_options,
+                )
+
+    def test_metrics_exporter_init_implicit_project(self):
+        with self._make_client() as client:
+            assert client._gcp_metrics_exporter.project_id == client.project
+
     def test_veneer_grpc_headers(self):
         client_component = "data-async" if CrossSync._Sync_Impl.is_async else "data"
         VENEER_HEADER_REGEX = re.compile(
@@ -941,6 +972,7 @@ def test_ctor(self):
         from google.cloud.bigtable.data._metrics import (
             BigtableClientSideMetricsController,
         )
+        from google.cloud.bigtable.data._metrics import GoogleCloudMetricsHandler
 
         expected_table_id = "table-id"
         expected_instance_id = "instance-id"
@@ -982,6 +1014,8 @@ def test_ctor(self):
         assert instance_key in client._active_instances
         assert client._instance_owners[instance_key] == {id(table)}
         assert isinstance(table._metrics, BigtableClientSideMetricsController)
+        assert len(table._metrics.handlers) == 1
+        assert isinstance(table._metrics.handlers[0], GoogleCloudMetricsHandler)
         assert table.default_operation_timeout == expected_operation_timeout
         assert table.default_attempt_timeout == expected_attempt_timeout
         assert (
@@ -1201,6 +1235,7 @@ def test_ctor(self):
         from google.cloud.bigtable.data._helpers import _WarmedInstanceKey
         from google.cloud.bigtable.data._metrics import (
             BigtableClientSideMetricsController,
+            GoogleCloudMetricsHandler,
         )
 
         expected_table_id = "table-id"
@@ -1250,6 +1285,8 @@ def test_ctor(self):
         assert instance_key in client._active_instances
         assert client._instance_owners[instance_key] == {id(view)}
         assert isinstance(view._metrics, BigtableClientSideMetricsController)
+        assert len(view._metrics.handlers) == 1
+        assert isinstance(view._metrics.handlers[0], GoogleCloudMetricsHandler)
         assert view.default_operation_timeout == expected_operation_timeout
         assert view.default_attempt_timeout == expected_attempt_timeout
         assert (