Added updates based on code review

- test scenario labels updated
- pep8 compliance changes
- changed some var to more descriptive names

Author: ayefimov-1

roles/telemetry_chargeback/README.md

@@ -51,7 +51,7 @@ These variables are used internally by the role and typically do not need to be
 | `cloudkitty_loki_totals_metrics_suffix` | `-loki_metrics_totals.yml` | Suffix for metric totals computed from Loki-retrieved JSON (retrieve_loki_data task). |
 | `cloudkitty_synth_script` | `{{ role_path }}/files/gen_synth_loki_data.py` | Path to the synthetic data generation script. |
 | `cloudkitty_data_template` | `{{ role_path }}/templates/loki_data_templ.j2` | Path to the Jinja2 template for Loki data format. |
-| `cloudkitty_totals_script` | `{{ role_path }}/files/gen_synth_loki_metrics_totals.py` | Path to the metric totals calculation script. |
+| `cloudkitty_summary_script` | `{{ role_path }}/files/gen_db_summary.py` | Path to the summary script (gen_db_summary.py). |
 
 ### Loki / OpenShift Variables (vars/main.yml)
 

roles/telemetry_chargeback/files/gen_db_summary.py

@@ -44,10 +44,9 @@ def _extract_from_loki_json(data: dict) -> list[tuple[str, str]]:
                 continue
             ts_str = val[0]
             log_str = val[1]
-            if not isinstance(ts_str, str) or not isinstance(log_str, str):
-                continue
-            if not _is_valid_timestep(ts_str):
+            if not _is_valid_timestep(ts_str) or not isinstance(log_str, str):
                 continue
+
             try:
                 entry = json.loads(log_str)
                 if isinstance(entry, dict) and _has_required_keys(entry):

roles/telemetry_chargeback/files/gen_synth_loki_data.py

@@ -2,6 +2,7 @@
 import logging
 import argparse
 import json
+import sys
 import yaml
 from datetime import datetime, timezone, timedelta
 from pathlib import Path
