review comments

.github/actions/run-python-test-set/action.yml

@@ -48,12 +48,8 @@ inputs:
     description: 'benchmark durations JSON'
     required: false
     default: '{}'
-  reacquire_s3_credentials:
-    description: 'Whether to reacquire s3 credentials for allure report upload'
-    required: false
-    default: 'false'
-  reacquire_aws_oicd_role_arn:
-    description: 'the OIDC role arn to acquire for allure report upload'
+  aws_oicd_role_arn:
+    description: 'the OIDC role arn to (re-)acquire for allure report upload - if not set call must acquire OIDC role'
     required: false
     default: ''
 
@@ -231,11 +227,11 @@ runs:
         skip-if-does-not-exist: true
 
     - name: (Re-)configure AWS credentials # necessary to upload reports to S3 after a long-running test
-      if: ${{ !cancelled() && (inputs.reacquire_s3_credentials == 'true') }}
+      if: ${{ !cancelled() && (inputs.aws_oicd_role_arn != '') }}
       uses: aws-actions/configure-aws-credentials@v4
       with:
         aws-region: eu-central-1
-        role-to-assume: ${{ inputs.reacquire_aws_oicd_role_arn }}
+        role-to-assume: ${{ inputs.aws_oicd_role_arn }}
         role-duration-seconds: 3600 # 1 hour should be more than enough to upload report
     - name: Upload test results
       if: ${{ !cancelled() }}

.github/workflows/ingest_benchmark.yml

@@ -135,7 +135,8 @@ jobs:
       env:
         BENCHMARK_INGEST_SOURCE_CONNSTR: ${{ secrets.BENCHMARK_INGEST_SOURCE_CONNSTR }}
         TARGET_PROJECT_TYPE: ${{ matrix.target_project }}
-        COMMIT_HASH: ${{ github.sha }}
+        # we report PLATFORM in zenbenchmark NeonBenchmarker perf database and want to distinguish between new project and large tenant
+        PLATFORM: "pg16-${{ matrix.target_project }}-us-east-2-staging"
         PERF_TEST_RESULT_CONNSTR: "${{ secrets.PERF_TEST_RESULT_CONNSTR }}"
 
     - name: show tables sizes after ingest

scripts/ingest_perf_test_result.py

@@ -26,7 +26,7 @@
     metric_unit VARCHAR(10),
     metric_report_type TEXT,
     recorded_at_timestamp TIMESTAMP WITH TIME ZONE DEFAULT NOW(),
-    label_1 TEXT
+    labels JSONB with default '{}'
 )
 """
 
@@ -92,7 +92,7 @@ def ingest_perf_test_result(cursor, data_file: Path, recorded_at_timestamp: int)
                 "metric_unit": metric["unit"],
                 "metric_report_type": metric["report"],
                 "recorded_at_timestamp": datetime.utcfromtimestamp(recorded_at_timestamp),
-                "label_1": metric.get("label"),
+                "labels": json.dumps(metric.get("labels")),
             }
             args_list.append(values)
 
@@ -108,7 +108,7 @@ def ingest_perf_test_result(cursor, data_file: Path, recorded_at_timestamp: int)
             metric_unit,
             metric_report_type,
             recorded_at_timestamp,
-            label_1
+            labels
         ) VALUES %s
         """,
         args_list,
@@ -121,7 +121,7 @@ def ingest_perf_test_result(cursor, data_file: Path, recorded_at_timestamp: int)
             %(metric_unit)s,
             %(metric_report_type)s,
             %(recorded_at_timestamp)s,
-            %(label_1)s
+            %(labels)s
         )""",
     )
     return len(args_list)

test_runner/fixtures/benchmark_fixture.py

@@ -256,23 +256,25 @@ def record(
         metric_value: float,
         unit: str,
         report: MetricReport,
-        label: Optional[
-            str
-        ] = None,  # use this to assocaite additional Neon object IDs like project ID with the metric
+        labels: Optional[
+            dict[str, str]
+        ] = None,  # use this to associate additional key/value pairs in json format for associated Neon object IDs like project ID with the metric
     ):
         """
         Record a benchmark result.
         """
         # just to namespace the value
         name = f"{self.PROPERTY_PREFIX}_{metric_name}"
+        if labels is None:
+            labels = {}
         self.property_recorder(
             name,
             {
                 "name": metric_name,
                 "value": metric_value,
                 "unit": unit,
                 "report": report,
-                "label": label,
+                "labels": labels,
             },
         )
 

test_runner/performance/test_perf_ingest_using_pgcopydb.py

@@ -2,6 +2,7 @@
 import re
 import subprocess
 import sys
+import textwrap
 from pathlib import Path
 from typing import cast
 from urllib.parse import urlparse
@@ -24,7 +25,6 @@ def setup_environment():
     - BENCHMARK_INGEST_TARGET_CONNSTR
     - PERF_TEST_RESULT_CONNSTR
     - TARGET_PROJECT_TYPE
-    - COMMIT_HASH
 
     """
     # Ensure required environment variables are set
