Skip to content

Commit

Permalink
Review changes
Browse files Browse the repository at this point in the history 
- Added the MetricTimeSeries dataclass to put query results in a more organized way
- Added pretty_print() method to the MetricTimeSeries dataclass
- Now when we access all_results[metric] we know we are getting a MetricTimeSeries object back
  • Loading branch information
@stanley-cheung
stanley-cheung committed Feb 1, 2024
1 parent c25bfb2 commit 00414b1
Showing 1 changed file with 68 additions and 41 deletions.
109 changes: 68 additions & 41 deletions tests/gamma/csm_observability_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# 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 dataclasses
import logging
import time
from typing import Callable, Iterable
Expand All @@ -19,6 +20,7 @@
from absl import flags
from absl.testing import absltest
from google.cloud import monitoring_v3
import yaml

from framework import xds_gamma_testcase
from framework import xds_k8s_testcase
Expand Down Expand Up @@ -89,6 +91,34 @@
ALL_METRICS = HISTOGRAM_METRICS + COUNTER_METRICS


@dataclasses.dataclass(eq=False)
class MetricTimeSeries:
name: str
metric_labels: dict[str, str]
resource_labels: dict[str, str]
resource_type: str
points: list[monitoring_v3.types.Point]

@classmethod
def from_response(
cls,
name: str,
response: monitoring_v3.types.TimeSeries,
) -> "MetricTimeSeries":
return cls(
name=name,
metric_labels=dict(response.metric.labels),
resource_labels=dict(response.resource.labels),
resource_type=response.resource.type,
points=list(response.points),
)

def pretty_print(self):
metric = dataclasses.asdict(self)
metric.pop("points")
logger.info(yaml.dump(metric, sort_keys=True))


class CsmObservabilityTest(xds_gamma_testcase.GammaXdsKubernetesTestCase):
metric_client: monitoring_v3.MetricServiceClient

Expand All @@ -107,7 +137,7 @@ def setUpClass(cls):
BuildQueryFn = Callable[[str], str]

@classmethod
def _build_histogram_query(cls, metric_type: str) -> str:
def build_histogram_query(cls, metric_type: str) -> str:
#
# The list_time_series API requires us to query one metric
# at a time.
Expand All @@ -127,7 +157,7 @@ def _build_histogram_query(cls, metric_type: str) -> str:
)

@classmethod
def _build_counter_query(cls, metric_type: str) -> str:
def build_counter_query(cls, metric_type: str) -> str:
# For these num rpcs started counter metrics, they do not have the
# 'grpc_status' label
return (
Expand All @@ -142,28 +172,26 @@ def query_metrics(
metric_names: Iterable[str],
build_query_fn: BuildQueryFn,
interval: monitoring_v3.TimeInterval,
):
) -> dict[str, MetricTimeSeries]:
results = {}
metric_log_lines = [""]
for metric in metric_names:
response = self.metric_client.list_time_series(
name=f"projects/{self.project}",
filter=build_query_fn(metric),
interval=interval,
view=monitoring_v3.ListTimeSeriesRequest.TimeSeriesView.FULL,
)
time_series = []
metric_log_lines.append(f"Metric: {metric}")
for series in response:
metric_log_lines.append(" Metric Labels:")
for label_key, label_value in series.metric.labels.items():
metric_log_lines.append(f" {label_key}: {label_value}")
metric_log_lines.append(" Resource Labels:")
for label_key, label_value in series.resource.labels.items():
metric_log_lines.append(f" {label_key}: {label_value}")
time_series.append(series)
results[metric] = time_series
logger.info("\n".join(metric_log_lines))
time_series = list(response)
if len(time_series) > 1:
self.fail(
f"Query for {metric} should return exactly 1 time series. "
f"Found {len(time_series)}."
)
metric_time_series = MetricTimeSeries.from_response(
metric, time_series[0]
)
metric_time_series.pretty_print()
results[metric] = metric_time_series
return results

# A helper function to check whether at least one of the "points" whose
Expand Down Expand Up @@ -217,10 +245,10 @@ def test_csm_observability(self):
}
)
histogram_results = self.query_metrics(
HISTOGRAM_METRICS, self._build_histogram_query, interval
HISTOGRAM_METRICS, self.build_histogram_query, interval
)
counter_results = self.query_metrics(
COUNTER_METRICS, self._build_counter_query, interval
COUNTER_METRICS, self.build_counter_query, interval
)
all_results = {**histogram_results, **counter_results}
self.assertNotEmpty(all_results, msg="No query metrics results")
Expand All @@ -230,11 +258,6 @@ def test_csm_observability(self):
# Every metric needs to exist in the query results
self.assertIn(metric, all_results)