@@ -108,8 +109,8 @@ def generate_loki_data(
     end_time: datetime,
     time_step_seconds: int,
     config: Dict[str, Any],
-    project_id: Union[str, int, None] = None,
-    user_id: Union[str, int, None] = None,
+    project: Union[str, int, None] = None,
+    user: Union[str, int, None] = None,
     reverse_timestamps: bool = False,
 ):
     """
@@ -122,9 +123,9 @@ def generate_loki_data(
         end_time (datetime): The end time for data generation.
         time_step_seconds (int): The duration of each log entry in seconds.
         config (Dict[str, Any]): Configuration dictionary loaded from file.
-        project_id: Optional value to inject as groupby.project_id in every
+        project: Optional value to inject as groupby.project in every
             log entry in the output (overrides test_* file value when set).
-        user_id: Optional value to inject as groupby.user_id in every
+        user: Optional value to inject as groupby.user in every
             log entry in the output (overrides test_* file value when set).
         reverse_timestamps (bool): If True, reverse the order of timestamps
             in the JSON output (youngest first, oldest last).
@@ -324,10 +325,10 @@ def tojson_preserve_order(obj):
             log_type_with_dates = log_type_data.copy()
             log_type_with_dates["groupby"] = log_type_data["groupby"].copy()
             log_type_with_dates["groupby"].update(date_fields)
-            if project_id is not None:
-                log_type_with_dates["groupby"]["project_id"] = project_id
-            if user_id is not None:
-                log_type_with_dates["groupby"]["user_id"] = user_id
+            if project is not None:
+                log_type_with_dates["groupby"]["project"] = project
+            if user is not None:
+                log_type_with_dates["groupby"]["user"] = user
             # Select qty and price based on step index distribution
             log_type_with_dates["qty"] = _get_value_for_step(
                 log_type_data["qty"], idx, num_steps
@@ -408,15 +409,15 @@ def main():
         type=str,
         default=None,
         metavar="ID",
-        help="Optional alphanumeric value to use as groupby.project_id in "
+        help="Optional alphanumeric value to use as groupby.project in "
              "every log entry in the output (overrides value from test file)."
     )
     parser.add_argument(
         "-u", "--user-id",
         type=str,
         default=None,
         metavar="ID",
-        help="Optional alphanumeric value to use as groupby.user_id in "
+        help="Optional alphanumeric value to use as groupby.user in "
              "every log entry in the output (overrides value from test file)."
     )
 
@@ -464,8 +465,8 @@ def main():
             end_time=end_time_utc,
             time_step_seconds=step_seconds,
             config=config,
-            project_id=args.project_id,
-            user_id=args.user_id,
+            project=args.project_id,
+            user=args.user_id,
             reverse_timestamps=args.reverse,
         )
     except FileNotFoundError:

roles/telemetry_chargeback/files/test_dyn_basic.yml

@@ -9,8 +9,9 @@ generation:
 # Log type definitions (single "type" = identifier and value pushed to output)
 log_types:
   - type: ceilometer_image_size
-    unit: MiB
     description: "Size of ceilometer image"
+    unit: MiB
+    factor: 1/1048576
     qty:
       - 20.6
       - 25.0
@@ -23,103 +24,104 @@ log_types:
       - 0.07
       - 0.10
     groupby:
-      id: null
-      user_id: null
-      project_id: null
-      tenant_id: tenant-01
+      resource: null
+      user: null
+      project: null
+      tenant: tenant-01
     metadata:
       container_format: bare
       disk_format: qcow2
 
-  - type: ceilometer_cpu_num
-    unit: scalar
+  - type: ceilometer_cpu
     description: "max number of cpus used in time step"
+    unit: instance
+    alt_name: instance
     qty:
       - 1.0
     price:
       - 0.3
     groupby:
-      id: null
-      user_id: null
-      project_id: null
-      tenant_id: tenant-02
-    metadata:
-      flavor_name: m1.tiny
-      flavor_id: "1"
-      vcpus: ""
+      resource: null
+      user: null
+      project: null
+      tenant: tenant-02
+      flavor_name: null
+      flavor_id: null
+    mutate: NUMBOOL
 
   - type: ceilometer_ip_floating
-    unit: ip
     description: null
+    unit: ip
     qty:
       - 0.0
     price:
       - 0.50
     groupby:
-      id: null
-      user_id: null
-      project_id: null
-      tenant_id: tenant-01
+      resource: null
+      user: null
+      project: null
+      tenant: tenant-01
     metadata:
       state: null
+    mutate: NUMBOOL
 
   - type: ceilometer_disk_ephemeral_size
-    unit: GiB
     description: "Max at each timestep"
+    unit: GiB
     qty:
       - 0.0
     price:
       - 0.0
     groupby:
-      id: null
-      user_id: null
-      project_id: null
-      tenant_id: tenant-01
+      resource: null
+      user: null
+      project: null
+      tenant: tenant-01
     metadata:
       type: null
 
-  - type: ceilometer-disk-root_size
-    unit: GiB
+  - type: ceilometer_disk_root_size
     description: null
+    unit: GiB
     qty:
       - 0.0
     price:
       - 0.0
     groupby:
-      id: null
-      user_id: null
-      project_id: null
-      tenant_id: tenant-02
+      resource: null
+      user: null
+      project: null
+      tenant: tenant-02
     metadata:
-      disk_format: null
+      type: null
 
   - type: ceilometer_network_outgoing_bytes
-    unit: B
     description: null
+    unit: B
     qty:
       - 0.0
     price:
       - 0.0
     groupby:
-      id: null
-      user_id: null
-      project_id: null
-      tenant_id: tenant-01
+      resource: null
+      user: null
+      project: null
+      tenant: tenant-01
     metadata:
       vm_instance: null
 
   - type: ceilometer_network_incoming_bytes
-    unit: B
     description: null
+    unit: B
     qty:
       - 0.0
     price:
       - 0.0
     groupby:
-      id: null
-      user_id: null
-      project_id: null
-      tenant_id: tenant-02
+      resource: null
+      user: null
+      project: null
+      tenant: tenant-02
     metadata:
       vm_instance: null
 

roles/telemetry_chargeback/tasks/gen_synth_loki_data.yml

@@ -26,7 +26,7 @@
 - name: "Generate chargeback rating from synthetic data file {{ item }}"
   ansible.builtin.command:
     cmd: >
-      python3 "{{ cloudkitty_totals_script }}"
+      python3 "{{ cloudkitty_summary_script }}"
       -j "{{ cloudkitty_data_file }}"
       -o "{{ cloudkitty_synth_totals_file }}"
       --debug "{{ cloudkitty_debug_dir }}"

roles/telemetry_chargeback/tasks/retrieve_loki_data.yml

@@ -63,7 +63,7 @@
 - name: "Generate chargeback stats from Loki-retrieved data file: {{ item }}"
   ansible.builtin.command:
     cmd: >
-      python3 "{{ cloudkitty_totals_script }}"
+      python3 "{{ cloudkitty_summary_script }}"
       -j "{{ artifacts_dir_zuul }}/{{ item }}{{ cloudkitty_loki_data_suffix }}"
       -o "{{ artifacts_dir_zuul }}/{{ item }}{{ cloudkitty_loki_totals_metrics_suffix }}"
       --debug "{{ cloudkitty_debug_dir }}"

roles/telemetry_chargeback/vars/main.yml

@@ -8,13 +8,13 @@ cloudkitty_debug: false
 cloudkitty_scenario_dir: "{{ role_path }}/files"
 cloudkitty_synth_data_suffix: "-synth_data.json"
 cloudkitty_loki_data_suffix: "-loki_data.json"
-cloudkitty_synth_totals_metrics_suffix: "-synth_metrics_totals.yml"
-cloudkitty_loki_totals_metrics_suffix: "-loki_metrics_totals.yml"
-cloudkitty_loki_totals_suffix: "-loki_totals.yml"
+cloudkitty_synth_totals_metrics_suffix: "-synth_metrics_summary.yml"
+cloudkitty_loki_totals_metrics_suffix: "-loki_metrics_summary.yml"
+cloudkitty_loki_totals_suffix: "-rating.yml"
 
 cloudkitty_synth_script: "{{ role_path }}/files/gen_synth_loki_data.py"
 cloudkitty_data_template: "{{ role_path }}/templates/loki_data_templ.j2"
-cloudkitty_totals_script: "{{ role_path }}/files/gen_db_summary.py"
+cloudkitty_summary_script: "{{ role_path }}/files/gen_db_summary.py"
 
 # Cloudkitty certificates
 cert_secret_name: "cert-cloudkitty-client-internal"