From bf314a8a17bf3ca2c855e1cbf7bdd8ee533c6261 Mon Sep 17 00:00:00 2001 From: Chris Munns Date: Thu, 22 Oct 2026 18:43:55 -0700 Subject: [PATCH] Collect null_frac, default sequences, and AWS replica and billing fields PostgreSQL column statistics now carry pg_stats.null_frac, and each column names the sequence its nextval() default calls. Each RDS instance and Aurora cluster records its replica source and replicas, and each RDS instance records storage throughput, max allocated storage, dedicated log volume and extended support. Ports #187, #189 and #202. --- docs/providers/snowflake.md | 4 +- .../cloud/analyzers/aws_analyzer.py | 41 +++++ .../database/analyzers/schema_analyzer.py | 20 ++- tests/unit/test_aws_rds_billing_fields.py | 70 +++++++++ tests/unit/test_aws_rds_replicas.py | 144 ++++++++++++++++++ tests/unit/test_schema_analyzer.py | 72 +++++++++ 6 files changed, 347 insertions(+), 4 deletions(-) create mode 100644 tests/unit/test_aws_rds_billing_fields.py create mode 100644 tests/unit/test_aws_rds_replicas.py diff --git a/docs/providers/snowflake.md b/docs/providers/snowflake.md index 8cb5a45..c54ba5e 100644 --- a/docs/providers/snowflake.md +++ b/docs/providers/snowflake.md @@ -171,7 +171,7 @@ GRANT CONNECT ON DATABASE your_database TO planetscale_discovery; GRANT pg_read_all_stats, pg_read_all_settings, pg_monitor TO planetscale_discovery; ``` -This login cannot read table rows. Discovery still reports the same tables, indexes, extensions, and settings as `snowflake_admin`. The one difference is per-column `n_distinct` / `correlation`, which Postgres only shows for tables the user can `SELECT`. Drop the login after the run: +This login cannot read table rows. Discovery still reports the same tables, indexes, extensions, and settings as `snowflake_admin`. The one difference is per-column `n_distinct` / `null_frac` / `correlation`, which Postgres only shows for tables the user can `SELECT`. Drop the login after the run: ```sql REVOKE pg_read_all_stats, pg_read_all_settings, pg_monitor FROM planetscale_discovery; @@ -311,7 +311,7 @@ Use `name` for the instance display name. Use `host` for the connection endpoint - Table contents or row data - Application code - Passwords, password hashes, private keys, or other credentials (used for the connection, never written to the report). Postgres roles appear only as names with a `has_password` flag -- Column values. Column statistics are limited to `n_distinct` and `correlation` +- Column values. Column statistics are limited to `n_distinct`, `null_frac` and `correlation` The report does include hostnames, instance names, the account locator, Postgres role names, and table, column, and index names. diff --git a/planetscale_discovery/cloud/analyzers/aws_analyzer.py b/planetscale_discovery/cloud/analyzers/aws_analyzer.py index ec10fce..a6e0f8a 100644 --- a/planetscale_discovery/cloud/analyzers/aws_analyzer.py +++ b/planetscale_discovery/cloud/analyzers/aws_analyzer.py @@ -48,6 +48,40 @@ ) +def _rds_instance_replication(db_instance: Dict[str, Any]) -> Dict[str, Any]: + return { + "read_replica_source_db_instance_identifier": db_instance.get( + "ReadReplicaSourceDBInstanceIdentifier" + ), + "read_replica_db_instance_identifiers": db_instance.get( + "ReadReplicaDBInstanceIdentifiers", [] + ), + "read_replica_source_db_cluster_identifier": db_instance.get( + "ReadReplicaSourceDBClusterIdentifier" + ), + "read_replica_db_cluster_identifiers": db_instance.get( + "ReadReplicaDBClusterIdentifiers", [] + ), + "replica_mode": db_instance.get("ReplicaMode"), + } + + +def _rds_instance_billing(db_instance: Dict[str, Any]) -> Dict[str, Any]: + return { + "storage_throughput": db_instance.get("StorageThroughput"), + "max_allocated_storage": db_instance.get("MaxAllocatedStorage"), + "dedicated_log_volume": db_instance.get("DedicatedLogVolume", False), + "engine_lifecycle_support": db_instance.get("EngineLifecycleSupport"), + } + + +def _aurora_cluster_replication(db_cluster: Dict[str, Any]) -> Dict[str, Any]: + return { + "replication_source_identifier": db_cluster.get("ReplicationSourceIdentifier"), + "read_replica_identifiers": db_cluster.get("ReadReplicaIdentifiers", []), + } + + class AWSAnalyzer(CloudAnalyzer): """AWS cloud database environment analyzer.""" @@ -470,6 +504,8 @@ def _analyze_rds_instances(self, region: str) -> List[Dict[str, Any]]: "region": region, "db_cluster_identifier": db_instance.get("DBClusterIdentifier"), "dbi_resource_id": db_instance.get("DbiResourceId"), + **_rds_instance_replication(db_instance), + **_rds_instance_billing(db_instance), } instances.append(instance_analysis) @@ -532,6 +568,7 @@ def _analyze_aurora_clusters(self, region: str) -> List[Dict[str, Any]]: ), "backtrack_window": db_cluster.get("BacktrackWindow", 0), "region": region, + **_aurora_cluster_replication(db_cluster), } # Get cluster members @@ -2058,6 +2095,7 @@ def _analyze_single_rds_instance( "db_instance_class": instance_data.get("DBInstanceClass"), "allocated_storage": instance_data.get("AllocatedStorage"), "storage_type": instance_data.get("StorageType"), + "iops": instance_data.get("Iops"), "storage_encrypted": instance_data.get("StorageEncrypted", False), "kms_key_id": instance_data.get("KmsKeyId"), "multi_az": instance_data.get("MultiAZ", False), @@ -2092,6 +2130,8 @@ def _analyze_single_rds_instance( "monitoring_interval": instance_data.get("MonitoringInterval", 0), "db_cluster_identifier": instance_data.get("DBClusterIdentifier"), "dbi_resource_id": instance_data.get("DbiResourceId"), + **_rds_instance_replication(instance_data), + **_rds_instance_billing(instance_data), } self._collect_region_metrics(region, [analysis], []) @@ -2144,6 +2184,7 @@ def _analyze_single_aurora_cluster( } for member in cluster_data.get("DBClusterMembers", []) ], + **_aurora_cluster_replication(cluster_data), } if analysis["engine_mode"] == "serverless" or cluster_data.get( diff --git a/planetscale_discovery/database/analyzers/schema_analyzer.py b/planetscale_discovery/database/analyzers/schema_analyzer.py index fb3e9c4..39ea41f 100644 --- a/planetscale_discovery/database/analyzers/schema_analyzer.py +++ b/planetscale_discovery/database/analyzers/schema_analyzer.py @@ -320,7 +320,11 @@ def _get_table_analysis(self) -> List[Dict[str, Any]]: for row in rows: table_info = dict(row) - # Get column information + # Get column information. + # default_sequence is the sequence that the column default + # calls when the default is one nextval() call and nothing + # else. The default text stays in the database: the query + # selects only the sequence name, from pg_depend. try: with self.connection.cursor() as detail_cursor: detail_cursor.execute( @@ -348,7 +352,18 @@ def _get_table_analysis(self) -> List[Dict[str, Any]]: AND dep.refobjsubid = a.attnum AND dep.deptype IN ('a', 'i') LIMIT 1 - ) as owned_sequence + ) as owned_sequence, + ( + SELECT dep.refobjid::regclass::text + FROM pg_depend dep + JOIN pg_class seq ON seq.oid = dep.refobjid AND seq.relkind = 'S' + WHERE dep.classid = 'pg_attrdef'::regclass + AND dep.objid = d.oid + AND dep.refclassid = 'pg_class'::regclass + AND pg_get_expr(d.adbin, d.adrelid) + ~ '^nextval\\(''(?:[^'']|'''')+''::regclass\\)$' + LIMIT 1 + ) as default_sequence FROM pg_attribute a LEFT JOIN pg_attrdef d ON (a.attrelid = d.adrelid AND a.attnum = d.adnum) WHERE a.attrelid = %s @@ -378,6 +393,7 @@ def _get_table_analysis(self) -> List[Dict[str, Any]]: tablename, attname, n_distinct, + null_frac, correlation FROM pg_stats WHERE schemaname = %s AND tablename = %s diff --git a/tests/unit/test_aws_rds_billing_fields.py b/tests/unit/test_aws_rds_billing_fields.py new file mode 100644 index 0000000..04d5bd8 --- /dev/null +++ b/tests/unit/test_aws_rds_billing_fields.py @@ -0,0 +1,70 @@ +import logging + +import pytest + +from planetscale_discovery.cloud.analyzers.aws_analyzer import AWSAnalyzer +from planetscale_discovery.config.config_manager import AWSConfig + +REGION = "us-east-1" + +BILLED_INSTANCE = { + "DBInstanceIdentifier": "app-primary", + "Engine": "postgres", + "EngineVersion": "13.20", + "DBInstanceClass": "db.r6g.4xlarge", + "AllocatedStorage": 2000, + "StorageType": "gp3", + "Iops": 12000, + "StorageThroughput": 500, + "MaxAllocatedStorage": 4000, + "DedicatedLogVolume": True, + "EngineLifecycleSupport": "open-source-rds-extended-support", +} + +UNBILLED_INSTANCE = { + "DBInstanceIdentifier": "app-small", + "Engine": "postgres", + "EngineVersion": "17.4", + "DBInstanceClass": "db.t3.micro", +} + + +def _analyzer(mocker) -> AWSAnalyzer: + analyzer = AWSAnalyzer( + AWSConfig(enabled=True, regions=[REGION]), logging.getLogger("t") + ) + analyzer.session = mocker.Mock() + mocker.patch.object(analyzer, "_collect_region_metrics") + return analyzer + + +def _full_path(analyzer, mocker, instance): + mocker.patch.object(analyzer, "_fetch_rds_db_instances", return_value=[instance]) + [analysis] = analyzer._analyze_rds_instances(REGION) + return analysis + + +def _focused_path(analyzer, mocker, instance): + return analyzer._analyze_single_rds_instance(instance, REGION) + + +@pytest.mark.parametrize("path", [_full_path, _focused_path]) +class TestRdsBillingFields: + def test_billed_storage_and_support_fields_are_kept(self, mocker, path): + analysis = path(_analyzer(mocker), mocker, BILLED_INSTANCE) + + assert analysis["iops"] == 12000 + assert analysis["storage_throughput"] == 500 + assert analysis["max_allocated_storage"] == 4000 + assert analysis["dedicated_log_volume"] is True + assert analysis["engine_lifecycle_support"] == ( + "open-source-rds-extended-support" + ) + + def test_absent_fields_stay_unset(self, mocker, path): + analysis = path(_analyzer(mocker), mocker, UNBILLED_INSTANCE) + + assert analysis["storage_throughput"] is None + assert analysis["max_allocated_storage"] is None + assert analysis["dedicated_log_volume"] is False + assert analysis["engine_lifecycle_support"] is None diff --git a/tests/unit/test_aws_rds_replicas.py b/tests/unit/test_aws_rds_replicas.py new file mode 100644 index 0000000..9dc1758 --- /dev/null +++ b/tests/unit/test_aws_rds_replicas.py @@ -0,0 +1,144 @@ +import logging + +import boto3 +import pytest +from moto import mock_aws + +from planetscale_discovery.cloud.analyzers.aws_analyzer import AWSAnalyzer +from planetscale_discovery.config.config_manager import AWSConfig + +REGION = "us-east-1" +PW = "password123" + + +@pytest.fixture +def aws_env(aws_credentials): + with mock_aws(): + yield boto3.client("rds", region_name=REGION) + + +def _analyzer() -> AWSAnalyzer: + cfg = AWSConfig(enabled=True, regions=[REGION], discover_all=True) + analyzer = AWSAnalyzer(cfg, logging.getLogger("rds-replica-test")) + assert analyzer.authenticate() is True + return analyzer + + +class TestRdsReadReplicaLinks: + def test_primary_and_replica_name_each_other(self, aws_env): + aws_env.create_db_instance( + DBInstanceIdentifier="app-primary", + DBInstanceClass="db.r6g.large", + Engine="postgres", + MasterUsername="admin", + MasterUserPassword=PW, + AllocatedStorage=100, + ) + aws_env.create_db_instance_read_replica( + DBInstanceIdentifier="app-replica", + SourceDBInstanceIdentifier="app-primary", + ) + + result = _analyzer().analyze()["resources"][REGION] + by_id = {i["db_instance_identifier"]: i for i in result["rds_instances"]} + + primary = by_id["app-primary"] + replica = by_id["app-replica"] + assert primary["read_replica_db_instance_identifiers"] == ["app-replica"] + assert primary["read_replica_source_db_instance_identifier"] is None + assert replica["read_replica_source_db_instance_identifier"] == "app-primary" + assert replica["read_replica_db_instance_identifiers"] == [] + + def test_standalone_instance_has_empty_links(self, aws_env): + aws_env.create_db_instance( + DBInstanceIdentifier="solo", + DBInstanceClass="db.t3.micro", + Engine="postgres", + MasterUsername="admin", + MasterUserPassword=PW, + AllocatedStorage=20, + ) + + [solo] = _analyzer().analyze()["resources"][REGION]["rds_instances"] + + assert solo["read_replica_source_db_instance_identifier"] is None + assert solo["read_replica_db_instance_identifiers"] == [] + assert solo["read_replica_source_db_cluster_identifier"] is None + assert solo["read_replica_db_cluster_identifiers"] == [] + assert solo["replica_mode"] is None + + +class TestAuroraClusterReplicaLinks: + def test_replica_cluster_names_its_source(self, aws_env): + west = boto3.client("rds", region_name="us-west-2") + source_arn = west.create_db_cluster( + DBClusterIdentifier="app-cluster", + Engine="aurora-postgresql", + MasterUsername="admin", + MasterUserPassword=PW, + )["DBCluster"]["DBClusterArn"] + aws_env.create_db_cluster( + DBClusterIdentifier="app-cluster-east", + Engine="aurora-postgresql", + MasterUsername="admin", + MasterUserPassword=PW, + ReplicationSourceIdentifier=source_arn, + ) + aws_env.create_db_cluster( + DBClusterIdentifier="app-cluster-solo", + Engine="aurora-postgresql", + MasterUsername="admin", + MasterUserPassword=PW, + ) + + result = _analyzer().analyze()["resources"][REGION] + by_id = {c["identifier"]: c for c in result["aurora_clusters"]} + + assert by_id["app-cluster-east"]["replication_source_identifier"] == source_arn + assert by_id["app-cluster-solo"]["replication_source_identifier"] is None + assert by_id["app-cluster-solo"]["read_replica_identifiers"] == [] + + +class TestFocusedPathReplicaLinks: + def test_cross_region_replica_keeps_the_source_arn(self, mocker): + analyzer = AWSAnalyzer( + AWSConfig(enabled=True, regions=[REGION]), logging.getLogger("t") + ) + analyzer.session = mocker.Mock() + mocker.patch.object(analyzer, "_collect_region_metrics") + source_arn = "arn:aws:rds:us-west-2:123456789012:db:app-primary" + + analysis = analyzer._analyze_single_rds_instance( + { + "DBInstanceIdentifier": "app-replica-east", + "Engine": "postgres", + "DBInstanceClass": "db.r6g.large", + "ReadReplicaSourceDBInstanceIdentifier": source_arn, + "ReplicaMode": "open-read-only", + }, + REGION, + ) + + assert analysis["read_replica_source_db_instance_identifier"] == source_arn + assert analysis["replica_mode"] == "open-read-only" + assert analysis["read_replica_db_instance_identifiers"] == [] + + def test_aurora_replica_cluster_names_its_source(self, mocker): + analyzer = AWSAnalyzer( + AWSConfig(enabled=True, regions=[REGION]), logging.getLogger("t") + ) + mocker.patch.object(analyzer, "_collect_region_metrics") + source_arn = "arn:aws:rds:us-west-2:123456789012:cluster:app-cluster" + + analysis = analyzer._analyze_single_aurora_cluster( + { + "DBClusterIdentifier": "app-cluster-east", + "Engine": "aurora-postgresql", + "EngineMode": "provisioned", + "ReplicationSourceIdentifier": source_arn, + }, + REGION, + ) + + assert analysis["replication_source_identifier"] == source_arn + assert analysis["read_replica_identifiers"] == [] diff --git a/tests/unit/test_schema_analyzer.py b/tests/unit/test_schema_analyzer.py index d61c99e..03f4a34 100644 --- a/tests/unit/test_schema_analyzer.py +++ b/tests/unit/test_schema_analyzer.py @@ -540,6 +540,49 @@ def cursor_factory(): assert result[0]["columns"][0]["column_name"] == "id" assert len(result[0]["statistics"]) == 1 + def test_get_table_analysis_statistics_select_null_frac(self, mock_connection): + """The pg_stats query selects null_frac. + + Without it a column with no NULLs reads the same as one whose NULL + share was never measured. + """ + connection, _ = mock_connection + version_cursor, main_cursor, detail_cursor = ( + MagicMock(), + MagicMock(), + MagicMock(), + ) + cursors = iter([version_cursor, main_cursor]) + + def cursor_factory(): + ctx = MagicMock() + ctx.__enter__ = MagicMock(return_value=next(cursors, detail_cursor)) + ctx.__exit__ = MagicMock(return_value=None) + return ctx + + connection.cursor.side_effect = cursor_factory + version_cursor.fetchone.return_value = {"server_version_num": "140011"} + main_cursor.fetchall.return_value = [ + { + "schema_name": "public", + "table_name": "users", + "table_oid": 16385, + "table_type": "r", + "is_partition": False, + } + ] + detail_cursor.fetchall.return_value = [] + + SchemaAnalyzer(connection)._get_table_analysis() + + stats_sql = [ + c.args[0] + for c in detail_cursor.execute.call_args_list + if "FROM pg_stats" in c.args[0] + ] + assert len(stats_sql) == 1 + assert "null_frac" in stats_sql[0] + def test_get_table_analysis_empty(self, mock_connection): """Test table analysis with no tables returns empty list""" connection, _ = mock_connection @@ -794,6 +837,35 @@ def test_get_table_analysis_column_identity_and_generated( for column in absent: assert column not in column_sql + def test_get_table_analysis_column_default_sequence( + self, analyzer, mock_connection + ): + """Column query names the sequence a nextval() default calls, never the default text""" + _, cursor = mock_connection + cursor.fetchall.return_value = [ + {"table_oid": 16385, "table_name": "users", "table_type": "r"} + ] + + with patch.object(analyzer, "_get_server_version_num", return_value=170000): + analyzer._get_table_analysis() + + column_sql = next( + c.args[0] + for c in cursor.execute.call_args_list + if "FROM pg_attribute a" in c.args[0] + ) + assert "as default_sequence" in column_sql + # The sequence comes from pg_depend, through the column's pg_attrdef row. + assert "dep.classid = 'pg_attrdef'::regclass" in column_sql + assert "dep.objid = d.oid" in column_sql + # Only a default that is one nextval() call names a sequence. + assert r"~ '^nextval\(''(?:[^'']|'''')+''::regclass\)$'" in column_sql + # The default text appears only in that filter. It is never a + # selected column, so it cannot reach the report. + assert column_sql.count("pg_get_expr") == 1 + assert "AND pg_get_expr(d.adbin, d.adrelid)" in column_sql + assert "adsrc" not in column_sql + # --------------------------------------------------------------- # _get_index_analysis() # ---------------------------------------------------------------