@@ -38,7 +38,6 @@ def setup_environment():
         "BENCHMARK_INGEST_TARGET_CONNSTR",
         "PERF_TEST_RESULT_CONNSTR",
         "TARGET_PROJECT_TYPE",
-        "COMMIT_HASH",
     ]
     for var in required_env_vars:
         if not os.getenv(var):
@@ -78,35 +77,35 @@ def build_pgcopydb_command(pgcopydb_filter_file: Path, test_output_dir: Path):
 @pytest.fixture()  # must be function scoped because test_output_dir is function scoped
 def pgcopydb_filter_file(test_output_dir: Path) -> Path:
     """Creates the pgcopydb_filter.txt file required by pgcopydb."""
-    filter_content = """\
-[include-only-table]
-public.events
-public.emails
-public.email_transmissions
-public.payments
-public.editions
-public.edition_modules
-public.sp_content
-public.email_broadcasts
-public.user_collections
-public.devices
-public.user_accounts
-public.lessons
-public.lesson_users
-public.payment_methods
-public.orders
-public.course_emails
-public.modules
-public.users
-public.module_users
-public.courses
-public.payment_gateway_keys
-public.accounts
-public.roles
-public.payment_gateways
-public.management
-public.event_names
-"""
+    filter_content = textwrap.dedent("""\
+        [include-only-table]
+        public.events
+        public.emails
+        public.email_transmissions
+        public.payments
+        public.editions
+        public.edition_modules
+        public.sp_content
+        public.email_broadcasts
+        public.user_collections
+        public.devices
+        public.user_accounts
+        public.lessons
+        public.lesson_users
+        public.payment_methods
+        public.orders
+        public.course_emails
+        public.modules
+        public.users
+        public.module_users
+        public.courses
+        public.payment_gateway_keys
+        public.accounts
+        public.roles
+        public.payment_gateways
+        public.management
+        public.event_names
+        """)
     filter_path = test_output_dir / "pgcopydb_filter.txt"
     filter_path.write_text(filter_content)
     return filter_path
@@ -193,19 +192,12 @@ def parse_log_and_report_metrics(
                         duration_seconds = humantime_to_ms(rust_like_humantime) / 1000.0
                         metrics[metric_name] = duration_seconds
 
-    endpoint_id = get_endpoint_id()
+    endpoint_id = {"endpoint_id": get_endpoint_id()}
     for metric_name, duration_seconds in metrics.items():
         zenbenchmark.record(
             metric_name, duration_seconds, "s", MetricReport.LOWER_IS_BETTER, endpoint_id
         )
 
-    # make sure we report project type as part of platform in NeonBenchmarker report
-    project_type = os.getenv("TARGET_PROJECT_TYPE")
-    if not project_type:
-        raise OSError("TARGET_PROJECT_TYPE environment variable is not set.")
-    platform = f"pg16-{project_type}-us-east-2-staging"
-    os.environ["PLATFORM"] = platform
-
 
 def get_endpoint_id():
     """Extracts and returns the first segment of the hostname from the PostgreSQL URI stored in BENCHMARK_INGEST_TARGET_CONNSTR."""
@@ -273,7 +265,3 @@ def test_ingest_performance_using_pgcopydb(
 
     # Parse log file and report metrics, including backpressure time difference
     parse_log_and_report_metrics(zenbenchmark, log_file_path, backpressure_time_diff)
-
-    # Check log file creation and content
-    assert log_file_path.exists(), "Log file should be created"
-    assert log_file_path.stat().st_size > 0, "Log file should not be empty"