time_series = all_results[metric]
# There should be exactly 1 time series per metric with these
# label value combinations that we are expecting.
self.assertEqual(1, len(time_series))

# Testing whether each metric has the correct set of metric keys and
# values
with self.subTest("6_check_metrics_labels"):
Expand All @@ -258,9 +281,9 @@ def test_csm_observability(self):
"pod": test_client.hostname,
}
for metric in HISTOGRAM_CLIENT_METRICS:
actual_metric_labels = all_results[metric][0].metric.labels
actual_metric_labels = all_results[metric].metric_labels
self.assertDictEqual(
expected_metric_labels, dict(actual_metric_labels)
expected_metric_labels, actual_metric_labels
)

# Testing whether each metric has the correct set of metric keys and
Expand All @@ -283,9 +306,9 @@ def test_csm_observability(self):
"pod": test_server.hostname,
}
for metric in HISTOGRAM_SERVER_METRICS:
actual_metric_labels = all_results[metric][0].metric.labels
actual_metric_labels = all_results[metric].metric_labels
self.assertDictEqual(
expected_metric_labels, dict(actual_metric_labels)
expected_metric_labels, actual_metric_labels
)

# Testing whether each metric has the correct set of metric keys and
Expand All @@ -299,9 +322,9 @@ def test_csm_observability(self):
"pod": test_client.hostname,
}
for metric in COUNTER_CLIENT_METRICS:
actual_metric_labels = all_results[metric][0].metric.labels
actual_metric_labels = all_results[metric].metric_labels
self.assertDictEqual(
expected_metric_labels, dict(actual_metric_labels)
expected_metric_labels, actual_metric_labels
)

# Testing whether each metric has the correct set of metric keys and
Expand All @@ -314,9 +337,9 @@ def test_csm_observability(self):
"pod": test_server.hostname,
}
for metric in COUNTER_SERVER_METRICS:
actual_metric_labels = all_results[metric][0].metric.labels
actual_metric_labels = all_results[metric].metric_labels
self.assertDictEqual(
expected_metric_labels, dict(actual_metric_labels)
expected_metric_labels, actual_metric_labels
)

# Testing whether each metric has the right set of monitored resource
Expand All @@ -333,12 +356,14 @@ def test_csm_observability(self):
"project_id": self.project,
}
for metric in CLIENT_METRICS:
time_series = all_results[metric][0]
self.assertEqual("prometheus_target", time_series.resource.type)
metric_time_series = all_results[metric]
self.assertEqual(
"prometheus_target", metric_time_series.resource_type
)

actual_resource_labels = time_series.resource.labels
actual_resource_labels = metric_time_series.resource_labels
self.assertDictEqual(
expected_resource_labels, dict(actual_resource_labels)
expected_resource_labels, actual_resource_labels
)

# Testing whether each metric has the right set of monitored resource
Expand All @@ -355,12 +380,14 @@ def test_csm_observability(self):
"project_id": self.project,
}
for metric in SERVER_METRICS:
time_series = all_results[metric][0]
self.assertEqual("prometheus_target", time_series.resource.type)
metric_time_series = all_results[metric]
self.assertEqual(
"prometheus_target", metric_time_series.resource_type
)

actual_resource_labels = time_series.resource.labels
actual_resource_labels = metric_time_series.resource_labels
self.assertDictEqual(
expected_resource_labels, dict(actual_resource_labels)
expected_resource_labels, actual_resource_labels
)

# This tests whether each of the "byes sent" histogram type metric
Expand All @@ -372,15 +399,15 @@ def test_csm_observability(self):
METRIC_SERVER_CALL_RCVD,
):
self.assertAtLeastOnePointWithinRange(
all_results[metric][0].points, REQUEST_PAYLOAD_SIZE
all_results[metric].points, REQUEST_PAYLOAD_SIZE
)

for metric in (
METRIC_CLIENT_ATTEMPT_RCVD,
METRIC_SERVER_CALL_SENT,
):
self.assertAtLeastOnePointWithinRange(
all_results[metric][0].points, RESPONSE_PAYLOAD_SIZE
all_results[metric].points, RESPONSE_PAYLOAD_SIZE
)


Expand Down

0 comments on commit 00414b1

Please sign in to comment.