From 7fd21caa797c32f55766da10078d57b9715134da Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 8 Aug 2025 15:38:15 -0700 Subject: [PATCH 01/33] copied files from previous branch --- .../cloud/bigtable/data/_metrics/__init__.py | 10 + .../data/_metrics/handlers/_stdout.py | 48 ++++ .../data/_metrics/handlers/gcp_exporter.py | 269 ++++++++++++++++++ .../data/_metrics/handlers/opentelemetry.py | 233 +++++++++++++++ 4 files changed, 560 insertions(+) create mode 100644 google/cloud/bigtable/data/_metrics/handlers/_stdout.py create mode 100644 google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py create mode 100644 google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py diff --git a/google/cloud/bigtable/data/_metrics/__init__.py b/google/cloud/bigtable/data/_metrics/__init__.py index 20d36d4c8..9baf179bc 100644 --- a/google/cloud/bigtable/data/_metrics/__init__.py +++ b/google/cloud/bigtable/data/_metrics/__init__.py @@ -11,6 +11,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from google.cloud.bigtable.data._metrics.handlers.opentelemetry import ( + OpenTelemetryMetricsHandler, +) +from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import ( + GoogleCloudMetricsHandler, +) +from google.cloud.bigtable.data._metrics.handlers._stdout import _StdoutMetricsHandler from google.cloud.bigtable.data._metrics.metrics_controller import ( BigtableClientSideMetricsController, ) @@ -23,6 +30,9 @@ __all__ = ( "BigtableClientSideMetricsController", + "OpenTelemetryMetricsHandler", + "GoogleCloudMetricsHandler", + "_StdoutMetricsHandler", "OperationType", "ActiveOperationMetric", "ActiveAttemptMetric", diff --git a/google/cloud/bigtable/data/_metrics/handlers/_stdout.py b/google/cloud/bigtable/data/_metrics/handlers/_stdout.py new file mode 100644 index 000000000..e0a4bc8da --- /dev/null +++ b/google/cloud/bigtable/data/_metrics/handlers/_stdout.py @@ -0,0 +1,48 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from google.cloud.bigtable.data._metrics.handlers._base import MetricsHandler +from google.cloud.bigtable.data._metrics.data_model import CompletedOperationMetric + + +class _StdoutMetricsHandler(MetricsHandler): + """ + Prints a table of metric data after each operation, for debugging purposes. + """ + + def __init__(self, **kwargs): + self._completed_ops = {} + + def on_operation_complete(self, op: CompletedOperationMetric) -> None: + """ + After each operation, update the state and print the metrics table. + """ + current_list = self._completed_ops.setdefault(op.op_type, []) + current_list.append(op) + self.print() + + def print(self): + """ + Print the current state of the metrics table. + """ + print("Bigtable Metrics:") + for ops_type, ops_list in self._completed_ops.items(): + count = len(ops_list) + total_latency = sum([op.duration_ns // 1_000_000 for op in ops_list]) + total_attempts = sum([len(op.completed_attempts) for op in ops_list]) + avg_latency = total_latency / count + avg_attempts = total_attempts / count + print( + f"{ops_type}: count: {count}, avg latency: {avg_latency:.2f} ms, avg attempts: {avg_attempts:.1f}" + ) + print() diff --git a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py new file mode 100644 index 000000000..27941c5b5 --- /dev/null +++ b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py @@ -0,0 +1,269 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import annotations + +import time + +from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics import view +from opentelemetry.sdk.metrics.export import ( + HistogramDataPoint, + MetricExporter, + MetricExportResult, + MetricsData, + NumberDataPoint, + PeriodicExportingMetricReader, +) +from google.protobuf.timestamp_pb2 import Timestamp +from google.api.distribution_pb2 import Distribution +from google.api.metric_pb2 import Metric as GMetric +from google.api.monitored_resource_pb2 import MonitoredResource +from google.api.metric_pb2 import MetricDescriptor +from google.api_core import gapic_v1 +from google.cloud.monitoring_v3 import ( + CreateTimeSeriesRequest, + MetricServiceClient, + Point, + TimeInterval, + TimeSeries, + TypedValue, +) + +from google.cloud.bigtable.data._metrics.handlers.opentelemetry import ( + OpenTelemetryMetricsHandler, +) +from google.cloud.bigtable.data._metrics.handlers.opentelemetry import ( + _OpenTelemetryInstruments, +) + + +# create OpenTelemetry views for Bigtable metrics +# avoid reformatting into individual lines +# fmt: off +MILLIS_AGGREGATION = view.ExplicitBucketHistogramAggregation( + [ + 0, 0.01, 0.05, 0.1, 0.3, 0.6, 0.8, 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, 1000, 2000, 5000, 10000, 20000, 50000, 100000, + ] +) +# fmt: on +COUNT_AGGREGATION = view.SumAggregation() +INSTRUMENT_NAMES = ( + "operation_latencies", + "first_response_latencies", + "attempt_latencies", + "retry_count", + "server_latencies", + "connectivity_error_count", + "application_latencies", + "throttling_latencies", +) +VIEW_LIST = [ + view.View( + instrument_name=n, + name=n, + aggregation=MILLIS_AGGREGATION + if n.endswith("latencies") + else COUNT_AGGREGATION, + ) + for n in INSTRUMENT_NAMES +] + + +class GoogleCloudMetricsHandler(OpenTelemetryMetricsHandler): + """ + Maintains an internal set of OpenTelemetry metrics for the Bigtable client library, + and periodically exports them to Google Cloud Monitoring. + + The OpenTelemetry metrics that are tracked are as follows: + - operation_latencies: latency of each client method call, over all of it's attempts. + - first_response_latencies: latency of receiving the first row in a ReadRows operation. + - attempt_latencies: latency of each client attempt RPC. + - retry_count: Number of additional RPCs sent after the initial attempt. + - server_latencies: latency recorded on the server side for each attempt. + - connectivity_error_count: number of attempts that failed to reach Google's network. + - application_latencies: the time spent waiting for the application to process the next response. + - throttling_latencies: latency introduced by waiting when there are too many outstanding requests in a bulk operation. + + Args: + - project_id: The Google Cloud project ID for the associated Bigtable Table + - export_interval: The interval (in seconds) at which to export metrics to Cloud Monitoring. + """ + + def __init__(self, *args, project_id: str, export_interval=60, **kwargs): + # internal exporter to write metrics to Cloud Monitoring + exporter = _BigtableMetricsExporter(project_id=project_id) + # periodically executes exporter + gcp_reader = PeriodicExportingMetricReader( + exporter, export_interval_millis=export_interval * 1000 + ) + # use private meter provider to store instruments and views + meter_provider = MeterProvider(metric_readers=[gcp_reader], views=VIEW_LIST) + otel = _OpenTelemetryInstruments(meter_provider=meter_provider) + super().__init__(*args, instruments=otel, project_id=project_id, **kwargs) + + +class _BigtableMetricsExporter(MetricExporter): + """ + OpenTelemetry Exporter implementation for sending metrics to Google Cloud Monitoring. + + We must use a custom exporter because the public one doesn't support writing to internal + metrics like `bigtable.googleapis.com/internal/client/` + + Each GoogleCloudMetricsHandler will maintain its own exporter instance associated with the + project_id it is configured with. + + Args: + - project_id: GCP project id to associate metrics with + """ + + def __init__(self, project_id: str): + super().__init__() + self.client = MetricServiceClient() + self.prefix = "bigtable.googleapis.com/internal/client" + self.project_name = self.client.common_project_path(project_id) + + def export( + self, metrics_data: MetricsData, timeout_millis: float = 10_000, **kwargs + ) -> MetricExportResult: + """ + Write a set of metrics to Cloud Monitoring. + This method is called by the OpenTelemetry SDK + """ + deadline = time.time() + (timeout_millis / 1000) + metric_kind = MetricDescriptor.MetricKind.CUMULATIVE + all_series: list[TimeSeries] = [] + # process each metric from OTel format into Cloud Monitoring format + for resource_metric in metrics_data.resource_metrics: + for scope_metric in resource_metric.scope_metrics: + for metric in scope_metric.metrics: + for data_point in [ + pt for pt in metric.data.data_points if pt.attributes + ]: + if data_point.attributes: + project_id = data_point.attributes.get("resource_project") + if not isinstance(project_id, str): + # we expect string for project_id field + continue + monitored_resource = MonitoredResource( + type="bigtable_client_raw", + labels={ + "project_id": project_id, + "instance": data_point.attributes[ + "resource_instance" + ], + "cluster": data_point.attributes[ + "resource_cluster" + ], + "table": data_point.attributes["resource_table"], + "zone": data_point.attributes["resource_zone"], + }, + ) + point = self._to_point(data_point) + series = TimeSeries( + resource=monitored_resource, + metric_kind=metric_kind, + points=[point], + metric=GMetric( + type=f"{self.prefix}/{metric.name}", + labels={ + k: v + for k, v in data_point.attributes.items() + if not k.startswith("resource_") + }, + ), + unit=metric.unit, + ) + all_series.append(series) + # send all metrics to Cloud Monitoring + try: + self._batch_write(all_series, deadline) + return MetricExportResult.SUCCESS + except Exception: + return MetricExportResult.FAILURE + + def _batch_write( + self, series: list[TimeSeries], deadline=None, max_batch_size=200 + ) -> None: + """ + Adapted from CloudMonitoringMetricsExporter + https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/blob/3668dfe7ce3b80dd01f42af72428de957b58b316/opentelemetry-exporter-gcp-monitoring/src/opentelemetry/exporter/cloud_monitoring/__init__.py#L82 + + Args: + - series: list of TimeSeries to write. Will be split into batches if necessary + - deadline: designates the time.time() at which to stop writing. If None, uses API default + - max_batch_size: maximum number of time series to write at once. + Cloud Monitoring allows up to 200 per request + """ + write_ind = 0 + while write_ind < len(series): + # find time left for next batch + timeout = deadline - time.time() if deadline else gapic_v1.method.DEFAULT + # write next batch + self.client.create_service_time_series( + CreateTimeSeriesRequest( + name=self.project_name, + time_series=series[write_ind : write_ind + max_batch_size], + ), + timeout=timeout, + ) + write_ind += max_batch_size + + @staticmethod + def _to_point(data_point: NumberDataPoint | HistogramDataPoint) -> Point: + """ + Adapted from CloudMonitoringMetricsExporter + https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/blob/3668dfe7ce3b80dd01f42af72428de957b58b316/opentelemetry-exporter-gcp-monitoring/src/opentelemetry/exporter/cloud_monitoring/__init__.py#L82 + """ + if isinstance(data_point, HistogramDataPoint): + mean = data_point.sum / data_point.count if data_point.count else 0.0 + point_value = TypedValue( + distribution_value=Distribution( + count=data_point.count, + mean=mean, + bucket_counts=data_point.bucket_counts, + bucket_options=Distribution.BucketOptions( + explicit_buckets=Distribution.BucketOptions.Explicit( + bounds=data_point.explicit_bounds, + ) + ), + ) + ) + else: + if isinstance(data_point.value, int): + point_value = TypedValue(int64_value=data_point.value) + else: + point_value = TypedValue(double_value=data_point.value) + start_time = Timestamp() + start_time.FromNanoseconds(data_point.start_time_unix_nano) + end_time = Timestamp() + end_time.FromNanoseconds(data_point.time_unix_nano) + interval = TimeInterval(start_time=start_time, end_time=end_time) + return Point(interval=interval, value=point_value) + + def shutdown(self, timeout_millis: float = 30_000, **kwargs): + """ + Adapted from CloudMonitoringMetricsExporter + https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/blob/3668dfe7ce3b80dd01f42af72428de957b58b316/opentelemetry-exporter-gcp-monitoring/src/opentelemetry/exporter/cloud_monitoring/__init__.py#L82 + """ + pass + + def force_flush(self, timeout_millis: float = 10_000): + """ + Adapted from CloudMonitoringMetricsExporter + https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/blob/3668dfe7ce3b80dd01f42af72428de957b58b316/opentelemetry-exporter-gcp-monitoring/src/opentelemetry/exporter/cloud_monitoring/__init__.py#L82 + """ + return True \ No newline at end of file diff --git a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py new file mode 100644 index 000000000..7bc5b20f4 --- /dev/null +++ b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py @@ -0,0 +1,233 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from __future__ import annotations + +import os +import socket +import uuid + +from google.cloud.bigtable import __version__ as bigtable_version +from google.cloud.bigtable.data._metrics.handlers._base import MetricsHandler +from google.cloud.bigtable.data._metrics.data_model import OperationType +from google.cloud.bigtable.data._metrics.data_model import DEFAULT_CLUSTER_ID +from google.cloud.bigtable.data._metrics.data_model import DEFAULT_ZONE +from google.cloud.bigtable.data._metrics.data_model import ActiveOperationMetric +from google.cloud.bigtable.data._metrics.data_model import CompletedAttemptMetric +from google.cloud.bigtable.data._metrics.data_model import CompletedOperationMetric + + +class _OpenTelemetryInstruments: + """ + class that holds OpenTelelmetry instrument objects + """ + + def __init__(self, meter_provider=None): + if meter_provider is None: + # use global meter provider + from opentelemetry import metrics + + meter_provider = metrics + # grab meter for this module + meter = meter_provider.get_meter("bigtable.googleapis.com") + # create instruments + self.operation_latencies = meter.create_histogram( + name="operation_latencies", + description=""" + The total end-to-end latency across all RPC attempts associated with a Bigtable operation. + This metric measures an operation's round trip from the client to Bigtable and back to the client and includes all retries. + + For ReadRows requests, the operation latencies include the application processing time for each returned message. + """, + unit="ms", + ) + self.first_response_latencies = meter.create_histogram( + name="first_response_latencies", + description="Latencies from when a client sends a request and receives the first row of the response.", + unit="ms", + ) + self.attempt_latencies = meter.create_histogram( + name="attempt_latencies", + description=""" + The latencies of a client RPC attempt. + + Under normal circumstances, this value is identical to operation_latencies. + If the client receives transient errors, however, then operation_latencies is the sum of all attempt_latencies and the exponential delays. + """, + unit="ms", + ) + self.retry_count = meter.create_counter( + name="retry_count", + description=""" + A counter that records the number of attempts that an operation required to complete. + Under normal circumstances, this value is empty. + """, + ) + self.server_latencies = meter.create_histogram( + name="server_latencies", + description="Latencies between the time when the Google frontend receives an RPC and when it sends the first byte of the response.", + unit="ms", + ) + self.connectivity_error_count = meter.create_counter( + name="connectivity_error_count", + description=""" + The number of requests that failed to reach Google's network. + In normal cases, this number is 0. When the number is not 0, it can indicate connectivity issues between the application and the Google network. + """, + ) + self.application_latencies = meter.create_histogram( + name="application_latencies", + description=""" + The time from when the client receives the response to a request until the application reads the response. + This metric is most relevant for ReadRows requests. + The start and stop times for this metric depend on the way that you send the read request; see Application blocking latencies timer examples for details. + """, + unit="ms", + ) + self.throttling_latencies = meter.create_histogram( + name="throttling_latencies", + description="Latencies introduced when the client blocks the sending of more requests to the server because of too many pending requests in a bulk operation.", + unit="ms", + ) + + +class OpenTelemetryMetricsHandler(MetricsHandler): + """ + Maintains a set of OpenTelemetry metrics for the Bigtable client library, + and updates them with each completed operation and attempt. + + The OpenTelemetry metrics that are tracked are as follows: + - operation_latencies: latency of each client method call, over all of it's attempts. + - first_response_latencies: latency of receiving the first row in a ReadRows operation. + - attempt_latencies: latency of each client attempt RPC. + - retry_count: Number of additional RPCs sent after the initial attempt. + - server_latencies: latency recorded on the server side for each attempt. + - connectivity_error_count: number of attempts that failed to reach Google's network. + - application_latencies: the time spent waiting for the application to process the next response. + - throttling_latencies: latency introduced by waiting when there are too many outstanding requests in a bulk operation. + """ + + def __init__( + self, + *, + project_id: str, + instance_id: str, + table_id: str, + app_profile_id: str | None = None, + client_uid: str | None = None, + instruments: _OpenTelemetryInstruments = _OpenTelemetryInstruments(), + **kwargs, + ): + super().__init__() + self.otel = instruments + # fixed labels sent with each metric update + self.shared_labels = { + "client_name": f"python-bigtable/{bigtable_version}", + "client_uid": client_uid or self._generate_client_uid(), + "resource_project": project_id, + "resource_instance": instance_id, + "resource_table": table_id, + "app_profile": app_profile_id or "default", + } + + @staticmethod + def _generate_client_uid(): + """ + client_uid will take the format `python-@` where uuid is a + random value, pid is the process id, and hostname is the hostname of the machine. + + If not found, localhost will be used in place of hostname, and a random number + will be used in place of pid. + """ + try: + hostname = socket.gethostname() or "localhost" + except Exception: + hostname = "localhost" + try: + pid = os.getpid() or "" + except Exception: + pid = "" + return f"python-{uuid.uuid4()}-{pid}@{hostname}" + + def on_operation_complete(self, op: CompletedOperationMetric) -> None: + """ + Update the metrics associated with a completed operation: + - operation_latencies + - retry_count + """ + labels = { + "method": op.op_type.value, + "status": op.final_status.name, + "resource_zone": op.zone, + "resource_cluster": op.cluster_id, + **self.shared_labels, + } + is_streaming = str(op.is_streaming) + + self.otel.operation_latencies.record( + op.duration_ms, {"streaming": is_streaming, **labels} + ) + # only record completed attempts if there were retries + if op.completed_attempts: + self.otel.retry_count.add(len(op.completed_attempts) - 1, labels) + + def on_attempt_complete( + self, attempt: CompletedAttemptMetric, op: ActiveOperationMetric + ): + """ + Update the metrics associated with a completed attempt: + - attempt_latencies + - first_response_latencies + - server_latencies + - connectivity_error_count + - application_latencies + - throttling_latencies + """ + labels = { + "method": op.op_type.value, + "resource_zone": op.zone or DEFAULT_ZONE, # fallback to default if unset + "resource_cluster": op.cluster_id or DEFAULT_CLUSTER_ID, + **self.shared_labels, + } + status = attempt.end_status.name + is_streaming = str(op.is_streaming) + + self.otel.attempt_latencies.record( + attempt.duration_ms, {"streaming": is_streaming, "status": status, **labels} + ) + combined_throttling = attempt.grpc_throttling_time_ms + if not op.completed_attempts: + # add flow control latency to first attempt's throttling latency + combined_throttling += op.flow_throttling_time_ms + self.otel.throttling_latencies.record(combined_throttling, labels) + self.otel.application_latencies.record( + attempt.application_blocking_time_ms + attempt.backoff_before_attempt_ms, labels + ) + if ( + op.op_type == OperationType.READ_ROWS + and attempt.first_response_latency_ms is not None + ): + self.otel.first_response_latencies.record( + attempt.first_response_latency_ms, {"status": status, **labels} + ) + if attempt.gfe_latency_ms is not None: + self.otel.server_latencies.record( + attempt.gfe_latency_ms, + {"streaming": is_streaming, "status": status, **labels}, + ) + else: + # gfe headers not attached. Record a connectivity error. + # TODO: this should not be recorded as an error when direct path is enabled + self.otel.connectivity_error_count.add( + 1, {"status": status, **labels} + ) \ No newline at end of file From 0fa28bb0d2acca89d4a6a77da6eeb3222b3242df Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 8 Aug 2025 16:02:19 -0700 Subject: [PATCH 02/33] create metrics client using same credentials as bt client --- google/cloud/bigtable/data/_async/client.py | 24 ++++++++++++++----- .../data/_metrics/handlers/gcp_exporter.py | 20 +++++++++------- .../data/_metrics/metrics_controller.py | 5 ---- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/google/cloud/bigtable/data/_async/client.py b/google/cloud/bigtable/data/_async/client.py index d63909282..874e289de 100644 --- a/google/cloud/bigtable/data/_async/client.py +++ b/google/cloud/bigtable/data/_async/client.py @@ -86,6 +86,8 @@ from google.cloud.bigtable.data.row_filters import CellsRowLimitFilter from google.cloud.bigtable.data.row_filters import RowFilterChain from google.cloud.bigtable.data._metrics import BigtableClientSideMetricsController +from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import BigtableMetricsExporter +from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import GoogleCloudMetricsHandler from google.cloud.bigtable.data._cross_sync import CrossSync @@ -212,6 +214,12 @@ def __init__( credentials = google.auth.credentials.AnonymousCredentials() if project is None: project = _DEFAULT_BIGTABLE_EMULATOR_CLIENT + # create a metrics exporter using the same client configuration + self._gcp_metrics_exporter = BigtableMetricsExporter( + credentials=credentials, + project=project, + client_options=client_options, + ) self._metrics_interceptor = MetricInterceptorType() # initialize client ClientWithProject.__init__( @@ -939,13 +947,17 @@ def __init__( self.default_retryable_errors: Sequence[type[Exception]] = ( default_retryable_errors or () ) - self._metrics = BigtableClientSideMetricsController( - client._metrics_interceptor, - project_id=self.client.project, - instance_id=instance_id, - table_id=table_id, - app_profile_id=app_profile_id, + interceptor=client._metrics_interceptor, + handlers=[ + GoogleCloudMetricsHandler( + exporter=client._gcp_metrics_exporter, + project_id=self.client.project, + instance_id=instance_id, + table_id=table_id, + app_profile_id=app_profile_id + ) + ] ) try: diff --git a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py index 27941c5b5..6e845e9fa 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py +++ b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py @@ -99,13 +99,14 @@ class GoogleCloudMetricsHandler(OpenTelemetryMetricsHandler): - throttling_latencies: latency introduced by waiting when there are too many outstanding requests in a bulk operation. Args: - - project_id: The Google Cloud project ID for the associated Bigtable Table + - exporter: The exporter object used to write metrics to Cloud Montitoring. + Should correspond 1:1 with a bigtable client, and share auth configuration - export_interval: The interval (in seconds) at which to export metrics to Cloud Monitoring. + - *args: configuration positional arguments passed down to super class + - *kwargs: configuration keyword arguments passed down to super class """ - def __init__(self, *args, project_id: str, export_interval=60, **kwargs): - # internal exporter to write metrics to Cloud Monitoring - exporter = _BigtableMetricsExporter(project_id=project_id) + def __init__(self, exporter, *args, export_interval=60, **kwargs): # periodically executes exporter gcp_reader = PeriodicExportingMetricReader( exporter, export_interval_millis=export_interval * 1000 @@ -113,10 +114,10 @@ def __init__(self, *args, project_id: str, export_interval=60, **kwargs): # use private meter provider to store instruments and views meter_provider = MeterProvider(metric_readers=[gcp_reader], views=VIEW_LIST) otel = _OpenTelemetryInstruments(meter_provider=meter_provider) - super().__init__(*args, instruments=otel, project_id=project_id, **kwargs) + super().__init__(*args, instruments=otel, project_id=exporter.roject_id, **kwargs) -class _BigtableMetricsExporter(MetricExporter): +class BigtableMetricsExporter(MetricExporter): """ OpenTelemetry Exporter implementation for sending metrics to Google Cloud Monitoring. @@ -130,11 +131,12 @@ class _BigtableMetricsExporter(MetricExporter): - project_id: GCP project id to associate metrics with """ - def __init__(self, project_id: str): + def __init__(self, *client_args, **client_kwargs): super().__init__() - self.client = MetricServiceClient() + self.client = MetricServiceClient(*client_args, **client_kwargs) self.prefix = "bigtable.googleapis.com/internal/client" - self.project_name = self.client.common_project_path(project_id) + self.project_id = self.client.project + self.project_name = self.client.common_project_path(self.project_id) def export( self, metrics_data: MetricsData, timeout_millis: float = 10_000, **kwargs diff --git a/google/cloud/bigtable/data/_metrics/metrics_controller.py b/google/cloud/bigtable/data/_metrics/metrics_controller.py index 169109e28..c0b6ba732 100644 --- a/google/cloud/bigtable/data/_metrics/metrics_controller.py +++ b/google/cloud/bigtable/data/_metrics/metrics_controller.py @@ -40,7 +40,6 @@ def __init__( self, interceptor: AsyncBigtableMetricsInterceptor | BigtableMetricsInterceptor, handlers: list[MetricsHandler] | None = None, - **kwargs, ): """ Initializes the metrics controller. @@ -52,10 +51,6 @@ def __init__( """ self.interceptor = interceptor self.handlers: list[MetricsHandler] = handlers or [] - if handlers is None: - # handlers not given. Use default handlers. - # TODO: add default handlers - pass def add_handler(self, handler: MetricsHandler) -> None: """ From 33cf00e941c44fd7ce59751edc8fbcd98c6ba558 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Mon, 11 Aug 2025 14:00:02 -0700 Subject: [PATCH 03/33] updated aggregation values --- .../cloud/bigtable/data/_metrics/handlers/gcp_exporter.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py index 6e845e9fa..8fee5f9b4 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py +++ b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py @@ -53,10 +53,10 @@ # avoid reformatting into individual lines # fmt: off MILLIS_AGGREGATION = view.ExplicitBucketHistogramAggregation( - [ - 0, 0.01, 0.05, 0.1, 0.3, 0.6, 0.8, 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, 1000, 2000, 5000, 10000, 20000, 50000, 100000, + [ 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 From e10741b30f79da0108af9f6f3bdb783019078654 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 20:40:57 -0700 Subject: [PATCH 04/33] fixed init issues --- google/cloud/bigtable/data/_async/client.py | 5 +++-- .../cloud/bigtable/data/_metrics/handlers/_base.py | 3 +++ .../bigtable/data/_metrics/handlers/gcp_exporter.py | 13 ++++++++----- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/google/cloud/bigtable/data/_async/client.py b/google/cloud/bigtable/data/_async/client.py index 09dda8abd..4fe9de287 100644 --- a/google/cloud/bigtable/data/_async/client.py +++ b/google/cloud/bigtable/data/_async/client.py @@ -215,8 +215,8 @@ def __init__( project = _DEFAULT_BIGTABLE_EMULATOR_CLIENT # create a metrics exporter using the same client configuration self._gcp_metrics_exporter = BigtableMetricsExporter( + project_id=project, credentials=credentials, - project=project, client_options=client_options, ) self._metrics_interceptor = MetricInterceptorType() @@ -981,7 +981,6 @@ def __init__( handlers=[ GoogleCloudMetricsHandler( exporter=client._gcp_metrics_exporter, - project_id=self.client.project, instance_id=instance_id, table_id=table_id, app_profile_id=app_profile_id @@ -1702,6 +1701,8 @@ async def close(self): """ Called to close the Table instance and release any resources held by it. """ + for handler in self._metrics.handlers: + handler.close() if self._register_instance_future: self._register_instance_future.cancel() await self.client._remove_instance_registration(self.instance_id, self) diff --git a/google/cloud/bigtable/data/_metrics/handlers/_base.py b/google/cloud/bigtable/data/_metrics/handlers/_base.py index 72f5aa550..884091fdd 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/_base.py +++ b/google/cloud/bigtable/data/_metrics/handlers/_base.py @@ -33,3 +33,6 @@ def on_attempt_complete( self, attempt: CompletedAttemptMetric, op: ActiveOperationMetric ) -> None: pass + + def close(self): + pass diff --git a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py index 8fee5f9b4..319e15d3a 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py +++ b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py @@ -112,9 +112,12 @@ def __init__(self, exporter, *args, export_interval=60, **kwargs): exporter, export_interval_millis=export_interval * 1000 ) # use private meter provider to store instruments and views - meter_provider = MeterProvider(metric_readers=[gcp_reader], views=VIEW_LIST) - otel = _OpenTelemetryInstruments(meter_provider=meter_provider) - super().__init__(*args, instruments=otel, project_id=exporter.roject_id, **kwargs) + self.meter_provider = MeterProvider(metric_readers=[gcp_reader], views=VIEW_LIST) + otel = _OpenTelemetryInstruments(meter_provider=self.meter_provider) + super().__init__(*args, instruments=otel, project_id=exporter.project_id, **kwargs) + + def close(self): + self.meter_provider.shutdown() class BigtableMetricsExporter(MetricExporter): @@ -131,11 +134,11 @@ class BigtableMetricsExporter(MetricExporter): - project_id: GCP project id to associate metrics with """ - def __init__(self, *client_args, **client_kwargs): + def __init__(self, project_id: str, *client_args, **client_kwargs): super().__init__() self.client = MetricServiceClient(*client_args, **client_kwargs) self.prefix = "bigtable.googleapis.com/internal/client" - self.project_id = self.client.project + self.project_id = project_id self.project_name = self.client.common_project_path(self.project_id) def export( From 1dd87cae613a8f57555406bb9f2faf250f84926e Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 20:42:20 -0700 Subject: [PATCH 05/33] added otel to setup.py --- setup.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/setup.py b/setup.py index 3cb9d465d..91c038b83 100644 --- a/setup.py +++ b/setup.py @@ -39,12 +39,16 @@ dependencies = [ "google-api-core[grpc] >= 2.17.0, <3.0.0", "google-cloud-core >= 1.4.4, <3.0.0", + "google-cloud-monitoring >= 2.0.0, <3.0.0dev", "google-auth >= 2.23.0, <3.0.0,!=2.24.0,!=2.25.0", "grpc-google-iam-v1 >= 0.12.4, <1.0.0", "proto-plus >= 1.22.3, <2.0.0", "proto-plus >= 1.25.0, <2.0.0; python_version>='3.13'", "protobuf>=3.20.2,<7.0.0,!=4.21.0,!=4.21.1,!=4.21.2,!=4.21.3,!=4.21.4,!=4.21.5", "google-crc32c>=1.5.0, <2.0.0dev", + "googleapis-common-protos[grpc] >= 1.57.0, <2.0.0dev", + "opentelemetry-api >= 1.0.0, <2.0.0dev", + "opentelemetry-sdk >= 1.0.0, <2.0.0dev", ] extras = {"libcst": "libcst >= 0.2.5"} From 18afdb77c13e3332e0b46d238c8d9cf0c408cab4 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 20:42:32 -0700 Subject: [PATCH 06/33] added new test file --- tests/system/data/test_metrics_async.py | 94 +++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 tests/system/data/test_metrics_async.py diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py new file mode 100644 index 000000000..7495a8b1b --- /dev/null +++ b/tests/system/data/test_metrics_async.py @@ -0,0 +1,94 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import asyncio +import pytest +import os + +from . import TEST_FAMILY, SystemTestRunner + +from google.cloud.bigtable.data.read_rows_query import ReadRowsQuery + +from google.cloud.bigtable.data._cross_sync import CrossSync + +__CROSS_SYNC_OUTPUT__ = "tests.system.data.test_metrics_autogen" + + +@CrossSync.convert_class(sync_name="TestExportedMetrics") +class TestExportedExportedMetricsAsync(SystemTestRunner): + + @CrossSync.drop + @pytest.fixture(scope="session") + def event_loop(self): + loop = asyncio.get_event_loop() + yield loop + loop.stop() + loop.close() + + def _make_client(self): + project = os.getenv("GOOGLE_CLOUD_PROJECT") or None + return CrossSync.DataClient(project=project) + + @CrossSync.convert + @CrossSync.pytest_fixture(scope="session") + async def client(self): + async with self._make_client() as client: + yield client + + @pytest.fixture(scope="session") + def metrics_client(self, client): + yield client._gcp_metrics_exporter.client + + + @CrossSync.convert + @CrossSync.pytest_fixture(scope="function") + async def temp_rows(self, table): + builder = CrossSync.TempRowBuilder(table) + yield builder + await builder.delete_rows() + + @CrossSync.convert + @CrossSync.pytest_fixture(scope="session") + async def table(self, client, table_id, instance_id): + async with client.get_table(instance_id, table_id) as table: + yield table + + @CrossSync.pytest + async def test_read_rows(self, table, temp_rows, metrics_client): + from datetime import datetime, timedelta, timezone + from google.cloud import monitoring_v3 + + await temp_rows.add_row(b"row_key_1") + await temp_rows.add_row(b"row_key_2") + row_list = await table.read_rows(ReadRowsQuery()) + # read back metrics + + # 1. Define the Time Interval + now = datetime.now(timezone.utc) + # The end time is inclusive + end_time = now + # The start time is exclusive, for an interval (startTime, endTime] + start_time = now - timedelta(minutes=5) + + interval = {"start_time": start_time, "end_time": end_time} + metric_filter = ( + 'metric.type = "bigtable.googleapis.com/client/attempt_latencies" AND metric.labels.client_name != "go-bigtable/1.40.0"' + ) + results = metrics_client.list_time_series( + name=f"projects/{table.client.project}", + filter=metric_filter, + interval=interval, + view=monitoring_v3.ListTimeSeriesRequest.TimeSeriesView.FULL, + ) + print(results) \ No newline at end of file From 8179f97f3149f5d6afa64bfece6bbfab2e14a322 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 21:20:50 -0700 Subject: [PATCH 07/33] fixed metric export --- google/cloud/bigtable/data/_async/client.py | 12 ++++---- .../data/_metrics/handlers/opentelemetry.py | 28 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/google/cloud/bigtable/data/_async/client.py b/google/cloud/bigtable/data/_async/client.py index 305399572..8b413d555 100644 --- a/google/cloud/bigtable/data/_async/client.py +++ b/google/cloud/bigtable/data/_async/client.py @@ -214,12 +214,6 @@ def __init__( credentials = google.auth.credentials.AnonymousCredentials() if project is None: project = _DEFAULT_BIGTABLE_EMULATOR_CLIENT - # create a metrics exporter using the same client configuration - self._gcp_metrics_exporter = BigtableMetricsExporter( - project_id=project, - credentials=credentials, - client_options=client_options, - ) self._metrics_interceptor = MetricInterceptorType() # initialize client ClientWithProject.__init__( @@ -250,6 +244,12 @@ def __init__( "configured the universe domain explicitly, `googleapis.com` " "is the default." ) + # create a metrics exporter using the same client configuration + self._gcp_metrics_exporter = BigtableMetricsExporter( + project_id=self.project, + credentials=credentials, + client_options=client_options, + ) self._is_closed = CrossSync.Event() self.transport = cast(TransportType, self._gapic_client.transport) # keep track of active instances to for warmup on channel refresh diff --git a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py index 7bc5b20f4..884d07806 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py +++ b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py @@ -175,8 +175,15 @@ def on_operation_complete(self, op: CompletedOperationMetric) -> None: is_streaming = str(op.is_streaming) self.otel.operation_latencies.record( - op.duration_ms, {"streaming": is_streaming, **labels} + op.duration_ns / 1e6, {"streaming": is_streaming, **labels} ) + if ( + op.op_type == OperationType.READ_ROWS + and op.first_response_latency_ns is not None + ): + self.otel.first_response_latencies.record( + op.first_response_latency_ns / 1e6, labels + ) # only record completed attempts if there were retries if op.completed_attempts: self.otel.retry_count.add(len(op.completed_attempts) - 1, labels) @@ -203,26 +210,19 @@ def on_attempt_complete( is_streaming = str(op.is_streaming) self.otel.attempt_latencies.record( - attempt.duration_ms, {"streaming": is_streaming, "status": status, **labels} + attempt.duration_ns, {"streaming": is_streaming, "status": status, **labels} ) - combined_throttling = attempt.grpc_throttling_time_ms + combined_throttling = attempt.grpc_throttling_time_ns / 1e6 if not op.completed_attempts: # add flow control latency to first attempt's throttling latency - combined_throttling += op.flow_throttling_time_ms + combined_throttling += (op.flow_throttling_time_ns / 1e6 if op.flow_throttling_time_ns else 0) self.otel.throttling_latencies.record(combined_throttling, labels) self.otel.application_latencies.record( - attempt.application_blocking_time_ms + attempt.backoff_before_attempt_ms, labels + (attempt.application_blocking_time_ns + attempt.backoff_before_attempt_ns) / 1e6, labels ) - if ( - op.op_type == OperationType.READ_ROWS - and attempt.first_response_latency_ms is not None - ): - self.otel.first_response_latencies.record( - attempt.first_response_latency_ms, {"status": status, **labels} - ) - if attempt.gfe_latency_ms is not None: + if attempt.gfe_latency_ns is not None: self.otel.server_latencies.record( - attempt.gfe_latency_ms, + attempt.gfe_latency_ns / 1e6, {"streaming": is_streaming, "status": status, **labels}, ) else: From 71f2125bc09bbb890810c5351ca4a41ce2ccedc7 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 21:21:01 -0700 Subject: [PATCH 08/33] fixed test --- tests/system/data/test_metrics_async.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 7b3991bb2..8e90b669c 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -2190,7 +2190,7 @@ async def test_check_and_mutate_row_failure_unauthorized( @CrossSync.convert_class(sync_name="TestExportedMetrics") -class TestExportedExportedMetricsAsync(SystemTestRunner): +class TestExportedMetricsAsync(SystemTestRunner): @CrossSync.drop @pytest.fixture(scope="session") @@ -2232,6 +2232,7 @@ async def table(self, client, table_id, instance_id): async def test_read_rows(self, table, temp_rows, metrics_client): from datetime import datetime, timedelta, timezone from google.cloud import monitoring_v3 + import google.cloud.bigtable await temp_rows.add_row(b"row_key_1") await temp_rows.add_row(b"row_key_2") @@ -2247,7 +2248,7 @@ async def test_read_rows(self, table, temp_rows, metrics_client): interval = {"start_time": start_time, "end_time": end_time} metric_filter = ( - 'metric.type = "bigtable.googleapis.com/client/attempt_latencies" AND metric.labels.client_name != "go-bigtable/1.40.0"' + f'metric.type = "bigtable.googleapis.com/client/attempt_latencies" AND metric.labels.client_name = "python-bigtable/{google.cloud.bigtable.__version__}"' ) results = metrics_client.list_time_series( name=f"projects/{table.client.project}", From 6ee0c4d1c2c04aca2174c0fbfbd726ac9a06b30d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 21:30:02 -0700 Subject: [PATCH 09/33] apply ordering --- noxfile.py | 1 + tests/system/data/test_metrics_async.py | 1 + 2 files changed, 2 insertions(+) diff --git a/noxfile.py b/noxfile.py index 548bfd0ec..51e412b93 100644 --- a/noxfile.py +++ b/noxfile.py @@ -66,6 +66,7 @@ ] SYSTEM_TEST_EXTERNAL_DEPENDENCIES: List[str] = [ "pytest-asyncio==0.21.2", + "pytest-order==1.3.0", BLACK_VERSION, "pyyaml==6.0.2", ] diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 8e90b669c..b8ab9e1a8 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -2189,6 +2189,7 @@ async def test_check_and_mutate_row_failure_unauthorized( ) +@pytest.mark.order('last') @CrossSync.convert_class(sync_name="TestExportedMetrics") class TestExportedMetricsAsync(SystemTestRunner): From 666ea7496d33c2a72ba5723dcf73c270562fa426 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 21:56:26 -0700 Subject: [PATCH 10/33] fixed time format --- google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py index 884d07806..0b1580738 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py +++ b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py @@ -210,7 +210,7 @@ def on_attempt_complete( is_streaming = str(op.is_streaming) self.otel.attempt_latencies.record( - attempt.duration_ns, {"streaming": is_streaming, "status": status, **labels} + attempt.duration_ns / 1e6, {"streaming": is_streaming, "status": status, **labels} ) combined_throttling = attempt.grpc_throttling_time_ns / 1e6 if not op.completed_attempts: From 2426f500d157a1e678884ffb72b19d81d607342d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 21:56:50 -0700 Subject: [PATCH 11/33] simplified export test --- tests/system/data/test_metrics_async.py | 56 +++++++------------------ 1 file changed, 16 insertions(+), 40 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index b8ab9e1a8..98631cb77 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -2192,69 +2192,45 @@ async def test_check_and_mutate_row_failure_unauthorized( @pytest.mark.order('last') @CrossSync.convert_class(sync_name="TestExportedMetrics") class TestExportedMetricsAsync(SystemTestRunner): + """ + Checks to make sure metrics were exported by tests - @CrossSync.drop - @pytest.fixture(scope="session") - def event_loop(self): - loop = asyncio.get_event_loop() - yield loop - loop.stop() - loop.close() + Runs at the end of test suite, to allow other tests to write metrics + """ - def _make_client(self): + @pytest.fixture(scope="session") + def client(self): + from google.cloud.bigtable.data import BigtableDataClient project = os.getenv("GOOGLE_CLOUD_PROJECT") or None - return CrossSync.DataClient(project=project) - - @CrossSync.convert - @CrossSync.pytest_fixture(scope="session") - async def client(self): - async with self._make_client() as client: + with BigtableDataClient(project=project) as client: yield client @pytest.fixture(scope="session") def metrics_client(self, client): yield client._gcp_metrics_exporter.client - - @CrossSync.convert - @CrossSync.pytest_fixture(scope="function") - async def temp_rows(self, table): - builder = CrossSync.TempRowBuilder(table) - yield builder - await builder.delete_rows() - - @CrossSync.convert - @CrossSync.pytest_fixture(scope="session") - async def table(self, client, table_id, instance_id): - async with client.get_table(instance_id, table_id) as table: - yield table - + @pytest.mark.parametrize("method", ["ReadRows", "MutateRows", "MutateRow", "SampleRowKeys", "CheckAndMutateRow", "ReadModifyWriteRow"]) @CrossSync.pytest - async def test_read_rows(self, table, temp_rows, metrics_client): + async def test_attempt_latency(self, client, metrics_client, method): from datetime import datetime, timedelta, timezone from google.cloud import monitoring_v3 import google.cloud.bigtable - await temp_rows.add_row(b"row_key_1") - await temp_rows.add_row(b"row_key_2") - row_list = await table.read_rows(ReadRowsQuery()) - # read back metrics - # 1. Define the Time Interval now = datetime.now(timezone.utc) # The end time is inclusive end_time = now # The start time is exclusive, for an interval (startTime, endTime] - start_time = now - timedelta(minutes=5) + start_time = now - timedelta(minutes=60) interval = {"start_time": start_time, "end_time": end_time} metric_filter = ( - f'metric.type = "bigtable.googleapis.com/client/attempt_latencies" AND metric.labels.client_name = "python-bigtable/{google.cloud.bigtable.__version__}"' + f'metric.type = "bigtable.googleapis.com/client/attempt_latencies" AND metric.labels.client_name = "python-bigtable/{google.cloud.bigtable.__version__}" AND metric.labels.method = "{method}"' ) - results = metrics_client.list_time_series( - name=f"projects/{table.client.project}", + results = list(metrics_client.list_time_series( + name=f"projects/{client.project}", filter=metric_filter, interval=interval, view=monitoring_v3.ListTimeSeriesRequest.TimeSeriesView.FULL, - ) - print(results) \ No newline at end of file + )) + assert len(results) > 0 \ No newline at end of file From de36f369e08ddf72dff8f0b2dd7677ff5ed1d198 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 30 Sep 2025 22:10:36 -0700 Subject: [PATCH 12/33] use tighter assertions --- tests/system/data/test_metrics_async.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 98631cb77..1ba989268 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -218,7 +218,8 @@ async def temp_rows(self, table): @CrossSync.pytest_fixture(scope="session") async def table(self, client, table_id, instance_id, handler): async with client.get_table(instance_id, table_id) as table: - table._metrics.add_handler(handler) + # override handlers with custom test object + table._metrics.handlers = [handler] yield table @CrossSync.convert @@ -2221,7 +2222,7 @@ async def test_attempt_latency(self, client, metrics_client, method): # The end time is inclusive end_time = now # The start time is exclusive, for an interval (startTime, endTime] - start_time = now - timedelta(minutes=60) + start_time = now - timedelta(minutes=10) interval = {"start_time": start_time, "end_time": end_time} metric_filter = ( From 589c2214a546934a60fd5c5dfeb8658e0ea695d3 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 1 Oct 2025 16:52:33 -0700 Subject: [PATCH 13/33] improved test structure --- tests/system/data/setup_fixtures.py | 9 ++++++ tests/system/data/test_metrics_async.py | 37 ++++++++++++++++--------- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/tests/system/data/setup_fixtures.py b/tests/system/data/setup_fixtures.py index 169e2396b..11d29614f 100644 --- a/tests/system/data/setup_fixtures.py +++ b/tests/system/data/setup_fixtures.py @@ -19,6 +19,7 @@ import pytest import os import uuid +import datetime from . import TEST_FAMILY, TEST_FAMILY_2, TEST_AGGREGATE_FAMILY @@ -86,6 +87,12 @@ def column_split_config(): """ return [(num * 1000).to_bytes(8, "big") for num in range(1, 10)] +@pytest.fixture(scope="session") +def start_timestamp(): + """ + A timestamp taken before any tests are run. Used to fetch back metrics relevant to the tests + """ + return datetime.datetime.now(datetime.timezone.utc) @pytest.fixture(scope="session") def table_id( @@ -95,6 +102,7 @@ def table_id( column_family_config, init_table_id, column_split_config, + start_timestamp, ): """ Returns BIGTABLE_TEST_TABLE if set, otherwise creates a new temporary table for the test session @@ -108,6 +116,7 @@ def table_id( - init_table_id: The table ID to give to the test table, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. Supplied by the init_table_id fixture. - column_split_config: A list of row keys to use as initial splits when creating the test table. + - start_timestamp: accessed when building table to ensure timestamp data is loaded before tests are run """ from google.api_core import exceptions from google.api_core import retry diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 1ba989268..629d70d7c 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -15,6 +15,7 @@ import os import pytest import uuid +import datetime from grpc import StatusCode @@ -25,6 +26,7 @@ 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 @@ -2199,6 +2201,7 @@ class TestExportedMetricsAsync(SystemTestRunner): Runs at the end of test suite, to allow other tests to write metrics """ + @pytest.fixture(scope="session") def client(self): from google.cloud.bigtable.data import BigtableDataClient @@ -2210,28 +2213,36 @@ def client(self): def metrics_client(self, client): yield client._gcp_metrics_exporter.client - @pytest.mark.parametrize("method", ["ReadRows", "MutateRows", "MutateRow", "SampleRowKeys", "CheckAndMutateRow", "ReadModifyWriteRow"]) + @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("method", [m.value for m in OperationType]) @CrossSync.pytest - async def test_attempt_latency(self, client, metrics_client, method): - from datetime import datetime, timedelta, timezone + async def test_attempt_latency(self, client, metrics_client, time_interval, method): from google.cloud import monitoring_v3 import google.cloud.bigtable - # 1. Define the Time Interval - now = datetime.now(timezone.utc) - # The end time is inclusive - end_time = now - # The start time is exclusive, for an interval (startTime, endTime] - start_time = now - timedelta(minutes=10) - - interval = {"start_time": start_time, "end_time": end_time} metric_filter = ( - f'metric.type = "bigtable.googleapis.com/client/attempt_latencies" AND metric.labels.client_name = "python-bigtable/{google.cloud.bigtable.__version__}" AND metric.labels.method = "{method}"' + f'metric.type = "bigtable.googleapis.com/client/attempt_latencies" ' + + f'AND metric.labels.client_name = "python-bigtable/{google.cloud.bigtable.__version__}" ' + + f'AND metric.labels.method = "{method}"' ) results = list(metrics_client.list_time_series( name=f"projects/{client.project}", filter=metric_filter, - interval=interval, + interval=time_interval, view=monitoring_v3.ListTimeSeriesRequest.TimeSeriesView.FULL, )) assert len(results) > 0 \ No newline at end of file From 473f2bca54abf273f9414bf68adc46c7c6cabb13 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 1 Oct 2025 16:55:51 -0700 Subject: [PATCH 14/33] removed imports --- tests/system/data/test_metrics_async.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 629d70d7c..4734b8403 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -30,6 +30,7 @@ ) 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 @@ -2231,18 +2232,15 @@ def time_interval(self, start_timestamp): @pytest.mark.parametrize("method", [m.value for m in OperationType]) @CrossSync.pytest async def test_attempt_latency(self, client, metrics_client, time_interval, method): - from google.cloud import monitoring_v3 - import google.cloud.bigtable - metric_filter = ( f'metric.type = "bigtable.googleapis.com/client/attempt_latencies" ' + - f'AND metric.labels.client_name = "python-bigtable/{google.cloud.bigtable.__version__}" ' + + f'AND metric.labels.client_name = "python-bigtable/{CLIENT_VERSION}" ' + f'AND metric.labels.method = "{method}"' ) results = list(metrics_client.list_time_series( name=f"projects/{client.project}", filter=metric_filter, interval=time_interval, - view=monitoring_v3.ListTimeSeriesRequest.TimeSeriesView.FULL, + view=0, )) assert len(results) > 0 \ No newline at end of file From e9119e7f7311eea8227a9f04b4c5db03430f80ee Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 1 Oct 2025 17:34:19 -0700 Subject: [PATCH 15/33] test other metrics --- tests/system/data/test_metrics_async.py | 37 +++++++++++++++---------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 4734b8403..3fe999207 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -2229,18 +2229,27 @@ def time_interval(self, start_timestamp): return {"start_time": start_timestamp, "end_time": end_time} - @pytest.mark.parametrize("method", [m.value for m in OperationType]) + @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]), + ]) @CrossSync.pytest - async def test_attempt_latency(self, client, metrics_client, time_interval, method): - metric_filter = ( - f'metric.type = "bigtable.googleapis.com/client/attempt_latencies" ' + - f'AND metric.labels.client_name = "python-bigtable/{CLIENT_VERSION}" ' + - f'AND metric.labels.method = "{method}"' - ) - results = list(metrics_client.list_time_series( - name=f"projects/{client.project}", - filter=metric_filter, - interval=time_interval, - view=0, - )) - assert len(results) > 0 \ No newline at end of file + async def test_metric_existence(self, client, metrics_client, time_interval, metric, methods): + for m in methods: + metric_filter = ( + f'metric.type = "bigtable.googleapis.com/client/{metric}" ' + + f'AND metric.labels.client_name = "python-bigtable/{CLIENT_VERSION}" ' + + f'AND metric.labels.method = "{m}"' + ) + results = list(metrics_client.list_time_series( + name=f"projects/{client.project}", + filter=metric_filter, + interval=time_interval, + view=0, + )) + assert len(results) > 0, f"No data found for {metric} {m}" \ No newline at end of file From 018bcc4a8361d09380d7b3379b7f3ec17cb53144 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 1 Oct 2025 17:40:18 -0700 Subject: [PATCH 16/33] remove OK status requirement --- tests/system/data/test_metrics_async.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 3fe999207..7f52b854a 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -2243,8 +2243,7 @@ async def test_metric_existence(self, client, metrics_client, time_interval, met for m in methods: metric_filter = ( f'metric.type = "bigtable.googleapis.com/client/{metric}" ' + - f'AND metric.labels.client_name = "python-bigtable/{CLIENT_VERSION}" ' + - f'AND metric.labels.method = "{m}"' + f'AND metric.labels.client_name = "python-bigtable/{CLIENT_VERSION}" ' ) results = list(metrics_client.list_time_series( name=f"projects/{client.project}", From e5958db35c99ab8d122972d820c15925540e8081 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 10:32:18 -0700 Subject: [PATCH 17/33] test with table --- tests/system/data/test_metrics_async.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 7f52b854a..2daa0a065 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -2239,11 +2239,13 @@ def time_interval(self, start_timestamp): ("application_blocking_latencies", [OperationType.READ_ROWS]), ]) @CrossSync.pytest - async def test_metric_existence(self, client, metrics_client, time_interval, metric, methods): + async def test_metric_existence(self, table_id, client, metrics_client, time_interval, metric, methods): + print(f"using table: {table_id}") for m in methods: metric_filter = ( f'metric.type = "bigtable.googleapis.com/client/{metric}" ' + - f'AND metric.labels.client_name = "python-bigtable/{CLIENT_VERSION}" ' + f'AND metric.labels.client_name = "python-bigtable/{CLIENT_VERSION}" ' + + f'AND resource.labels.table = "{table_id}" ' ) results = list(metrics_client.list_time_series( name=f"projects/{client.project}", From 05ae3ecc7071f775a5be56b78d7d571285d5e79d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 12:25:22 -0700 Subject: [PATCH 18/33] keep project id in exporter --- .../bigtable/data/_metrics/handlers/gcp_exporter.py | 11 +++-------- .../bigtable/data/_metrics/handlers/opentelemetry.py | 2 -- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py index 319e15d3a..520852fe3 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py +++ b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py @@ -114,7 +114,7 @@ def __init__(self, exporter, *args, export_interval=60, **kwargs): # use private meter provider to store instruments and views self.meter_provider = MeterProvider(metric_readers=[gcp_reader], views=VIEW_LIST) otel = _OpenTelemetryInstruments(meter_provider=self.meter_provider) - super().__init__(*args, instruments=otel, project_id=exporter.project_id, **kwargs) + super().__init__(*args, instruments=otel, **kwargs) def close(self): self.meter_provider.shutdown() @@ -139,7 +139,6 @@ def __init__(self, project_id: str, *client_args, **client_kwargs): self.client = MetricServiceClient(*client_args, **client_kwargs) self.prefix = "bigtable.googleapis.com/internal/client" self.project_id = project_id - self.project_name = self.client.common_project_path(self.project_id) def export( self, metrics_data: MetricsData, timeout_millis: float = 10_000, **kwargs @@ -159,14 +158,10 @@ def export( pt for pt in metric.data.data_points if pt.attributes ]: if data_point.attributes: - project_id = data_point.attributes.get("resource_project") - if not isinstance(project_id, str): - # we expect string for project_id field - continue monitored_resource = MonitoredResource( type="bigtable_client_raw", labels={ - "project_id": project_id, + "project_id": self.project_id, "instance": data_point.attributes[ "resource_instance" ], @@ -220,7 +215,7 @@ def _batch_write( # write next batch self.client.create_service_time_series( CreateTimeSeriesRequest( - name=self.project_name, + name=f"projects/{self.project_id}", time_series=series[write_ind : write_ind + max_batch_size], ), timeout=timeout, diff --git a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py index 0b1580738..2a0c8e794 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py +++ b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py @@ -120,7 +120,6 @@ class OpenTelemetryMetricsHandler(MetricsHandler): def __init__( self, *, - project_id: str, instance_id: str, table_id: str, app_profile_id: str | None = None, @@ -134,7 +133,6 @@ def __init__( self.shared_labels = { "client_name": f"python-bigtable/{bigtable_version}", "client_uid": client_uid or self._generate_client_uid(), - "resource_project": project_id, "resource_instance": instance_id, "resource_table": table_id, "app_profile": app_profile_id or "default", From 5ccb57c0d3831a88795b6366844a949ed852eb35 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 13:11:40 -0700 Subject: [PATCH 19/33] removed sync export metrics --- tests/system/data/test_metrics_async.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 2daa0a065..4683f2e10 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -2194,15 +2194,14 @@ async def test_check_and_mutate_row_failure_unauthorized( @pytest.mark.order('last') -@CrossSync.convert_class(sync_name="TestExportedMetrics") -class TestExportedMetricsAsync(SystemTestRunner): +@CrossSync.drop +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 client(self): from google.cloud.bigtable.data import BigtableDataClient @@ -2238,8 +2237,7 @@ def time_interval(self, start_timestamp): ("connectivity_error_count", [m.value for m in OperationType]), ("application_blocking_latencies", [OperationType.READ_ROWS]), ]) - @CrossSync.pytest - async def test_metric_existence(self, table_id, client, metrics_client, time_interval, metric, methods): + def test_metric_existence(self, table_id, client, metrics_client, time_interval, metric, methods): print(f"using table: {table_id}") for m in methods: metric_filter = ( From 65cd359d3add0b82f9ea9bea5c3aa6becf62ff1f Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 14:47:59 -0700 Subject: [PATCH 20/33] moved exported metrics test to main system test class --- .../bigtable/data/_sync_autogen/client.py | 8 +- tests/system/data/__init__.py | 143 +++++++++++++++++- tests/system/data/setup_fixtures.py | 131 ---------------- tests/system/data/test_metrics_async.py | 70 +-------- tests/system/data/test_metrics_autogen.py | 9 +- tests/system/data/test_system_async.py | 71 +++++++++ tests/system/data/test_system_autogen.py | 67 ++++++++ 7 files changed, 294 insertions(+), 205 deletions(-) diff --git a/google/cloud/bigtable/data/_sync_autogen/client.py b/google/cloud/bigtable/data/_sync_autogen/client.py index bc526413d..47282f1a4 100644 --- a/google/cloud/bigtable/data/_sync_autogen/client.py +++ b/google/cloud/bigtable/data/_sync_autogen/client.py @@ -157,9 +157,6 @@ def __init__( credentials = google.auth.credentials.AnonymousCredentials() if project is None: project = _DEFAULT_BIGTABLE_EMULATOR_CLIENT - self._gcp_metrics_exporter = BigtableMetricsExporter( - project_id=project, credentials=credentials, client_options=client_options - ) self._metrics_interceptor = MetricInterceptorType() ClientWithProject.__init__( self, @@ -183,6 +180,11 @@ def __init__( raise ValueError( f"The configured universe domain ({self.universe_domain}) does not match the universe domain found in the credentials ({self._credentials.universe_domain}). If you haven't configured the universe domain explicitly, `googleapis.com` is the default." ) + self._gcp_metrics_exporter = BigtableMetricsExporter( + project_id=self.project, + credentials=credentials, + client_options=client_options, + ) self._is_closed = CrossSync._Sync_Impl.Event() self.transport = cast(TransportType, self._gapic_client.transport) self._active_instances: Set[_WarmedInstanceKey] = set() diff --git a/tests/system/data/__init__.py b/tests/system/data/__init__.py index dcd14c5f9..36c1a0c04 100644 --- a/tests/system/data/__init__.py +++ b/tests/system/data/__init__.py @@ -15,12 +15,19 @@ # import pytest import uuid +import os +import datetime TEST_FAMILY = "test-family" TEST_FAMILY_2 = "test-family-2" TEST_AGGREGATE_FAMILY = "test-aggregate-family" +# authorized view subset to allow all qualifiers +ALLOW_ALL = "" +ALL_QUALIFIERS = {"qualifier_prefixes": [ALLOW_ALL]} + + class SystemTestRunner: """ configures a system test class with configuration for clusters/tables/etc @@ -28,13 +35,6 @@ class SystemTestRunner: used by standard system tests, and metrics tests """ - @pytest.fixture(scope="session") - def init_table_id(self): - """ - The table_id to use when creating a new test table - """ - return f"test-table-{uuid.uuid4().hex}" - @pytest.fixture(scope="session") def cluster_config(self, project_id): """ @@ -68,3 +68,132 @@ def column_family_config(self): value_type=types.Type(aggregate_type=int_aggregate_type) ), } + + @pytest.fixture(scope="session") + def start_timestamp(self): + """ + A timestamp taken before any tests are run. Used to fetch back metrics relevant to the tests + """ + return datetime.datetime.now(datetime.timezone.utc) + + def _generate_table_id(self): + return f"test-table-{uuid.uuid4().hex}" + + @pytest.fixture(scope="session") + def table_id( + self, + admin_client, + project_id, + instance_id, + column_family_config, + init_table_id, + column_split_config, + start_timestamp, + ): + """ + Returns BIGTABLE_TEST_TABLE if set, otherwise creates a new temporary table for the test session + + Args: + - admin_client: Client for interacting with the Table Admin API. Supplied by the admin_client fixture. + - project_id: The project ID of the GCP project to test against. Supplied by the project_id fixture. + - instance_id: The ID of the Bigtable instance to test against. Supplied by the instance_id fixture. + - init_column_families: A list of column families to initialize the table with, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. + Supplied by the init_column_families fixture. + - init_table_id: The table ID to give to the test table, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. + Supplied by the init_table_id fixture. + - column_split_config: A list of row keys to use as initial splits when creating the test table. + - start_timestamp: accessed when building table to ensure timestamp data is loaded before tests are run + """ + from google.api_core import exceptions + from google.api_core import retry + + # use user-specified instance if available + user_specified_table = os.getenv("BIGTABLE_TEST_TABLE") + if user_specified_table: + print("Using user-specified table: {}".format(user_specified_table)) + yield user_specified_table + return + + retry = retry.Retry( + predicate=retry.if_exception_type(exceptions.FailedPrecondition) + ) + try: + parent_path = f"projects/{project_id}/instances/{instance_id}" + print(f"Creating table: {parent_path}/tables/{init_table_id}") + admin_client.table_admin_client.create_table( + request={ + "parent": parent_path, + "table_id": init_table_id, + "table": {"column_families": column_family_config}, + "initial_splits": [{"key": key} for key in column_split_config], + }, + retry=retry, + ) + except exceptions.AlreadyExists: + pass + yield init_table_id + print(f"Deleting table: {parent_path}/tables/{init_table_id}") + try: + admin_client.table_admin_client.delete_table( + name=f"{parent_path}/tables/{init_table_id}" + ) + except exceptions.NotFound: + print(f"Table {init_table_id} not found, skipping deletion") + + @pytest.fixture(scope="session") + def authorized_view_id( + self, + admin_client, + project_id, + instance_id, + table_id, + ): + """ + Creates and returns a new temporary authorized view for the test session + + Args: + - admin_client: Client for interacting with the Table Admin API. Supplied by the admin_client fixture. + - project_id: The project ID of the GCP project to test against. Supplied by the project_id fixture. + - instance_id: The ID of the Bigtable instance to test against. Supplied by the instance_id fixture. + - table_id: The ID of the table to create the authorized view for. Supplied by the table_id fixture. + """ + from google.api_core import exceptions + from google.api_core import retry + + retry = retry.Retry( + predicate=retry.if_exception_type(exceptions.FailedPrecondition) + ) + new_view_id = uuid.uuid4().hex[:8] + parent_path = f"projects/{project_id}/instances/{instance_id}/tables/{table_id}" + new_path = f"{parent_path}/authorizedViews/{new_view_id}" + try: + print(f"Creating view: {new_path}") + admin_client.table_admin_client.create_authorized_view( + request={ + "parent": parent_path, + "authorized_view_id": new_view_id, + "authorized_view": { + "subset_view": { + "row_prefixes": [ALLOW_ALL], + "family_subsets": { + TEST_FAMILY: ALL_QUALIFIERS, + TEST_FAMILY_2: ALL_QUALIFIERS, + TEST_AGGREGATE_FAMILY: ALL_QUALIFIERS, + }, + }, + }, + }, + retry=retry, + ) + except exceptions.AlreadyExists: + pass + except exceptions.MethodNotImplemented: + # will occur when run in emulator. Pass empty id + new_view_id = None + yield new_view_id + if new_view_id: + print(f"Deleting view: {new_path}") + try: + admin_client.table_admin_client.delete_authorized_view(name=new_path) + except exceptions.NotFound: + print(f"View {new_view_id} not found, skipping deletion") \ No newline at end of file diff --git a/tests/system/data/setup_fixtures.py b/tests/system/data/setup_fixtures.py index 11d29614f..7c3d281c2 100644 --- a/tests/system/data/setup_fixtures.py +++ b/tests/system/data/setup_fixtures.py @@ -21,12 +21,6 @@ import uuid import datetime -from . import TEST_FAMILY, TEST_FAMILY_2, TEST_AGGREGATE_FAMILY - -# authorized view subset to allow all qualifiers -ALLOW_ALL = "" -ALL_QUALIFIERS = {"qualifier_prefixes": [ALLOW_ALL]} - @pytest.fixture(scope="session") def admin_client(): @@ -87,131 +81,6 @@ def column_split_config(): """ return [(num * 1000).to_bytes(8, "big") for num in range(1, 10)] -@pytest.fixture(scope="session") -def start_timestamp(): - """ - A timestamp taken before any tests are run. Used to fetch back metrics relevant to the tests - """ - return datetime.datetime.now(datetime.timezone.utc) - -@pytest.fixture(scope="session") -def table_id( - admin_client, - project_id, - instance_id, - column_family_config, - init_table_id, - column_split_config, - start_timestamp, -): - """ - Returns BIGTABLE_TEST_TABLE if set, otherwise creates a new temporary table for the test session - - Args: - - admin_client: Client for interacting with the Table Admin API. Supplied by the admin_client fixture. - - project_id: The project ID of the GCP project to test against. Supplied by the project_id fixture. - - instance_id: The ID of the Bigtable instance to test against. Supplied by the instance_id fixture. - - init_column_families: A list of column families to initialize the table with, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. - Supplied by the init_column_families fixture. - - init_table_id: The table ID to give to the test table, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. - Supplied by the init_table_id fixture. - - column_split_config: A list of row keys to use as initial splits when creating the test table. - - start_timestamp: accessed when building table to ensure timestamp data is loaded before tests are run - """ - from google.api_core import exceptions - from google.api_core import retry - - # use user-specified instance if available - user_specified_table = os.getenv("BIGTABLE_TEST_TABLE") - if user_specified_table: - print("Using user-specified table: {}".format(user_specified_table)) - yield user_specified_table - return - - retry = retry.Retry( - predicate=retry.if_exception_type(exceptions.FailedPrecondition) - ) - try: - parent_path = f"projects/{project_id}/instances/{instance_id}" - print(f"Creating table: {parent_path}/tables/{init_table_id}") - admin_client.table_admin_client.create_table( - request={ - "parent": parent_path, - "table_id": init_table_id, - "table": {"column_families": column_family_config}, - "initial_splits": [{"key": key} for key in column_split_config], - }, - retry=retry, - ) - except exceptions.AlreadyExists: - pass - yield init_table_id - print(f"Deleting table: {parent_path}/tables/{init_table_id}") - try: - admin_client.table_admin_client.delete_table( - name=f"{parent_path}/tables/{init_table_id}" - ) - except exceptions.NotFound: - print(f"Table {init_table_id} not found, skipping deletion") - - -@pytest.fixture(scope="session") -def authorized_view_id( - admin_client, - project_id, - instance_id, - table_id, -): - """ - Creates and returns a new temporary authorized view for the test session - - Args: - - admin_client: Client for interacting with the Table Admin API. Supplied by the admin_client fixture. - - project_id: The project ID of the GCP project to test against. Supplied by the project_id fixture. - - instance_id: The ID of the Bigtable instance to test against. Supplied by the instance_id fixture. - - table_id: The ID of the table to create the authorized view for. Supplied by the table_id fixture. - """ - from google.api_core import exceptions - from google.api_core import retry - - retry = retry.Retry( - predicate=retry.if_exception_type(exceptions.FailedPrecondition) - ) - new_view_id = uuid.uuid4().hex[:8] - parent_path = f"projects/{project_id}/instances/{instance_id}/tables/{table_id}" - new_path = f"{parent_path}/authorizedViews/{new_view_id}" - try: - print(f"Creating view: {new_path}") - admin_client.table_admin_client.create_authorized_view( - request={ - "parent": parent_path, - "authorized_view_id": new_view_id, - "authorized_view": { - "subset_view": { - "row_prefixes": [ALLOW_ALL], - "family_subsets": { - TEST_FAMILY: ALL_QUALIFIERS, - TEST_FAMILY_2: ALL_QUALIFIERS, - TEST_AGGREGATE_FAMILY: ALL_QUALIFIERS, - }, - }, - }, - }, - retry=retry, - ) - except exceptions.AlreadyExists: - pass - except exceptions.MethodNotImplemented: - # will occur when run in emulator. Pass empty id - new_view_id = None - yield new_view_id - if new_view_id: - print(f"Deleting view: {new_path}") - try: - admin_client.table_admin_client.delete_authorized_view(name=new_path) - except exceptions.NotFound: - print(f"View {new_view_id} not found, skipping deletion") - @pytest.fixture(scope="session") def project_id(client): diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 4683f2e10..c0b0ce749 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -217,6 +217,13 @@ async def temp_rows(self, table): yield builder await builder.delete_rows() + @pytest.fixture(scope="session") + def init_table_id(self): + """ + The table_id to use when creating a new test table + """ + return self._generate_table_id() + @CrossSync.convert @CrossSync.pytest_fixture(scope="session") async def table(self, client, table_id, instance_id, handler): @@ -2190,65 +2197,4 @@ async def test_check_and_mutate_row_failure_unauthorized( assert ( attempt.gfe_latency_ns >= 0 and attempt.gfe_latency_ns < operation.duration_ns - ) - - -@pytest.mark.order('last') -@CrossSync.drop -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 client(self): - from google.cloud.bigtable.data import BigtableDataClient - project = os.getenv("GOOGLE_CLOUD_PROJECT") or None - with BigtableDataClient(project=project) as client: - yield client - - @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]), - ]) - def test_metric_existence(self, table_id, client, metrics_client, time_interval, metric, methods): - print(f"using table: {table_id}") - for m in methods: - metric_filter = ( - f'metric.type = "bigtable.googleapis.com/client/{metric}" ' + - f'AND metric.labels.client_name = "python-bigtable/{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=time_interval, - view=0, - )) - assert len(results) > 0, f"No data found for {metric} {m}" \ No newline at end of file + ) \ No newline at end of file diff --git a/tests/system/data/test_metrics_autogen.py b/tests/system/data/test_metrics_autogen.py index fa633db65..aac470c80 100644 --- a/tests/system/data/test_metrics_autogen.py +++ b/tests/system/data/test_metrics_autogen.py @@ -1,4 +1,4 @@ -# Copyright 2024 Google LLC +# Copyright 2025 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -168,10 +168,15 @@ def temp_rows(self, table): yield builder builder.delete_rows() + @pytest.fixture(scope="session") + def init_table_id(self): + """The table_id to use when creating a new test table""" + return self._generate_table_id() + @pytest.fixture(scope="session") def table(self, client, table_id, instance_id, handler): with client.get_table(instance_id, table_id) as table: - table._metrics.add_handler(handler) + table._metrics.handlers = [handler] yield table @pytest.fixture(scope="session") diff --git a/tests/system/data/test_system_async.py b/tests/system/data/test_system_async.py index beea316bb..175e2739f 100644 --- a/tests/system/data/test_system_async.py +++ b/tests/system/data/test_system_async.py @@ -22,6 +22,7 @@ from google.cloud.bigtable.data.execute_query.metadata import SqlType from google.cloud.bigtable.data.read_modify_write_rules import _MAX_INCREMENT_VALUE +from google.cloud.bigtable.data._metrics import OperationType from google.cloud.environment_vars import BIGTABLE_EMULATOR from google.type import date_pb2 @@ -29,6 +30,7 @@ from . import TEST_FAMILY, TEST_FAMILY_2, TEST_AGGREGATE_FAMILY, SystemTestRunner + if CrossSync.is_async: from google.cloud.bigtable_v2.services.bigtable.transports.grpc_asyncio import ( _LoggingClientAIOInterceptor as GapicInterceptor, @@ -162,6 +164,17 @@ def event_loop(self): loop.stop() loop.close() + @pytest.fixture(scope="session") + def init_table_id(self): + """ + The table_id to use when creating a new test table + """ + base_id = self._generate_table_id() + if CrossSync.is_async: + base_id = f"{base_id}-async" + return base_id + + def _make_client(self): project = os.getenv("GOOGLE_CLOUD_PROJECT") or None return CrossSync.DataClient(project=project) @@ -1324,3 +1337,61 @@ async def test_execute_metadata_on_empty_response( assert md[TEST_AGGREGATE_FAMILY].column_type == SqlType.Map( SqlType.Bytes(), SqlType.Int64() ) + + @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}" ' + f'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, + )) + assert len(results) > 0, f"No data found for {metric} {m}" \ No newline at end of file diff --git a/tests/system/data/test_system_autogen.py b/tests/system/data/test_system_autogen.py index 66ca27a66..dab60f8f1 100644 --- a/tests/system/data/test_system_autogen.py +++ b/tests/system/data/test_system_autogen.py @@ -23,6 +23,7 @@ from google.api_core.exceptions import ClientError, PermissionDenied from google.cloud.bigtable.data.execute_query.metadata import SqlType from google.cloud.bigtable.data.read_modify_write_rules import _MAX_INCREMENT_VALUE +from google.cloud.bigtable.data._metrics import OperationType from google.cloud.environment_vars import BIGTABLE_EMULATOR from google.type import date_pb2 from google.cloud.bigtable.data._cross_sync import CrossSync @@ -126,6 +127,12 @@ def create_row_and_mutation( class TestSystem(SystemTestRunner): + @pytest.fixture(scope="session") + def init_table_id(self): + """The table_id to use when creating a new test table""" + base_id = self._generate_table_id() + return base_id + def _make_client(self): project = os.getenv("GOOGLE_CLOUD_PROJECT") or None return CrossSync._Sync_Impl.DataClient(project=project) @@ -1073,3 +1080,63 @@ def test_execute_metadata_on_empty_response( assert md[TEST_AGGREGATE_FAMILY].column_type == SqlType.Map( SqlType.Bytes(), SqlType.Int64() ) + + @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, + ) + ) + assert len(results) > 0, f"No data found for {metric} {m}" From 88fe0b37986ef8715c12e56fc11360bb1f5cb9c4 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:05:58 -0700 Subject: [PATCH 21/33] started otel unit tests --- google/cloud/bigtable/data/_async/client.py | 3 +- .../data/_metrics/handlers/opentelemetry.py | 5 +- tests/unit/data/_async/test_client.py | 31 +++- .../_metrics/test_opentelemetry_handler.py | 146 ++++++++++++++++++ 4 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 tests/unit/data/_metrics/test_opentelemetry_handler.py diff --git a/google/cloud/bigtable/data/_async/client.py b/google/cloud/bigtable/data/_async/client.py index 8b413d555..78390e6b7 100644 --- a/google/cloud/bigtable/data/_async/client.py +++ b/google/cloud/bigtable/data/_async/client.py @@ -984,7 +984,8 @@ def __init__( exporter=client._gcp_metrics_exporter, instance_id=instance_id, table_id=table_id, - app_profile_id=app_profile_id + app_profile_id=app_profile_id, + client_version=client._client_version(), ) ] ) diff --git a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py index 2a0c8e794..3b50e89f7 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py +++ b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py @@ -124,14 +124,15 @@ def __init__( table_id: str, app_profile_id: str | None = None, client_uid: str | None = None, + client_version: str | None = None, instruments: _OpenTelemetryInstruments = _OpenTelemetryInstruments(), - **kwargs, ): super().__init__() self.otel = instruments + client_version = client_version or bigtable_version # fixed labels sent with each metric update self.shared_labels = { - "client_name": f"python-bigtable/{bigtable_version}", + "client_name": f"python-bigtable/{client_version}", "client_uid": client_uid or self._generate_client_uid(), "resource_instance": instance_id, "resource_table": table_id, diff --git a/tests/unit/data/_async/test_client.py b/tests/unit/data/_async/test_client.py index c4b9923db..988c895cf 100644 --- a/tests/unit/data/_async/test_client.py +++ b/tests/unit/data/_async/test_client.py @@ -110,6 +110,10 @@ def _make_client(cls, *args, use_emulator=True, **kwargs): @CrossSync.pytest async 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( @@ -123,6 +127,8 @@ async def test_ctor(self): assert client._channel_refresh_task is not None assert client.transport._credentials == expected_credentials assert isinstance(client._metrics_interceptor, CrossSync.MetricsInterceptor) + assert client._gcp_metrics_exporter is not None + assert isinstance(client._gcp_metrics_exporter, BigtableMetricsExporter) await client.close() @CrossSync.pytest @@ -189,6 +195,22 @@ async def test_ctor_dict_options(self): start_background_refresh.assert_called_once() await client.close() + @CrossSync.pytest + async 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: + async 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) + + @CrossSync.pytest + async def test_metrics_exporter_init_implicit_project(self): + async with self._make_client() as client: + assert client._gcp_metrics_exporter.project_id == client.project + @CrossSync.pytest async def test_veneer_grpc_headers(self): client_component = "data-async" if CrossSync.is_async else "data" @@ -1163,6 +1185,9 @@ async 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" @@ -1205,6 +1230,8 @@ async 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 ( @@ -1495,7 +1522,7 @@ def _make_one( async def test_ctor(self): from google.cloud.bigtable.data._helpers import _WarmedInstanceKey from google.cloud.bigtable.data._metrics import ( - BigtableClientSideMetricsController, + BigtableClientSideMetricsController, GoogleCloudMetricsHandler ) expected_table_id = "table-id" @@ -1546,6 +1573,8 @@ async 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 ( diff --git a/tests/unit/data/_metrics/test_opentelemetry_handler.py b/tests/unit/data/_metrics/test_opentelemetry_handler.py new file mode 100644 index 000000000..2bab769ce --- /dev/null +++ b/tests/unit/data/_metrics/test_opentelemetry_handler.py @@ -0,0 +1,146 @@ +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import pytest +import mock + + +from google.cloud.bigtable.data._metrics.handlers.opentelemetry import _OpenTelemetryInstruments +from google.cloud.bigtable.data._metrics.handlers.opentelemetry import OpenTelemetryMetricsHandler + +class TestOpentelemetryInstruments: + + EXPPECTED_METRICS = [ + "operation_latencies", + "first_response_latencies", + "attempt_latencies", + "server_latencies", + "application_latencies", + "throttling_latencies", + "retry_count", + "connectivity_error_count" + ] + + def _make_one(self, meter_provider=None): + return _OpenTelemetryInstruments(meter_provider) + + def test_meter_name(self): + expected_name = "bigtable.googleapis.com" + mock_meter_provider = mock.Mock() + self._make_one(mock_meter_provider) + mock_meter_provider.get_meter.assert_called_once_with(expected_name) + + @pytest.mark.parametrize("metric_name", [ + m for m in EXPPECTED_METRICS if "latencies" in m + ]) + def test_histogram_creation(self, metric_name): + mock_meter_provider = mock.Mock() + instruments = self._make_one(mock_meter_provider) + mock_meter = mock_meter_provider.get_meter() + assert any([call.kwargs["name"] == metric_name for call in mock_meter.create_histogram.call_args_list]) + assert all([call.kwargs["unit"] == "ms" for call in mock_meter.create_histogram.call_args_list]) + assert all([call.kwargs["description"] is not None for call in mock_meter.create_histogram.call_args_list]) + assert getattr(instruments, metric_name) is not None + + @pytest.mark.parametrize("metric_name", [ + m for m in EXPPECTED_METRICS if "count" in m + ]) + def test_counter_creation(self, metric_name): + mock_meter_provider = mock.Mock() + instruments = self._make_one(mock_meter_provider) + mock_meter = mock_meter_provider.get_meter() + assert any([call.kwargs["name"] == metric_name for call in mock_meter.create_counter.call_args_list]) + assert all([call.kwargs["description"] is not None for call in mock_meter.create_histogram.call_args_list]) + assert getattr(instruments, metric_name) is not None + + def test_global_provider(self): + instruments = self._make_one() + # wait to import otel until after creating instance + import opentelemetry + for metric_name in self.EXPPECTED_METRICS: + metric = getattr(instruments, metric_name) + assert metric is not None + if "latencies" in metric_name: + assert isinstance(metric, opentelemetry.metrics.Histogram) + else: + assert isinstance(metric, opentelemetry.metrics.Counter) + +class TestOpentelemetryMetricsHandler: + + def _make_one(self, **kwargs): + return OpenTelemetryMetricsHandler(**kwargs) + + def test_ctor_defaults(self): + from google.cloud.bigtable import __version__ as CLIENT_VERSION + expected_instance = "my_instance" + expected_table = "my_table" + with mock.patch.object(OpenTelemetryMetricsHandler, "_generate_client_uid") as uid_mock: + handler = self._make_one( + instance_id=expected_instance, + table_id=expected_table + ) + assert isinstance(handler.otel, _OpenTelemetryInstruments) + assert handler.shared_labels["resource_instance"] == expected_instance + assert handler.shared_labels["resource_table"] == expected_table + assert handler.shared_labels["app_profile"] == "default" + assert handler.shared_labels["client_name"] == f"python-bigtable/{CLIENT_VERSION}" + assert handler.shared_labels["client_uid"] == uid_mock() + + def test_ctor_explicit(self): + expected_instance = "my_instance" + expected_table = "my_table" + expected_version = "my_version" + expected_uid = "my_uid" + expected_app_profile = "my_profile" + expected_instruments = object() + handler = self._make_one( + instance_id=expected_instance, + table_id=expected_table, + app_profile_id=expected_app_profile, + client_uid=expected_uid, + client_version=expected_version, + instruments=expected_instruments, + ) + assert handler.otel == expected_instruments + assert handler.shared_labels["resource_instance"] == expected_instance + assert handler.shared_labels["resource_table"] == expected_table + assert handler.shared_labels["app_profile"] == expected_app_profile + assert handler.shared_labels["client_name"] == f"python-bigtable/{expected_version}" + assert handler.shared_labels["client_uid"] == expected_uid + + @mock.patch("socket.gethostname", return_value="hostname") + @mock.patch("os.getpid", return_value="pid") + @mock.patch("uuid.uuid4", return_value="uid") + def test_generate_client_uid_mock(self, socket_mock, os_mock, uuid_mock): + uid = OpenTelemetryMetricsHandler._generate_client_uid() + assert uid == "python-uid-pid@hostname" + + @mock.patch("socket.gethostname", side_effect=[ValueError("fail")]) + @mock.patch("os.getpid", side_effect=[ValueError("fail")]) + @mock.patch("uuid.uuid4", return_value="uid") + def test_generate_client_uid_mock_with_exceptions(self, socket_mock, os_mock, uuid_mock): + uid = OpenTelemetryMetricsHandler._generate_client_uid() + assert uid == "python-uid-@localhost" + + def test_generate_client_uid(self): + import re + uid = OpenTelemetryMetricsHandler._generate_client_uid() + # The expected pattern is python--@ + expected_pattern = r"python-[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}-\d+@.+" + assert re.match(expected_pattern, uid) + + def test_on_operation_complete(self): + pass + + def test_on_attempt_complete(self): + pass \ No newline at end of file From f1ee7999665bbde616a99e5456274325078941e6 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:06:13 -0700 Subject: [PATCH 22/33] fixed comments --- google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py index 3b50e89f7..6baf357fc 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py +++ b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py @@ -163,6 +163,7 @@ def on_operation_complete(self, op: CompletedOperationMetric) -> None: Update the metrics associated with a completed operation: - operation_latencies - retry_count + - first_response_latencies """ labels = { "method": op.op_type.value, @@ -193,7 +194,6 @@ def on_attempt_complete( """ Update the metrics associated with a completed attempt: - attempt_latencies - - first_response_latencies - server_latencies - connectivity_error_count - application_latencies From 17ad4e0280168ae62dab8bdea4f87d1b90df3723 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:06:54 -0700 Subject: [PATCH 23/33] gemini tests --- .../_metrics/test_opentelemetry_handler.py | 255 ++++++++++++++++-- 1 file changed, 226 insertions(+), 29 deletions(-) diff --git a/tests/unit/data/_metrics/test_opentelemetry_handler.py b/tests/unit/data/_metrics/test_opentelemetry_handler.py index 2bab769ce..91a64bf14 100644 --- a/tests/unit/data/_metrics/test_opentelemetry_handler.py +++ b/tests/unit/data/_metrics/test_opentelemetry_handler.py @@ -14,13 +14,24 @@ import pytest import mock +from grpc import StatusCode -from google.cloud.bigtable.data._metrics.handlers.opentelemetry import _OpenTelemetryInstruments -from google.cloud.bigtable.data._metrics.handlers.opentelemetry import OpenTelemetryMetricsHandler +from google.cloud.bigtable.data._metrics.data_model import ( + DEFAULT_CLUSTER_ID, + DEFAULT_ZONE, + ActiveOperationMetric, + CompletedAttemptMetric, + CompletedOperationMetric, + OperationType, +) +from google.cloud.bigtable.data._metrics.handlers.opentelemetry import ( + _OpenTelemetryInstruments, + OpenTelemetryMetricsHandler, +) -class TestOpentelemetryInstruments: - EXPPECTED_METRICS = [ +class TestOpentelemetryInstruments: + EXPECTED_METRICS = [ "operation_latencies", "first_response_latencies", "attempt_latencies", @@ -28,7 +39,7 @@ class TestOpentelemetryInstruments: "application_latencies", "throttling_latencies", "retry_count", - "connectivity_error_count" + "connectivity_error_count", ] def _make_one(self, meter_provider=None): @@ -40,34 +51,58 @@ def test_meter_name(self): self._make_one(mock_meter_provider) mock_meter_provider.get_meter.assert_called_once_with(expected_name) - @pytest.mark.parametrize("metric_name", [ - m for m in EXPPECTED_METRICS if "latencies" in m - ]) + @pytest.mark.parametrize( + "metric_name", [m for m in EXPECTED_METRICS if "latencies" in m] + ) def test_histogram_creation(self, metric_name): mock_meter_provider = mock.Mock() instruments = self._make_one(mock_meter_provider) mock_meter = mock_meter_provider.get_meter() - assert any([call.kwargs["name"] == metric_name for call in mock_meter.create_histogram.call_args_list]) - assert all([call.kwargs["unit"] == "ms" for call in mock_meter.create_histogram.call_args_list]) - assert all([call.kwargs["description"] is not None for call in mock_meter.create_histogram.call_args_list]) + assert any( + [ + call.kwargs["name"] == metric_name + for call in mock_meter.create_histogram.call_args_list + ] + ) + assert all( + [ + call.kwargs["unit"] == "ms" + for call in mock_meter.create_histogram.call_args_list + ] + ) + assert all( + [ + call.kwargs["description"] is not None + for call in mock_meter.create_histogram.call_args_list + ] + ) assert getattr(instruments, metric_name) is not None - @pytest.mark.parametrize("metric_name", [ - m for m in EXPPECTED_METRICS if "count" in m - ]) + @pytest.mark.parametrize("metric_name", [m for m in EXPECTED_METRICS if "count" in m]) def test_counter_creation(self, metric_name): mock_meter_provider = mock.Mock() instruments = self._make_one(mock_meter_provider) mock_meter = mock_meter_provider.get_meter() - assert any([call.kwargs["name"] == metric_name for call in mock_meter.create_counter.call_args_list]) - assert all([call.kwargs["description"] is not None for call in mock_meter.create_histogram.call_args_list]) + assert any( + [ + call.kwargs["name"] == metric_name + for call in mock_meter.create_counter.call_args_list + ] + ) + assert all( + [ + call.kwargs["description"] is not None + for call in mock_meter.create_histogram.call_args_list + ] + ) assert getattr(instruments, metric_name) is not None def test_global_provider(self): instruments = self._make_one() # wait to import otel until after creating instance import opentelemetry - for metric_name in self.EXPPECTED_METRICS: + + for metric_name in self.EXPECTED_METRICS: metric = getattr(instruments, metric_name) assert metric is not None if "latencies" in metric_name: @@ -75,25 +110,29 @@ def test_global_provider(self): else: assert isinstance(metric, opentelemetry.metrics.Counter) -class TestOpentelemetryMetricsHandler: +class TestOpentelemetryMetricsHandler: def _make_one(self, **kwargs): return OpenTelemetryMetricsHandler(**kwargs) def test_ctor_defaults(self): from google.cloud.bigtable import __version__ as CLIENT_VERSION + expected_instance = "my_instance" expected_table = "my_table" - with mock.patch.object(OpenTelemetryMetricsHandler, "_generate_client_uid") as uid_mock: + with mock.patch.object( + OpenTelemetryMetricsHandler, "_generate_client_uid" + ) as uid_mock: handler = self._make_one( - instance_id=expected_instance, - table_id=expected_table + instance_id=expected_instance, table_id=expected_table ) assert isinstance(handler.otel, _OpenTelemetryInstruments) assert handler.shared_labels["resource_instance"] == expected_instance assert handler.shared_labels["resource_table"] == expected_table assert handler.shared_labels["app_profile"] == "default" - assert handler.shared_labels["client_name"] == f"python-bigtable/{CLIENT_VERSION}" + assert ( + handler.shared_labels["client_name"] == f"python-bigtable/{CLIENT_VERSION}" + ) assert handler.shared_labels["client_uid"] == uid_mock() def test_ctor_explicit(self): @@ -115,7 +154,9 @@ def test_ctor_explicit(self): assert handler.shared_labels["resource_instance"] == expected_instance assert handler.shared_labels["resource_table"] == expected_table assert handler.shared_labels["app_profile"] == expected_app_profile - assert handler.shared_labels["client_name"] == f"python-bigtable/{expected_version}" + assert ( + handler.shared_labels["client_name"] == f"python-bigtable/{expected_version}" + ) assert handler.shared_labels["client_uid"] == expected_uid @mock.patch("socket.gethostname", return_value="hostname") @@ -128,19 +169,175 @@ def test_generate_client_uid_mock(self, socket_mock, os_mock, uuid_mock): @mock.patch("socket.gethostname", side_effect=[ValueError("fail")]) @mock.patch("os.getpid", side_effect=[ValueError("fail")]) @mock.patch("uuid.uuid4", return_value="uid") - def test_generate_client_uid_mock_with_exceptions(self, socket_mock, os_mock, uuid_mock): + def test_generate_client_uid_mock_with_exceptions( + self, socket_mock, os_mock, uuid_mock + ): uid = OpenTelemetryMetricsHandler._generate_client_uid() assert uid == "python-uid-@localhost" def test_generate_client_uid(self): import re + uid = OpenTelemetryMetricsHandler._generate_client_uid() # The expected pattern is python--@ - expected_pattern = r"python-[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}-\d+@.+" + expected_pattern = ( + r"python-[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}-\d+@.+" + ) assert re.match(expected_pattern, uid) - def test_on_operation_complete(self): - pass + @pytest.mark.parametrize( + "op_type,is_streaming,first_response_latency_ns,attempts_count", + [ + (OperationType.READ_ROWS, True, 12345, 0), + (OperationType.READ_ROWS, True, None, 2), + (OperationType.MUTATE_ROW, False, None, 3), + ( + OperationType.SAMPLE_ROW_KEYS, + False, + 12345, + 1, + ), # first_response_latency should be ignored + ], + ) + def test_on_operation_complete( + self, op_type, is_streaming, first_response_latency_ns, attempts_count + ): + mock_instruments = mock.Mock( + operation_latencies=mock.Mock(), + first_response_latencies=mock.Mock(), + retry_count=mock.Mock(), + ) + handler = self._make_one( + instance_id="inst", table_id="table", instruments=mock_instruments + ) + attempts = [mock.Mock() for _ in range(attempts_count)] + op = CompletedOperationMetric( + op_type=op_type, + uuid="test-uuid", + duration_ns=1234567, + completed_attempts=attempts, + final_status=StatusCode.OK, + cluster_id="cluster", + zone="zone", + is_streaming=is_streaming, + first_response_latency_ns=first_response_latency_ns, + ) + + handler.on_operation_complete(op) - def test_on_attempt_complete(self): - pass \ No newline at end of file + expected_labels = { + "method": op.op_type.value, + "status": op.final_status.name, + "resource_zone": op.zone, + "resource_cluster": op.cluster_id, + **handler.shared_labels, + } + + # check operation_latencies + mock_instruments.operation_latencies.record.assert_called_once_with( + op.duration_ns / 1e6, + {"streaming": str(is_streaming), **expected_labels}, + ) + + # check first_response_latencies + if ( + op_type == OperationType.READ_ROWS + and first_response_latency_ns is not None + ): + mock_instruments.first_response_latencies.record.assert_called_once_with( + first_response_latency_ns / 1e6, expected_labels + ) + else: + mock_instruments.first_response_latencies.record.assert_not_called() + + # check retry_count + if attempts: + mock_instruments.retry_count.add.assert_called_once_with( + len(attempts) - 1, expected_labels + ) + else: + mock_instruments.retry_count.add.assert_not_called() + + @pytest.mark.parametrize( + "zone,cluster,gfe_latency_ns,is_first_attempt,flow_throttling_ns", + [ + ("zone", "cluster", 12345, True, 54321), + (None, None, None, False, 0), + ("zone", "cluster", 0, True, 0), # gfe_latency_ns is 0 + ], + ) + def test_on_attempt_complete( + self, zone, cluster, gfe_latency_ns, is_first_attempt, flow_throttling_ns + ): + mock_instruments = mock.Mock( + attempt_latencies=mock.Mock(), + throttling_latencies=mock.Mock(), + application_latencies=mock.Mock(), + server_latencies=mock.Mock(), + connectivity_error_count=mock.Mock(), + ) + handler = self._make_one( + instance_id="inst", table_id="table", instruments=mock_instruments + ) + attempt = CompletedAttemptMetric( + duration_ns=1234567, + end_status=StatusCode.OK, + gfe_latency_ns=gfe_latency_ns, + application_blocking_time_ns=234567, + backoff_before_attempt_ns=345678, + grpc_throttling_time_ns=456789, + ) + op = ActiveOperationMetric( + op_type=OperationType.READ_ROWS, + zone=zone, + cluster_id=cluster, + is_streaming=True, + flow_throttling_time_ns=flow_throttling_ns, + ) + if not is_first_attempt: + op.completed_attempts.append(mock.Mock()) + + handler.on_attempt_complete(attempt, op) + + expected_labels = { + "method": op.op_type.value, + "resource_zone": zone or DEFAULT_ZONE, + "resource_cluster": cluster or DEFAULT_CLUSTER_ID, + **handler.shared_labels, + } + status = attempt.end_status.name + is_streaming = str(op.is_streaming) + + # check attempt_latencies + mock_instruments.attempt_latencies.record.assert_called_once_with( + attempt.duration_ns / 1e6, + {"streaming": is_streaming, "status": status, **expected_labels}, + ) + + # check throttling_latencies + expected_throttling = attempt.grpc_throttling_time_ns / 1e6 + if is_first_attempt: + expected_throttling += flow_throttling_ns / 1e6 + mock_instruments.throttling_latencies.record.assert_called_once_with( + pytest.approx(expected_throttling), expected_labels + ) + + # check application_latencies + mock_instruments.application_latencies.record.assert_called_once_with( + (attempt.application_blocking_time_ns + attempt.backoff_before_attempt_ns) + / 1e6, + expected_labels, + ) + + # check server_latencies or connectivity_error_count + if gfe_latency_ns is not None: + mock_instruments.server_latencies.record.assert_called_once_with( + gfe_latency_ns / 1e6, + {"streaming": is_streaming, "status": status, **expected_labels}, + ) + mock_instruments.connectivity_error_count.add.assert_not_called() + else: + mock_instruments.server_latencies.record.assert_not_called() + mock_instruments.connectivity_error_count.add.assert_called_once_with( + 1, {"status": status, **expected_labels} + ) From 8ff32a5f0ba0f041bba7e5776b085facf499ab12 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:14:36 -0700 Subject: [PATCH 24/33] added constant instead of magic number --- .../data/_metrics/handlers/opentelemetry.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py index 6baf357fc..c36c90730 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py +++ b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py @@ -26,6 +26,8 @@ from google.cloud.bigtable.data._metrics.data_model import CompletedAttemptMetric from google.cloud.bigtable.data._metrics.data_model import CompletedOperationMetric +# conversion factor for converting from nanoseconds to milliseconds +NS_TO_MS= 1e6 class _OpenTelemetryInstruments: """ @@ -175,14 +177,14 @@ def on_operation_complete(self, op: CompletedOperationMetric) -> None: is_streaming = str(op.is_streaming) self.otel.operation_latencies.record( - op.duration_ns / 1e6, {"streaming": is_streaming, **labels} + op.duration_ns / NS_TO_MS, {"streaming": is_streaming, **labels} ) if ( op.op_type == OperationType.READ_ROWS and op.first_response_latency_ns is not None ): self.otel.first_response_latencies.record( - op.first_response_latency_ns / 1e6, labels + op.first_response_latency_ns / NS_TO_MS, labels ) # only record completed attempts if there were retries if op.completed_attempts: @@ -209,19 +211,19 @@ def on_attempt_complete( is_streaming = str(op.is_streaming) self.otel.attempt_latencies.record( - attempt.duration_ns / 1e6, {"streaming": is_streaming, "status": status, **labels} + attempt.duration_ns / NS_TO_MS, {"streaming": is_streaming, "status": status, **labels} ) - combined_throttling = attempt.grpc_throttling_time_ns / 1e6 + combined_throttling = attempt.grpc_throttling_time_ns / NS_TO_MS if not op.completed_attempts: # add flow control latency to first attempt's throttling latency - combined_throttling += (op.flow_throttling_time_ns / 1e6 if op.flow_throttling_time_ns else 0) + combined_throttling += (op.flow_throttling_time_ns / NS_TO_MS if op.flow_throttling_time_ns else 0) self.otel.throttling_latencies.record(combined_throttling, labels) self.otel.application_latencies.record( - (attempt.application_blocking_time_ns + attempt.backoff_before_attempt_ns) / 1e6, labels + (attempt.application_blocking_time_ns + attempt.backoff_before_attempt_ns) / NS_TO_MS, labels ) if attempt.gfe_latency_ns is not None: self.otel.server_latencies.record( - attempt.gfe_latency_ns / 1e6, + attempt.gfe_latency_ns / NS_TO_MS, {"streaming": is_streaming, "status": status, **labels}, ) else: From fdc609c1df87c9733dfdf647cc99ac6b6c4323c8 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:15:29 -0700 Subject: [PATCH 25/33] improved gemini tests --- .../_metrics/test_opentelemetry_handler.py | 229 +++++++++++------- 1 file changed, 143 insertions(+), 86 deletions(-) diff --git a/tests/unit/data/_metrics/test_opentelemetry_handler.py b/tests/unit/data/_metrics/test_opentelemetry_handler.py index 91a64bf14..536d9a92f 100644 --- a/tests/unit/data/_metrics/test_opentelemetry_handler.py +++ b/tests/unit/data/_metrics/test_opentelemetry_handler.py @@ -185,46 +185,22 @@ def test_generate_client_uid(self): ) assert re.match(expected_pattern, uid) - @pytest.mark.parametrize( - "op_type,is_streaming,first_response_latency_ns,attempts_count", - [ - (OperationType.READ_ROWS, True, 12345, 0), - (OperationType.READ_ROWS, True, None, 2), - (OperationType.MUTATE_ROW, False, None, 3), - ( - OperationType.SAMPLE_ROW_KEYS, - False, - 12345, - 1, - ), # first_response_latency should be ignored - ], - ) - def test_on_operation_complete( - self, op_type, is_streaming, first_response_latency_ns, attempts_count - ): - mock_instruments = mock.Mock( - operation_latencies=mock.Mock(), - first_response_latencies=mock.Mock(), - retry_count=mock.Mock(), - ) + def test_on_operation_complete_operation_latencies(self): + mock_instruments = mock.Mock(operation_latencies=mock.Mock()) handler = self._make_one( instance_id="inst", table_id="table", instruments=mock_instruments ) - attempts = [mock.Mock() for _ in range(attempts_count)] op = CompletedOperationMetric( - op_type=op_type, + op_type=OperationType.READ_ROWS, uuid="test-uuid", duration_ns=1234567, - completed_attempts=attempts, + completed_attempts=[], final_status=StatusCode.OK, cluster_id="cluster", zone="zone", - is_streaming=is_streaming, - first_response_latency_ns=first_response_latency_ns, + is_streaming=True, ) - handler.on_operation_complete(op) - expected_labels = { "method": op.op_type.value, "status": op.final_status.name, @@ -232,112 +208,193 @@ def test_on_operation_complete( "resource_cluster": op.cluster_id, **handler.shared_labels, } - - # check operation_latencies mock_instruments.operation_latencies.record.assert_called_once_with( op.duration_ns / 1e6, - {"streaming": str(is_streaming), **expected_labels}, + {"streaming": str(op.is_streaming), **expected_labels}, ) - # check first_response_latencies - if ( - op_type == OperationType.READ_ROWS - and first_response_latency_ns is not None - ): + @pytest.mark.parametrize( + "op_type,first_response_latency_ns,should_record", + [ + (OperationType.READ_ROWS, 12345, True), + (OperationType.READ_ROWS, None, False), + (OperationType.MUTATE_ROW, 12345, False), + ], + ) + def test_on_operation_complete_first_response_latencies( + self, op_type, first_response_latency_ns, should_record + ): + mock_instruments = mock.Mock(first_response_latencies=mock.Mock()) + handler = self._make_one( + instance_id="inst", table_id="table", instruments=mock_instruments + ) + op = CompletedOperationMetric( + op_type=op_type, + uuid="test-uuid", + duration_ns=1234567, + completed_attempts=[], + final_status=StatusCode.OK, + cluster_id="cluster", + zone="zone", + is_streaming=True, + first_response_latency_ns=first_response_latency_ns, + ) + handler.on_operation_complete(op) + if should_record: + expected_labels = { + "method": op.op_type.value, + "status": op.final_status.name, + "resource_zone": op.zone, + "resource_cluster": op.cluster_id, + **handler.shared_labels, + } mock_instruments.first_response_latencies.record.assert_called_once_with( first_response_latency_ns / 1e6, expected_labels ) else: mock_instruments.first_response_latencies.record.assert_not_called() - # check retry_count + @pytest.mark.parametrize("attempts_count", [0, 1, 5]) + def test_on_operation_complete_retry_count(self, attempts_count): + mock_instruments = mock.Mock(retry_count=mock.Mock()) + handler = self._make_one( + instance_id="inst", table_id="table", instruments=mock_instruments + ) + attempts = [mock.Mock()] * attempts_count + op = CompletedOperationMetric( + op_type=OperationType.READ_ROWS, + uuid="test-uuid", + duration_ns=1234567, + completed_attempts=attempts, + final_status=StatusCode.OK, + cluster_id="cluster", + zone="zone", + is_streaming=True, + ) + handler.on_operation_complete(op) if attempts: + expected_labels = { + "method": op.op_type.value, + "status": op.final_status.name, + "resource_zone": op.zone, + "resource_cluster": op.cluster_id, + **handler.shared_labels, + } mock_instruments.retry_count.add.assert_called_once_with( len(attempts) - 1, expected_labels ) else: mock_instruments.retry_count.add.assert_not_called() + def test_on_attempt_complete_attempt_latencies(self): + mock_instruments = mock.Mock(attempt_latencies=mock.Mock()) + handler = self._make_one( + instance_id="inst", table_id="table", instruments=mock_instruments + ) + attempt = CompletedAttemptMetric(duration_ns=1234567, end_status=StatusCode.OK) + op = ActiveOperationMetric( + op_type=OperationType.READ_ROWS, + zone="zone", + cluster_id="cluster", + is_streaming=True, + ) + handler.on_attempt_complete(attempt, op) + expected_labels = { + "method": op.op_type.value, + "resource_zone": op.zone, + "resource_cluster": op.cluster_id, + **handler.shared_labels, + } + mock_instruments.attempt_latencies.record.assert_called_once_with( + attempt.duration_ns / 1e6, + { + "streaming": str(op.is_streaming), + "status": attempt.end_status.name, + **expected_labels, + }, + ) + @pytest.mark.parametrize( - "zone,cluster,gfe_latency_ns,is_first_attempt,flow_throttling_ns", - [ - ("zone", "cluster", 12345, True, 54321), - (None, None, None, False, 0), - ("zone", "cluster", 0, True, 0), # gfe_latency_ns is 0 - ], + "is_first_attempt,flow_throttling_ns", + [(True, 54321), (False, 0), (True, 0)], ) - def test_on_attempt_complete( - self, zone, cluster, gfe_latency_ns, is_first_attempt, flow_throttling_ns + def test_on_attempt_complete_throttling_latencies( + self, is_first_attempt, flow_throttling_ns ): - mock_instruments = mock.Mock( - attempt_latencies=mock.Mock(), - throttling_latencies=mock.Mock(), - application_latencies=mock.Mock(), - server_latencies=mock.Mock(), - connectivity_error_count=mock.Mock(), - ) + mock_instruments = mock.Mock(throttling_latencies=mock.Mock()) handler = self._make_one( instance_id="inst", table_id="table", instruments=mock_instruments ) attempt = CompletedAttemptMetric( duration_ns=1234567, end_status=StatusCode.OK, - gfe_latency_ns=gfe_latency_ns, - application_blocking_time_ns=234567, - backoff_before_attempt_ns=345678, grpc_throttling_time_ns=456789, ) op = ActiveOperationMetric( op_type=OperationType.READ_ROWS, - zone=zone, - cluster_id=cluster, - is_streaming=True, flow_throttling_time_ns=flow_throttling_ns, ) if not is_first_attempt: op.completed_attempts.append(mock.Mock()) - handler.on_attempt_complete(attempt, op) - - expected_labels = { - "method": op.op_type.value, - "resource_zone": zone or DEFAULT_ZONE, - "resource_cluster": cluster or DEFAULT_CLUSTER_ID, - **handler.shared_labels, - } - status = attempt.end_status.name - is_streaming = str(op.is_streaming) - - # check attempt_latencies - mock_instruments.attempt_latencies.record.assert_called_once_with( - attempt.duration_ns / 1e6, - {"streaming": is_streaming, "status": status, **expected_labels}, - ) - - # check throttling_latencies expected_throttling = attempt.grpc_throttling_time_ns / 1e6 if is_first_attempt: expected_throttling += flow_throttling_ns / 1e6 mock_instruments.throttling_latencies.record.assert_called_once_with( - pytest.approx(expected_throttling), expected_labels + pytest.approx(expected_throttling), mock.ANY ) - # check application_latencies + def test_on_attempt_complete_application_latencies(self): + mock_instruments = mock.Mock(application_latencies=mock.Mock()) + handler = self._make_one( + instance_id="inst", table_id="table", instruments=mock_instruments + ) + attempt = CompletedAttemptMetric( + duration_ns=1234567, + end_status=StatusCode.OK, + application_blocking_time_ns=234567, + backoff_before_attempt_ns=345678, + ) + op = ActiveOperationMetric(op_type=OperationType.READ_ROWS) + handler.on_attempt_complete(attempt, op) mock_instruments.application_latencies.record.assert_called_once_with( (attempt.application_blocking_time_ns + attempt.backoff_before_attempt_ns) / 1e6, - expected_labels, + mock.ANY, ) - # check server_latencies or connectivity_error_count - if gfe_latency_ns is not None: + @pytest.mark.parametrize( + "gfe_latency_ns,should_record_server_latency", + [(12345, True), (None, False), (0, True)], + ) + def test_on_attempt_complete_server_latencies_and_connectivity_error( + self, gfe_latency_ns, should_record_server_latency + ): + mock_instruments = mock.Mock( + server_latencies=mock.Mock(), connectivity_error_count=mock.Mock() + ) + handler = self._make_one( + instance_id="inst", table_id="table", instruments=mock_instruments + ) + attempt = CompletedAttemptMetric( + duration_ns=1234567, + end_status=StatusCode.OK, + gfe_latency_ns=gfe_latency_ns, + ) + op = ActiveOperationMetric( + op_type=OperationType.READ_ROWS, + zone="zone", + cluster_id="cluster", + is_streaming=True, + ) + handler.on_attempt_complete(attempt, op) + if should_record_server_latency: mock_instruments.server_latencies.record.assert_called_once_with( - gfe_latency_ns / 1e6, - {"streaming": is_streaming, "status": status, **expected_labels}, + gfe_latency_ns / 1e6, mock.ANY ) mock_instruments.connectivity_error_count.add.assert_not_called() else: mock_instruments.server_latencies.record.assert_not_called() mock_instruments.connectivity_error_count.add.assert_called_once_with( - 1, {"status": status, **expected_labels} - ) + 1, mock.ANY + ) \ No newline at end of file From facfb252d81c3f9228c4796f671123dde0ec0e64 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:16:49 -0700 Subject: [PATCH 26/33] fixed lint --- tests/unit/data/_metrics/test_opentelemetry_handler.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/unit/data/_metrics/test_opentelemetry_handler.py b/tests/unit/data/_metrics/test_opentelemetry_handler.py index 536d9a92f..96090b9ed 100644 --- a/tests/unit/data/_metrics/test_opentelemetry_handler.py +++ b/tests/unit/data/_metrics/test_opentelemetry_handler.py @@ -78,7 +78,9 @@ def test_histogram_creation(self, metric_name): ) assert getattr(instruments, metric_name) is not None - @pytest.mark.parametrize("metric_name", [m for m in EXPECTED_METRICS if "count" in m]) + @pytest.mark.parametrize( + "metric_name", [m for m in EXPECTED_METRICS if "count" in m] + ) def test_counter_creation(self, metric_name): mock_meter_provider = mock.Mock() instruments = self._make_one(mock_meter_provider) @@ -155,7 +157,8 @@ def test_ctor_explicit(self): assert handler.shared_labels["resource_table"] == expected_table assert handler.shared_labels["app_profile"] == expected_app_profile assert ( - handler.shared_labels["client_name"] == f"python-bigtable/{expected_version}" + handler.shared_labels["client_name"] + == f"python-bigtable/{expected_version}" ) assert handler.shared_labels["client_uid"] == expected_uid @@ -397,4 +400,4 @@ def test_on_attempt_complete_server_latencies_and_connectivity_error( mock_instruments.server_latencies.record.assert_not_called() mock_instruments.connectivity_error_count.add.assert_called_once_with( 1, mock.ANY - ) \ No newline at end of file + ) From c5a08c57cca545f779c433dd28446b1d71388b40 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:32:01 -0700 Subject: [PATCH 27/33] ran lint --- google/cloud/bigtable/data/_async/client.py | 8 ++- .../data/_metrics/handlers/gcp_exporter.py | 8 ++- .../data/_metrics/handlers/opentelemetry.py | 20 ++++--- tests/system/data/__init__.py | 2 +- tests/system/data/test_metrics_async.py | 2 +- tests/system/data/test_system_async.py | 57 ++++++++++--------- tests/unit/data/_async/test_client.py | 24 +++++--- 7 files changed, 74 insertions(+), 47 deletions(-) diff --git a/google/cloud/bigtable/data/_async/client.py b/google/cloud/bigtable/data/_async/client.py index 78390e6b7..b5d16a429 100644 --- a/google/cloud/bigtable/data/_async/client.py +++ b/google/cloud/bigtable/data/_async/client.py @@ -86,8 +86,12 @@ from google.cloud.bigtable.data.row_filters import CellsRowLimitFilter from google.cloud.bigtable.data.row_filters import RowFilterChain from google.cloud.bigtable.data._metrics import BigtableClientSideMetricsController -from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import BigtableMetricsExporter -from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import GoogleCloudMetricsHandler +from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import ( + BigtableMetricsExporter, +) +from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import ( + GoogleCloudMetricsHandler, +) from google.cloud.bigtable.data._metrics import OperationType from google.cloud.bigtable.data._cross_sync import CrossSync diff --git a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py index 520852fe3..6c1422c46 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py +++ b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py @@ -99,7 +99,7 @@ class GoogleCloudMetricsHandler(OpenTelemetryMetricsHandler): - throttling_latencies: latency introduced by waiting when there are too many outstanding requests in a bulk operation. Args: - - exporter: The exporter object used to write metrics to Cloud Montitoring. + - exporter: The exporter object used to write metrics to Cloud Montitoring. Should correspond 1:1 with a bigtable client, and share auth configuration - export_interval: The interval (in seconds) at which to export metrics to Cloud Monitoring. - *args: configuration positional arguments passed down to super class @@ -112,7 +112,9 @@ def __init__(self, exporter, *args, export_interval=60, **kwargs): exporter, export_interval_millis=export_interval * 1000 ) # use private meter provider to store instruments and views - self.meter_provider = MeterProvider(metric_readers=[gcp_reader], views=VIEW_LIST) + self.meter_provider = MeterProvider( + metric_readers=[gcp_reader], views=VIEW_LIST + ) otel = _OpenTelemetryInstruments(meter_provider=self.meter_provider) super().__init__(*args, instruments=otel, **kwargs) @@ -266,4 +268,4 @@ def force_flush(self, timeout_millis: float = 10_000): Adapted from CloudMonitoringMetricsExporter https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/blob/3668dfe7ce3b80dd01f42af72428de957b58b316/opentelemetry-exporter-gcp-monitoring/src/opentelemetry/exporter/cloud_monitoring/__init__.py#L82 """ - return True \ No newline at end of file + return True diff --git a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py index c36c90730..b5cd92970 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py +++ b/google/cloud/bigtable/data/_metrics/handlers/opentelemetry.py @@ -27,7 +27,8 @@ from google.cloud.bigtable.data._metrics.data_model import CompletedOperationMetric # conversion factor for converting from nanoseconds to milliseconds -NS_TO_MS= 1e6 +NS_TO_MS = 1e6 + class _OpenTelemetryInstruments: """ @@ -211,15 +212,22 @@ def on_attempt_complete( is_streaming = str(op.is_streaming) self.otel.attempt_latencies.record( - attempt.duration_ns / NS_TO_MS, {"streaming": is_streaming, "status": status, **labels} + attempt.duration_ns / NS_TO_MS, + {"streaming": is_streaming, "status": status, **labels}, ) combined_throttling = attempt.grpc_throttling_time_ns / NS_TO_MS if not op.completed_attempts: # add flow control latency to first attempt's throttling latency - combined_throttling += (op.flow_throttling_time_ns / NS_TO_MS if op.flow_throttling_time_ns else 0) + combined_throttling += ( + op.flow_throttling_time_ns / NS_TO_MS + if op.flow_throttling_time_ns + else 0 + ) self.otel.throttling_latencies.record(combined_throttling, labels) self.otel.application_latencies.record( - (attempt.application_blocking_time_ns + attempt.backoff_before_attempt_ns) / NS_TO_MS, labels + (attempt.application_blocking_time_ns + attempt.backoff_before_attempt_ns) + / NS_TO_MS, + labels, ) if attempt.gfe_latency_ns is not None: self.otel.server_latencies.record( @@ -229,6 +237,4 @@ def on_attempt_complete( else: # gfe headers not attached. Record a connectivity error. # TODO: this should not be recorded as an error when direct path is enabled - self.otel.connectivity_error_count.add( - 1, {"status": status, **labels} - ) \ No newline at end of file + self.otel.connectivity_error_count.add(1, {"status": status, **labels}) diff --git a/tests/system/data/__init__.py b/tests/system/data/__init__.py index 36c1a0c04..e099f48c8 100644 --- a/tests/system/data/__init__.py +++ b/tests/system/data/__init__.py @@ -196,4 +196,4 @@ def authorized_view_id( try: admin_client.table_admin_client.delete_authorized_view(name=new_path) except exceptions.NotFound: - print(f"View {new_view_id} not found, skipping deletion") \ No newline at end of file + print(f"View {new_view_id} not found, skipping deletion") diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index c0b0ce749..df9abf34d 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -2197,4 +2197,4 @@ async def test_check_and_mutate_row_failure_unauthorized( assert ( attempt.gfe_latency_ns >= 0 and attempt.gfe_latency_ns < operation.duration_ns - ) \ No newline at end of file + ) diff --git a/tests/system/data/test_system_async.py b/tests/system/data/test_system_async.py index 4b9423ca2..0845f8a0d 100644 --- a/tests/system/data/test_system_async.py +++ b/tests/system/data/test_system_async.py @@ -174,7 +174,6 @@ def init_table_id(self): base_id = f"{base_id}-async" return base_id - def _make_client(self): project = os.getenv("GOOGLE_CLOUD_PROJECT") or None return CrossSync.DataClient(project=project) @@ -1338,7 +1337,7 @@ async def test_execute_metadata_on_empty_response( SqlType.Bytes(), SqlType.Int64() ) - @pytest.mark.order('last') + @pytest.mark.order("last") class TestExportedMetrics(SystemTestRunner): """ Checks to make sure metrics were exported by tests @@ -1361,36 +1360,42 @@ def time_interval(self, start_timestamp): 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)) + 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) + @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]), + ], ) - def test_metric_existence(self, client, table_id, metrics_client, time_interval, metric, methods): + @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 """ 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}" ' + 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=time_interval, + view=0, + ) ) - results = list(metrics_client.list_time_series( - name=f"projects/{client.project}", - filter=metric_filter, - interval=time_interval, - view=0, - )) - assert len(results) > 0, f"No data found for {metric} {m}" \ No newline at end of file + assert len(results) > 0, f"No data found for {metric} {m}" diff --git a/tests/unit/data/_async/test_client.py b/tests/unit/data/_async/test_client.py index 988c895cf..25fce2921 100644 --- a/tests/unit/data/_async/test_client.py +++ b/tests/unit/data/_async/test_client.py @@ -202,9 +202,20 @@ async def test_metrics_exporter_init_shares_arguments(self): 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: - async 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) + with mock.patch( + "google.cloud.bigtable.data._metrics.handlers.gcp_exporter.BigtableMetricsExporter.__init__", + return_value=None, + ) as exporter_mock: + async 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, + ) @CrossSync.pytest async def test_metrics_exporter_init_implicit_project(self): @@ -1185,9 +1196,7 @@ async def test_ctor(self): from google.cloud.bigtable.data._metrics import ( BigtableClientSideMetricsController, ) - from google.cloud.bigtable.data._metrics import ( - GoogleCloudMetricsHandler - ) + from google.cloud.bigtable.data._metrics import GoogleCloudMetricsHandler expected_table_id = "table-id" expected_instance_id = "instance-id" @@ -1522,7 +1531,8 @@ def _make_one( async def test_ctor(self): from google.cloud.bigtable.data._helpers import _WarmedInstanceKey from google.cloud.bigtable.data._metrics import ( - BigtableClientSideMetricsController, GoogleCloudMetricsHandler + BigtableClientSideMetricsController, + GoogleCloudMetricsHandler, ) expected_table_id = "table-id" From 53669f7423c7e993dc403ff938a4a1201709028b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:48:09 -0700 Subject: [PATCH 28/33] simplified metric existance check --- tests/system/data/test_system_async.py | 92 ++++++++++---------------- 1 file changed, 36 insertions(+), 56 deletions(-) diff --git a/tests/system/data/test_system_async.py b/tests/system/data/test_system_async.py index 0845f8a0d..ccac524be 100644 --- a/tests/system/data/test_system_async.py +++ b/tests/system/data/test_system_async.py @@ -1337,65 +1337,45 @@ async 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): + @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 allow other tests to write metrics + Runs at the end of test suite, to let other tests 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 - """ - 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=time_interval, - view=0, - ) + 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}" From c7481b8555b5a62a584a21aa31adb45e6e34f526 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 22:48:22 -0700 Subject: [PATCH 29/33] fixed docstring --- .../data/_metrics/handlers/gcp_exporter.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py index 6c1422c46..9d625ecd4 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py +++ b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py @@ -99,11 +99,11 @@ class GoogleCloudMetricsHandler(OpenTelemetryMetricsHandler): - throttling_latencies: latency introduced by waiting when there are too many outstanding requests in a bulk operation. Args: - - exporter: The exporter object used to write metrics to Cloud Montitoring. + exporter: The exporter object used to write metrics to Cloud Montitoring. Should correspond 1:1 with a bigtable client, and share auth configuration - - export_interval: The interval (in seconds) at which to export metrics to Cloud Monitoring. - - *args: configuration positional arguments passed down to super class - - *kwargs: configuration keyword arguments passed down to super class + export_interval: The interval (in seconds) at which to export metrics to Cloud Monitoring. + *args: configuration positional arguments passed down to super class + *kwargs: configuration keyword arguments passed down to super class """ def __init__(self, exporter, *args, export_interval=60, **kwargs): @@ -133,7 +133,7 @@ class BigtableMetricsExporter(MetricExporter): project_id it is configured with. Args: - - project_id: GCP project id to associate metrics with + project_id: GCP project id to associate metrics with """ def __init__(self, project_id: str, *client_args, **client_kwargs): @@ -205,9 +205,9 @@ def _batch_write( https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/blob/3668dfe7ce3b80dd01f42af72428de957b58b316/opentelemetry-exporter-gcp-monitoring/src/opentelemetry/exporter/cloud_monitoring/__init__.py#L82 Args: - - series: list of TimeSeries to write. Will be split into batches if necessary - - deadline: designates the time.time() at which to stop writing. If None, uses API default - - max_batch_size: maximum number of time series to write at once. + series: list of TimeSeries to write. Will be split into batches if necessary + deadline: designates the time.time() at which to stop writing. If None, uses API default + max_batch_size: maximum number of time series to write at once. Cloud Monitoring allows up to 200 per request """ write_ind = 0 From 9b2cd1cf50a75930bd1d910372464c09d38583f3 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 23:29:53 -0700 Subject: [PATCH 30/33] added tests for gcp_exporter file --- .../_metrics/test_gcp_exporter_handler.py | 421 ++++++++++++++++++ 1 file changed, 421 insertions(+) create mode 100644 tests/unit/data/_metrics/test_gcp_exporter_handler.py diff --git a/tests/unit/data/_metrics/test_gcp_exporter_handler.py b/tests/unit/data/_metrics/test_gcp_exporter_handler.py new file mode 100644 index 000000000..c9dee5961 --- /dev/null +++ b/tests/unit/data/_metrics/test_gcp_exporter_handler.py @@ -0,0 +1,421 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import pytest +import mock + +from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics.export import ( + NumberDataPoint, + HistogramDataPoint, + MetricExportResult, + MetricsData, + ResourceMetrics, + ScopeMetrics, + Metric, + Sum, + Histogram, + AggregationTemporality, +) +from google.cloud.monitoring_v3 import ( + Point, + TimeSeries, +) +from google.api.distribution_pb2 import Distribution + + +from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import ( + BigtableMetricsExporter, +) +from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import ( + GoogleCloudMetricsHandler, +) +from google.cloud.bigtable.data._metrics.handlers.opentelemetry import ( + _OpenTelemetryInstruments, +) + + +class TestGoogleCloudMetricsHandler: + + def _make_one(self, *args, **kwargs): + return GoogleCloudMetricsHandler(*args, **kwargs) + + def test_ctor_defaults(self): + from google.cloud.bigtable import __version__ as CLIENT_VERSION + + expected_instance = "my_instance" + expected_table = "my_table" + expected_exporter = BigtableMetricsExporter("project") + with mock.patch.object( + GoogleCloudMetricsHandler, "_generate_client_uid" + ) as uid_mock: + handler = self._make_one( + expected_exporter, + instance_id=expected_instance, + table_id=expected_table + ) + assert isinstance(handler.meter_provider, MeterProvider) + assert isinstance(handler.otel, _OpenTelemetryInstruments) + assert handler.shared_labels["resource_instance"] == expected_instance + assert handler.shared_labels["resource_table"] == expected_table + assert handler.shared_labels["app_profile"] == "default" + assert ( + handler.shared_labels["client_name"] == f"python-bigtable/{CLIENT_VERSION}" + ) + assert handler.shared_labels["client_uid"] == uid_mock() + + def test_ctor_explicit(self): + expected_instance = "my_instance" + expected_table = "my_table" + 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, + instance_id=expected_instance, + table_id=expected_table, + app_profile_id=expected_app_profile, + client_uid=expected_uid, + client_version=expected_version, + ) + assert handler.shared_labels["resource_instance"] == expected_instance + assert handler.shared_labels["resource_table"] == expected_table + assert handler.shared_labels["app_profile"] == expected_app_profile + assert ( + handler.shared_labels["client_name"] + == f"python-bigtable/{expected_version}" + ) + assert handler.shared_labels["client_uid"] == expected_uid + + + + @mock.patch( + "google.cloud.bigtable.data._metrics.handlers.gcp_exporter.PeriodicExportingMetricReader" + ) + @mock.patch( + "google.cloud.bigtable.data._metrics.handlers.gcp_exporter.MeterProvider" + ) + @mock.patch( + "google.cloud.bigtable.data._metrics.handlers.gcp_exporter._OpenTelemetryInstruments" + ) + @mock.patch( + "google.cloud.bigtable.data._metrics.handlers.gcp_exporter.OpenTelemetryMetricsHandler.__init__" + ) + def test_ctor_with_mocks( + self, mock_super_init, mock_otel_instruments, mock_meter_provider, mock_reader + ): + from google.cloud.bigtable.data._metrics.handlers.gcp_exporter import ( + VIEW_LIST, + ) + + exporter = mock.Mock() + export_interval = 90 + kwargs = {"instance_id": "test_instance", "table_id": "test_table"} + handler = self._make_one(exporter, export_interval=export_interval, **kwargs) + # check PeriodicExportingMetricReader + mock_reader.assert_called_once_with( + exporter, export_interval_millis=export_interval * 1000 + ) + # check MeterProvider + mock_meter_provider.assert_called_once_with( + metric_readers=[mock_reader.return_value], views=VIEW_LIST + ) + # check _OpenTelemetryInstruments + mock_otel_instruments.assert_called_once_with( + meter_provider=mock_meter_provider.return_value + ) + # check super().__init__ call + mock_super_init.assert_called_once_with( + instruments=mock_otel_instruments.return_value, **kwargs + ) + assert handler.meter_provider == mock_meter_provider.return_value + + def test_close(self): + mock_instance = mock.Mock() + assert mock_instance.meter_provider.shutdown.call_count == 0 + GoogleCloudMetricsHandler.close(mock_instance) + assert mock_instance.meter_provider.shutdown.call_count == 1 + + +class TestBigtableMetricsExporter: + def _make_one(self, *args, **kwargs): + return BigtableMetricsExporter(*args, **kwargs) + + def test_ctor_defaults(self): + from google.cloud.monitoring_v3 import MetricServiceClient + + expected_project = "custom" + instance = self._make_one(expected_project) + assert instance.project_id == expected_project + assert instance.prefix == "bigtable.googleapis.com/internal/client" + assert isinstance(instance.client, MetricServiceClient) + + def test_ctor_mocks(self): + expected_project = "custom" + with mock.patch( + "google.cloud.monitoring_v3.MetricServiceClient.__init__", + return_value=None, + ) as mock_client: + args = [mock.Mock(), object()] + kwargs = {"a": "b"} + instance = self._make_one(expected_project, *args, **kwargs) + assert instance.project_id == expected_project + assert instance.prefix == "bigtable.googleapis.com/internal/client" + mock_client.assert_called_once_with(*args, **kwargs) + + @pytest.mark.parametrize( + "value,expected_field", + [ + (123, "int64_value"), + (123.456, "double_value"), + ], + ) + def test__to_point_w_number(self, value, expected_field): + """Test that NumberDataPoint is converted to a Point correctly.""" + instance = self._make_one("project") + 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 + ) + 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 + + def test__to_point_w_histogram(self): + """Test that HistogramDataPoint is converted to a Point correctly.""" + instance = self._make_one("project") + expected_start_time_nanos = 100 + expected_end_time_nanos = 200 + expected_count = 10 + expected_sum = 100.0 + expected_bucket_counts = [1, 2, 7] + expected_explicit_bounds = [10, 20] + dp = HistogramDataPoint( + attributes={}, + start_time_unix_nano=expected_start_time_nanos, + time_unix_nano=expected_end_time_nanos, + count=expected_count, + sum=expected_sum, + bucket_counts=expected_bucket_counts, + explicit_bounds=expected_explicit_bounds, + min=0, + max=50, + ) + point = instance._to_point(dp) + assert isinstance(point, Point) + dist = point.value.distribution_value + assert isinstance(dist, Distribution) + 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 + + def test__to_point_w_histogram_zero_count(self): + """Test that HistogramDataPoint with zero count is converted to a Point correctly.""" + instance = self._make_one("project") + dp = HistogramDataPoint( + attributes={}, + start_time_unix_nano=100, + time_unix_nano=200, + count=0, + sum=0, + bucket_counts=[], + explicit_bounds=[], + min=0, + max=0, + ) + point = instance._to_point(dp) + assert isinstance(point, Point) + dist = point.value.distribution_value + assert isinstance(dist, Distribution) + assert dist.count == 0 + assert dist.mean == 0.0 + + @pytest.mark.parametrize( + "num_series, batch_size, expected_calls, expected_batch_sizes", + [ + (10, 200, 1, [10]), + (200, 200, 1, [200]), + (500, 200, 3, [200, 200, 100]), + (0, 200, 0, []), + ], + ) + def test__batch_write( + self, num_series, batch_size, expected_calls, expected_batch_sizes + ): + """Test that _batch_write splits series into batches correctly.""" + instance = self._make_one("project") + instance.client = mock.Mock() + series = [TimeSeries() for _ in range(num_series)] + instance._batch_write(series, max_batch_size=batch_size) + assert instance.client.create_service_time_series.call_count == expected_calls + for i, call in enumerate( + instance.client.create_service_time_series.call_args_list + ): + call_args, _ = call + assert len(call_args[0].time_series) == expected_batch_sizes[i] + + def test__batch_write_with_deadline(self): + """Test that _batch_write passes deadlines to gapic correctly.""" + import time + from google.api_core import gapic_v1 + + instance = self._make_one("project") + instance.client = mock.Mock() + series = [TimeSeries() for _ in range(10)] + # test with deadline + deadline = time.time() + 10 + instance._batch_write(series, deadline=deadline) + ( + call_args, + call_kwargs, + ) = instance.client.create_service_time_series.call_args_list[0] + assert "timeout" in call_kwargs + assert 9 < call_kwargs["timeout"] < 10 + # test without deadline + instance.client.create_service_time_series.reset_mock() + instance._batch_write(series, deadline=None) + ( + call_args, + call_kwargs, + ) = instance.client.create_service_time_series.call_args_list[0] + assert "timeout" in call_kwargs + assert call_kwargs["timeout"] == gapic_v1.method.DEFAULT + + def test_export(self): + """Test that export correctly converts metrics and calls _batch_write.""" + project_id = "project" + instance = self._make_one(project_id) + instance._batch_write = mock.Mock() + # create mock metrics data + expected_value = 123 + attributes = { + "resource_instance": "instance1", + "resource_cluster": "cluster1", + "resource_table": "table1", + "resource_zone": "zone1", + "method": "ReadRows", + } + data_point = NumberDataPoint( + attributes=attributes, + start_time_unix_nano=100, + time_unix_nano=200, + value=expected_value, + ) + metric = Metric( + name="operation_latencies", + description="", + unit="ms", + data=Sum( + data_points=[data_point], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=False, + ), + ) + scope_metric = ScopeMetrics(scope=mock.Mock(), metrics=[metric], schema_url=None) + resource_metric = ResourceMetrics( + resource=mock.Mock(), scope_metrics=[scope_metric], schema_url=None + ) + metrics_data = MetricsData(resource_metrics=[resource_metric]) + result = instance.export(metrics_data) + assert result == MetricExportResult.SUCCESS + instance._batch_write.assert_called_once() + # check the TimeSeries passed to _batch_write + call_args, call_kwargs = instance._batch_write.call_args_list[0] + series_list = call_args[0] + assert len(series_list) == 1 + series = series_list[0] + assert series.metric.type == f"{instance.prefix}/operation_latencies" + assert series.metric.labels["method"] == "ReadRows" + assert "resource_instance" not in series.metric.labels + assert series.resource.type == "bigtable_client_raw" + assert series.resource.labels["project_id"] == project_id + assert series.resource.labels["instance"] == "instance1" + assert series.resource.labels["cluster"] == "cluster1" + assert series.resource.labels["table"] == "table1" + assert series.resource.labels["zone"] == "zone1" + assert len(series.points) == 1 + point = series.points[0] + assert point.value.int64_value == expected_value + + def test_export_no_attributes(self): + """Test that export skips data points with no attributes.""" + instance = self._make_one("project") + instance._batch_write = mock.Mock() + data_point = NumberDataPoint( + attributes={}, start_time_unix_nano=100, time_unix_nano=200, value=123 + ) + metric = Metric( + name="operation_latencies", + description="", + unit="ms", + data=Sum( + data_points=[data_point], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=False, + ), + ) + scope_metric = ScopeMetrics(scope=mock.Mock(), metrics=[metric], schema_url=None) + resource_metric = ResourceMetrics( + resource=mock.Mock(), scope_metrics=[scope_metric], schema_url=None + ) + metrics_data = MetricsData(resource_metrics=[resource_metric]) + result = instance.export(metrics_data) + assert result == MetricExportResult.SUCCESS + instance._batch_write.assert_called_once() + series_list = instance._batch_write.call_args[0][0] + assert len(series_list) == 0 + + def test_exception_in_export(self): + """ + make sure exceptions don't raise + """ + instance = self._make_one("project") + instance._batch_write = mock.Mock(side_effect=Exception("test")) + # create mock metrics data with one valid data point + attributes = { + "resource_instance": "instance1", + "resource_cluster": "cluster1", + "resource_table": "table1", + "resource_zone": "zone1", + } + data_point = NumberDataPoint( + attributes=attributes, + start_time_unix_nano=100, + time_unix_nano=200, + value=123, + ) + metric = Metric( + name="operation_latencies", + description="", + unit="ms", + data=Sum( + data_points=[data_point], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=False, + ), + ) + scope_metric = ScopeMetrics(scope=mock.Mock(), metrics=[metric], schema_url=None) + resource_metric = ResourceMetrics( + resource=mock.Mock(), scope_metrics=[scope_metric], schema_url=None + ) + metrics_data = MetricsData(resource_metrics=[resource_metric]) + result = instance.export(metrics_data) + assert result == MetricExportResult.FAILURE From 2616894e77c8f3fae494349f62130ce64dda2d88 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 2 Oct 2025 23:43:38 -0700 Subject: [PATCH 31/33] fixed lint --- .../data/_metrics/handlers/gcp_exporter.py | 9 +- .../bigtable/data/_sync_autogen/client.py | 1 + tests/system/data/setup_fixtures.py | 1 - tests/system/data/test_metrics_async.py | 3 - tests/system/data/test_system_autogen.py | 95 ++++++++----------- .../_metrics/test_gcp_exporter_handler.py | 45 ++++++--- .../_metrics/test_opentelemetry_handler.py | 2 - tests/unit/data/_sync_autogen/test_client.py | 37 ++++++++ 8 files changed, 111 insertions(+), 82 deletions(-) diff --git a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py b/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py index 9d625ecd4..99e0dfc24 100644 --- a/google/cloud/bigtable/data/_metrics/handlers/gcp_exporter.py +++ b/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 diff --git a/google/cloud/bigtable/data/_sync_autogen/client.py b/google/cloud/bigtable/data/_sync_autogen/client.py index 47282f1a4..dc81e1ff0 100644 --- a/google/cloud/bigtable/data/_sync_autogen/client.py +++ b/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(), ) ] ) diff --git a/tests/system/data/setup_fixtures.py b/tests/system/data/setup_fixtures.py index 7c3d281c2..a9bb8b14c 100644 --- a/tests/system/data/setup_fixtures.py +++ b/tests/system/data/setup_fixtures.py @@ -19,7 +19,6 @@ import pytest import os import uuid -import datetime @pytest.fixture(scope="session") diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index df9abf34d..3c7bd4fe0 100644 --- a/tests/system/data/test_metrics_async.py +++ b/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 diff --git a/tests/system/data/test_system_autogen.py b/tests/system/data/test_system_autogen.py index dab60f8f1..7ce7cc423 100644 --- a/tests/system/data/test_system_autogen.py +++ b/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}" diff --git a/tests/unit/data/_metrics/test_gcp_exporter_handler.py b/tests/unit/data/_metrics/test_gcp_exporter_handler.py index c9dee5961..bcee9cf03 100644 --- a/tests/unit/data/_metrics/test_gcp_exporter_handler.py +++ b/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 ) diff --git a/tests/unit/data/_metrics/test_opentelemetry_handler.py b/tests/unit/data/_metrics/test_opentelemetry_handler.py index d0d17e439..593947282 100644 --- a/tests/unit/data/_metrics/test_opentelemetry_handler.py +++ b/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, diff --git a/tests/unit/data/_sync_autogen/test_client.py b/tests/unit/data/_sync_autogen/test_client.py index df4ca3675..cd713509b 100644 --- a/tests/unit/data/_sync_autogen/test_client.py +++ b/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 ( From 1fdd7ad5a8b2ad10d1b454d1d430448e45ebc46d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 3 Oct 2025 13:01:31 -0700 Subject: [PATCH 32/33] moved back some fixtures --- tests/system/data/__init__.py | 127 +--------------------- tests/system/data/setup_fixtures.py | 123 +++++++++++++++++++++ tests/system/data/test_metrics_async.py | 7 -- tests/system/data/test_metrics_autogen.py | 5 - tests/system/data/test_system_async.py | 14 +-- tests/system/data/test_system_autogen.py | 10 +- 6 files changed, 132 insertions(+), 154 deletions(-) diff --git a/tests/system/data/__init__.py b/tests/system/data/__init__.py index e099f48c8..896944090 100644 --- a/tests/system/data/__init__.py +++ b/tests/system/data/__init__.py @@ -23,11 +23,6 @@ TEST_AGGREGATE_FAMILY = "test-aggregate-family" -# authorized view subset to allow all qualifiers -ALLOW_ALL = "" -ALL_QUALIFIERS = {"qualifier_prefixes": [ALLOW_ALL]} - - class SystemTestRunner: """ configures a system test class with configuration for clusters/tables/etc @@ -76,124 +71,12 @@ def start_timestamp(self): """ return datetime.datetime.now(datetime.timezone.utc) - def _generate_table_id(self): - return f"test-table-{uuid.uuid4().hex}" - - @pytest.fixture(scope="session") - def table_id( - self, - admin_client, - project_id, - instance_id, - column_family_config, - init_table_id, - column_split_config, - start_timestamp, - ): - """ - Returns BIGTABLE_TEST_TABLE if set, otherwise creates a new temporary table for the test session - - Args: - - admin_client: Client for interacting with the Table Admin API. Supplied by the admin_client fixture. - - project_id: The project ID of the GCP project to test against. Supplied by the project_id fixture. - - instance_id: The ID of the Bigtable instance to test against. Supplied by the instance_id fixture. - - init_column_families: A list of column families to initialize the table with, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. - Supplied by the init_column_families fixture. - - init_table_id: The table ID to give to the test table, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. - Supplied by the init_table_id fixture. - - column_split_config: A list of row keys to use as initial splits when creating the test table. - - start_timestamp: accessed when building table to ensure timestamp data is loaded before tests are run - """ - from google.api_core import exceptions - from google.api_core import retry - - # use user-specified instance if available - user_specified_table = os.getenv("BIGTABLE_TEST_TABLE") - if user_specified_table: - print("Using user-specified table: {}".format(user_specified_table)) - yield user_specified_table - return - - retry = retry.Retry( - predicate=retry.if_exception_type(exceptions.FailedPrecondition) - ) - try: - parent_path = f"projects/{project_id}/instances/{instance_id}" - print(f"Creating table: {parent_path}/tables/{init_table_id}") - admin_client.table_admin_client.create_table( - request={ - "parent": parent_path, - "table_id": init_table_id, - "table": {"column_families": column_family_config}, - "initial_splits": [{"key": key} for key in column_split_config], - }, - retry=retry, - ) - except exceptions.AlreadyExists: - pass - yield init_table_id - print(f"Deleting table: {parent_path}/tables/{init_table_id}") - try: - admin_client.table_admin_client.delete_table( - name=f"{parent_path}/tables/{init_table_id}" - ) - except exceptions.NotFound: - print(f"Table {init_table_id} not found, skipping deletion") - @pytest.fixture(scope="session") - def authorized_view_id( - self, - admin_client, - project_id, - instance_id, - table_id, - ): + def init_table_id(self, start_timestamp): """ - Creates and returns a new temporary authorized view for the test session + The table_id to use when creating a new test table - Args: - - admin_client: Client for interacting with the Table Admin API. Supplied by the admin_client fixture. - - project_id: The project ID of the GCP project to test against. Supplied by the project_id fixture. - - instance_id: The ID of the Bigtable instance to test against. Supplied by the instance_id fixture. - - table_id: The ID of the table to create the authorized view for. Supplied by the table_id fixture. + Args + start_timestamp: accessed when building table to ensure timestamp data is loaded before tests are run """ - from google.api_core import exceptions - from google.api_core import retry - - retry = retry.Retry( - predicate=retry.if_exception_type(exceptions.FailedPrecondition) - ) - new_view_id = uuid.uuid4().hex[:8] - parent_path = f"projects/{project_id}/instances/{instance_id}/tables/{table_id}" - new_path = f"{parent_path}/authorizedViews/{new_view_id}" - try: - print(f"Creating view: {new_path}") - admin_client.table_admin_client.create_authorized_view( - request={ - "parent": parent_path, - "authorized_view_id": new_view_id, - "authorized_view": { - "subset_view": { - "row_prefixes": [ALLOW_ALL], - "family_subsets": { - TEST_FAMILY: ALL_QUALIFIERS, - TEST_FAMILY_2: ALL_QUALIFIERS, - TEST_AGGREGATE_FAMILY: ALL_QUALIFIERS, - }, - }, - }, - }, - retry=retry, - ) - except exceptions.AlreadyExists: - pass - except exceptions.MethodNotImplemented: - # will occur when run in emulator. Pass empty id - new_view_id = None - yield new_view_id - if new_view_id: - print(f"Deleting view: {new_path}") - try: - admin_client.table_admin_client.delete_authorized_view(name=new_path) - except exceptions.NotFound: - print(f"View {new_view_id} not found, skipping deletion") + return f"test-table-{uuid.uuid4().hex}" diff --git a/tests/system/data/setup_fixtures.py b/tests/system/data/setup_fixtures.py index a9bb8b14c..bdc43219b 100644 --- a/tests/system/data/setup_fixtures.py +++ b/tests/system/data/setup_fixtures.py @@ -20,6 +20,12 @@ import os import uuid +from . import TEST_FAMILY, TEST_FAMILY_2, TEST_AGGREGATE_FAMILY + +# authorized view subset to allow all qualifiers +ALLOW_ALL = "" +ALL_QUALIFIERS = {"qualifier_prefixes": [ALLOW_ALL]} + @pytest.fixture(scope="session") def admin_client(): @@ -81,6 +87,123 @@ def column_split_config(): return [(num * 1000).to_bytes(8, "big") for num in range(1, 10)] +@pytest.fixture(scope="session") +def table_id( + admin_client, + project_id, + instance_id, + column_family_config, + init_table_id, + column_split_config, +): + """ + Returns BIGTABLE_TEST_TABLE if set, otherwise creates a new temporary table for the test session + + Args: + admin_client: Client for interacting with the Table Admin API. Supplied by the admin_client fixture. + project_id: The project ID of the GCP project to test against. Supplied by the project_id fixture. + instance_id: The ID of the Bigtable instance to test against. Supplied by the instance_id fixture. + init_column_families: A list of column families to initialize the table with, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. + Supplied by the init_column_families fixture. + init_table_id: The table ID to give to the test table, if pre-initialized table is not given with BIGTABLE_TEST_TABLE. + Supplied by the init_table_id fixture. + column_split_config: A list of row keys to use as initial splits when creating the test table. + """ + from google.api_core import exceptions + from google.api_core import retry + + # use user-specified instance if available + user_specified_table = os.getenv("BIGTABLE_TEST_TABLE") + if user_specified_table: + print("Using user-specified table: {}".format(user_specified_table)) + yield user_specified_table + return + + retry = retry.Retry( + predicate=retry.if_exception_type(exceptions.FailedPrecondition) + ) + try: + parent_path = f"projects/{project_id}/instances/{instance_id}" + print(f"Creating table: {parent_path}/tables/{init_table_id}") + admin_client.table_admin_client.create_table( + request={ + "parent": parent_path, + "table_id": init_table_id, + "table": {"column_families": column_family_config}, + "initial_splits": [{"key": key} for key in column_split_config], + }, + retry=retry, + ) + except exceptions.AlreadyExists: + pass + yield init_table_id + print(f"Deleting table: {parent_path}/tables/{init_table_id}") + try: + admin_client.table_admin_client.delete_table( + name=f"{parent_path}/tables/{init_table_id}" + ) + except exceptions.NotFound: + print(f"Table {init_table_id} not found, skipping deletion") + + +@pytest.fixture(scope="session") +def authorized_view_id( + admin_client, + project_id, + instance_id, + table_id, +): + """ + Creates and returns a new temporary authorized view for the test session + + Args: + admin_client: Client for interacting with the Table Admin API. Supplied by the admin_client fixture. + project_id: The project ID of the GCP project to test against. Supplied by the project_id fixture. + instance_id: The ID of the Bigtable instance to test against. Supplied by the instance_id fixture. + table_id: The ID of the table to create the authorized view for. Supplied by the table_id fixture. + """ + from google.api_core import exceptions + from google.api_core import retry + + retry = retry.Retry( + predicate=retry.if_exception_type(exceptions.FailedPrecondition) + ) + new_view_id = uuid.uuid4().hex[:8] + parent_path = f"projects/{project_id}/instances/{instance_id}/tables/{table_id}" + new_path = f"{parent_path}/authorizedViews/{new_view_id}" + try: + print(f"Creating view: {new_path}") + admin_client.table_admin_client.create_authorized_view( + request={ + "parent": parent_path, + "authorized_view_id": new_view_id, + "authorized_view": { + "subset_view": { + "row_prefixes": [ALLOW_ALL], + "family_subsets": { + TEST_FAMILY: ALL_QUALIFIERS, + TEST_FAMILY_2: ALL_QUALIFIERS, + TEST_AGGREGATE_FAMILY: ALL_QUALIFIERS, + }, + }, + }, + }, + retry=retry, + ) + except exceptions.AlreadyExists: + pass + except exceptions.MethodNotImplemented: + # will occur when run in emulator. Pass empty id + new_view_id = None + yield new_view_id + if new_view_id: + print(f"Deleting view: {new_path}") + try: + admin_client.table_admin_client.delete_authorized_view(name=new_path) + except exceptions.NotFound: + print(f"View {new_view_id} not found, skipping deletion") + + @pytest.fixture(scope="session") def project_id(client): """Returns the project ID from the client.""" diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 3c7bd4fe0..ea811860e 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -214,13 +214,6 @@ async def temp_rows(self, table): yield builder await builder.delete_rows() - @pytest.fixture(scope="session") - def init_table_id(self): - """ - The table_id to use when creating a new test table - """ - return self._generate_table_id() - @CrossSync.convert @CrossSync.pytest_fixture(scope="session") async def table(self, client, table_id, instance_id, handler): diff --git a/tests/system/data/test_metrics_autogen.py b/tests/system/data/test_metrics_autogen.py index aac470c80..8d7baa225 100644 --- a/tests/system/data/test_metrics_autogen.py +++ b/tests/system/data/test_metrics_autogen.py @@ -168,11 +168,6 @@ def temp_rows(self, table): yield builder builder.delete_rows() - @pytest.fixture(scope="session") - def init_table_id(self): - """The table_id to use when creating a new test table""" - return self._generate_table_id() - @pytest.fixture(scope="session") def table(self, client, table_id, instance_id, handler): with client.get_table(instance_id, table_id) as table: diff --git a/tests/system/data/test_system_async.py b/tests/system/data/test_system_async.py index ccac524be..dc40ba016 100644 --- a/tests/system/data/test_system_async.py +++ b/tests/system/data/test_system_async.py @@ -18,7 +18,7 @@ import uuid import os from google.api_core import retry -from google.api_core.exceptions import ClientError, PermissionDenied +from google.api_core.exceptions import ClientError, PermissionDenied, ServerError from google.cloud.bigtable.data.execute_query.metadata import SqlType from google.cloud.bigtable.data.read_modify_write_rules import _MAX_INCREMENT_VALUE @@ -164,16 +164,6 @@ def event_loop(self): loop.stop() loop.close() - @pytest.fixture(scope="session") - def init_table_id(self): - """ - The table_id to use when creating a new test table - """ - base_id = self._generate_table_id() - if CrossSync.is_async: - base_id = f"{base_id}-async" - return base_id - def _make_client(self): project = os.getenv("GOOGLE_CLOUD_PROJECT") or None return CrossSync.DataClient(project=project) @@ -1354,7 +1344,7 @@ def metrics_client(self, client): ("application_blocking_latencies", [OperationType.READ_ROWS]), ], ) - @retry.Retry(predicate=retry.if_exception_type(AssertionError)) + @retry.Retry(predicate=retry.if_exception_type(AssertionError, ServerError)) def test_metric_existence( self, client, table_id, metrics_client, start_timestamp, metric, methods ): diff --git a/tests/system/data/test_system_autogen.py b/tests/system/data/test_system_autogen.py index 7ce7cc423..127e401be 100644 --- a/tests/system/data/test_system_autogen.py +++ b/tests/system/data/test_system_autogen.py @@ -20,7 +20,7 @@ import uuid import os from google.api_core import retry -from google.api_core.exceptions import ClientError, PermissionDenied +from google.api_core.exceptions import ClientError, PermissionDenied, ServerError from google.cloud.bigtable.data.execute_query.metadata import SqlType from google.cloud.bigtable.data.read_modify_write_rules import _MAX_INCREMENT_VALUE from google.cloud.bigtable.data._metrics import OperationType @@ -127,12 +127,6 @@ def create_row_and_mutation( class TestSystem(SystemTestRunner): - @pytest.fixture(scope="session") - def init_table_id(self): - """The table_id to use when creating a new test table""" - base_id = self._generate_table_id() - return base_id - def _make_client(self): project = os.getenv("GOOGLE_CLOUD_PROJECT") or None return CrossSync._Sync_Impl.DataClient(project=project) @@ -1098,7 +1092,7 @@ def metrics_client(self, client): ("application_blocking_latencies", [OperationType.READ_ROWS]), ], ) - @retry.Retry(predicate=retry.if_exception_type(AssertionError)) + @retry.Retry(predicate=retry.if_exception_type(AssertionError, ServerError)) def test_metric_existence( self, client, table_id, metrics_client, start_timestamp, metric, methods ): From 694e04287b3d6f116fde919ed9cb64d11b421701 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 3 Oct 2025 15:03:24 -0700 Subject: [PATCH 33/33] fixed event loop error --- tests/system/data/test_metrics_async.py | 8 -------- tests/system/data/test_system_async.py | 8 -------- 2 files changed, 16 deletions(-) diff --git a/tests/system/data/test_metrics_async.py b/tests/system/data/test_metrics_async.py index 4beda84cf..0d84777a5 100644 --- a/tests/system/data/test_metrics_async.py +++ b/tests/system/data/test_metrics_async.py @@ -135,14 +135,6 @@ def __getattr__(self, name): @CrossSync.convert_class(sync_name="TestMetrics") class TestMetricsAsync(SystemTestRunner): - @CrossSync.drop - @pytest.fixture(scope="session") - def event_loop(self): - loop = asyncio.get_event_loop() - yield loop - loop.stop() - loop.close() - def _make_client(self): project = os.getenv("GOOGLE_CLOUD_PROJECT") or None return CrossSync.DataClient(project=project) diff --git a/tests/system/data/test_system_async.py b/tests/system/data/test_system_async.py index dc40ba016..100de9e3d 100644 --- a/tests/system/data/test_system_async.py +++ b/tests/system/data/test_system_async.py @@ -156,14 +156,6 @@ async def create_row_and_mutation( @CrossSync.convert_class(sync_name="TestSystem") class TestSystemAsync(SystemTestRunner): - @CrossSync.drop - @pytest.fixture(scope="session") - def event_loop(self): - loop = asyncio.get_event_loop() - yield loop - loop.stop() - loop.close() - def _make_client(self): project = os.getenv("GOOGLE_CLOUD_PROJECT") or None return CrossSync.DataClient(project=project)