From e4735823f3f2fcdaa4408d8c1041512d8246c2a5 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 8 Jun 2023 12:20:59 -0700 Subject: [PATCH 1/4] Rm unused enable_sanitize_sql config --- solarwinds_apm/apm_config.py | 3 --- tests/unit/test_apm_config/fixtures/cnf_dict.py | 2 -- tests/unit/test_apm_config/test_apm_config_cnf_file.py | 7 ------- 3 files changed, 12 deletions(-) diff --git a/solarwinds_apm/apm_config.py b/solarwinds_apm/apm_config.py index 0a56520f..c387ce70 100644 --- a/solarwinds_apm/apm_config.py +++ b/solarwinds_apm/apm_config.py @@ -106,7 +106,6 @@ def __init__( "bufsize": -1, "histogram_precision": -1, "reporter_file_single": 0, - "enable_sanitize_sql": True, "log_trace_id": "never", "proxy": "", "is_grpc_clean_hack_enabled": False, @@ -451,8 +450,6 @@ def __setitem__(self, key: str, value: str) -> None: elif key == "log_trace_id": self._set_config_value(key, value) - elif key in {"enable_sanitize_sql", "warn_deprecated"}: - self._set_config_value(key, value) else: logger.warning( "Unsupported SolarWinds APM config key: %s", diff --git a/tests/unit/test_apm_config/fixtures/cnf_dict.py b/tests/unit/test_apm_config/fixtures/cnf_dict.py index c2657e15..f14436b1 100644 --- a/tests/unit/test_apm_config/fixtures/cnf_dict.py +++ b/tests/unit/test_apm_config/fixtures/cnf_dict.py @@ -24,7 +24,6 @@ def fixture_cnf_dict(): "bufsize": 2, "histogramPrecision": 2, "reporterFileSingle": 2, - "enableSanitizeSql": True, "logTraceId": "always", "proxy": "http://foo-bar", "isGrpcCleanHackEnabled": True, @@ -54,7 +53,6 @@ def fixture_cnf_dict_enabled_false(): "bufsize": 2, "histogramPrecision": 2, "reporterFileSingle": 2, - "enableSanitizeSql": True, "logTraceId": "always", "proxy": "http://foo-bar", "isGrpcCleanHackEnabled": True, diff --git a/tests/unit/test_apm_config/test_apm_config_cnf_file.py b/tests/unit/test_apm_config/test_apm_config_cnf_file.py index 86ac1d62..2fc4441b 100644 --- a/tests/unit/test_apm_config/test_apm_config_cnf_file.py +++ b/tests/unit/test_apm_config/test_apm_config_cnf_file.py @@ -130,7 +130,6 @@ def test_update_with_cnf_file_all_valid( assert resulting_config.get("bufsize") == 2 assert resulting_config.get("histogram_precision") == 2 assert resulting_config.get("reporter_file_single") == 2 - assert resulting_config.get("enable_sanitize_sql") == True assert resulting_config.get("log_trace_id") == "always" assert resulting_config.get("proxy") == "http://foo-bar" assert resulting_config.get("is_grpc_clean_hack_enabled") == True @@ -178,7 +177,6 @@ def test_update_with_cnf_file_mostly_invalid( "bufsize": "foo", "histogramPrecision": "foo", "reporterFileSingle": "foo", - "enableSanitizeSql": "foo", "log_trace_id": "not-never-always-etc", "proxy": "foo", "isGrpcCleanHackEnabled": "foo", @@ -211,7 +209,6 @@ def test_update_with_cnf_file_mostly_invalid( assert resulting_config.get("bufsize") == -1 assert resulting_config.get("histogram_precision") == -1 assert resulting_config.get("reporter_file_single") == 0 - assert resulting_config.get("enable_sanitize_sql") == True assert resulting_config.get("log_trace_id") == "never" assert resulting_config.get("proxy") == "" assert resulting_config.get("is_grpc_clean_hack_enabled") == False @@ -260,7 +257,6 @@ def test_update_with_cnf_file_and_all_validenv_vars( "SW_APM_BUFSIZE": "3", "SW_APM_HISTOGRAM_PRECISION": "3", "SW_APM_REPORTER_FILE_SINGLE": "3", - "SW_APM_ENABLE_SANITIZE_SQL": "false", "SW_APM_LOG_TRACE_ID": "never", "SW_APM_PROXY": "http://other-foo-bar", "SW_APM_IS_GRPC_CLEAN_HACK_ENABLED": "false", @@ -302,7 +298,6 @@ def test_update_with_cnf_file_and_all_validenv_vars( assert resulting_config.get("bufsize") == 3 assert resulting_config.get("histogram_precision") == 3 assert resulting_config.get("reporter_file_single") == 3 - assert resulting_config.get("enable_sanitize_sql") == False assert resulting_config.get("log_trace_id") == "never" assert resulting_config.get("proxy") == "http://other-foo-bar" assert resulting_config.get("is_grpc_clean_hack_enabled") == False @@ -344,7 +339,6 @@ def test_update_with_cnf_file_and_several_invalid_env_vars( "SW_APM_BUFSIZE": "other-foo-bar", "SW_APM_HISTOGRAM_PRECISION": "other-foo-bar", "SW_APM_REPORTER_FILE_SINGLE": "other-foo-bar", - "SW_APM_ENABLE_SANITIZE_SQL": "other-foo-bar", "SW_APM_LOG_TRACE_ID": "other-foo-bar", "SW_APM_PROXY": "other-foo-bar", "SW_APM_IS_GRPC_CLEAN_HACK_ENABLED": "other-foo-bar", @@ -383,7 +377,6 @@ def test_update_with_cnf_file_and_several_invalid_env_vars( assert resulting_config.get("bufsize") == 2 assert resulting_config.get("histogram_precision") == 2 assert resulting_config.get("reporter_file_single") == 2 - assert resulting_config.get("enable_sanitize_sql") == True assert resulting_config.get("log_trace_id") == "always" assert resulting_config.get("proxy") == "http://foo-bar" assert resulting_config.get("is_grpc_clean_hack_enabled") == True From 61b7fed7bc577f36a49020dff2c0ee526e66fcfc Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 8 Jun 2023 12:22:23 -0700 Subject: [PATCH 2/4] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eca3e902..5f9c6e4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add log warning when `set_transaction_name` with empty name ([#162](https://github.com/solarwindscloud/solarwinds-apm-python/pull/162)) + +- Removed unused enable_sanitize_sql config storage ([#164](https://github.com/solarwindscloud/solarwinds-apm-python/pull/164)) + ## [0.12.0](https://github.com/solarwindscloud/solarwinds-apm-python/releases/tag/rel-0.12.0) - 2023-06-01 ### Changed - Bugfix: APM tracing disabled when platform not supported ([#153](https://github.com/solarwindscloud/solarwinds-apm-python/pull/153)) From a3bc65ec1b4ecce8c2af0b0e080349e1bf720396 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 8 Jun 2023 12:41:09 -0700 Subject: [PATCH 3/4] Rm unused clean_hack config --- solarwinds_apm/apm_config.py | 3 --- tests/unit/test_apm_config/fixtures/cnf_dict.py | 2 -- tests/unit/test_apm_config/test_apm_config_cnf_file.py | 7 ------- 3 files changed, 12 deletions(-) diff --git a/solarwinds_apm/apm_config.py b/solarwinds_apm/apm_config.py index 0a56520f..648283cb 100644 --- a/solarwinds_apm/apm_config.py +++ b/solarwinds_apm/apm_config.py @@ -109,7 +109,6 @@ def __init__( "enable_sanitize_sql": True, "log_trace_id": "never", "proxy": "", - "is_grpc_clean_hack_enabled": False, "transaction_filters": [], } self.agent_enabled = True @@ -722,8 +721,6 @@ def _convert_to_bool(val): ]: raise ValueError self.__config[key] = val.lower() - elif keys == ["is_grpc_clean_hack_enabled"]: - self.__config[key] = _convert_to_bool(val) elif isinstance(sub_dict, dict) and keys[-1] in sub_dict: if isinstance(sub_dict[keys[-1]], bool): val = _convert_to_bool(val) diff --git a/tests/unit/test_apm_config/fixtures/cnf_dict.py b/tests/unit/test_apm_config/fixtures/cnf_dict.py index c2657e15..1b428436 100644 --- a/tests/unit/test_apm_config/fixtures/cnf_dict.py +++ b/tests/unit/test_apm_config/fixtures/cnf_dict.py @@ -27,7 +27,6 @@ def fixture_cnf_dict(): "enableSanitizeSql": True, "logTraceId": "always", "proxy": "http://foo-bar", - "isGrpcCleanHackEnabled": True, } @pytest.fixture @@ -57,5 +56,4 @@ def fixture_cnf_dict_enabled_false(): "enableSanitizeSql": True, "logTraceId": "always", "proxy": "http://foo-bar", - "isGrpcCleanHackEnabled": True, } diff --git a/tests/unit/test_apm_config/test_apm_config_cnf_file.py b/tests/unit/test_apm_config/test_apm_config_cnf_file.py index 86ac1d62..8b0cdf73 100644 --- a/tests/unit/test_apm_config/test_apm_config_cnf_file.py +++ b/tests/unit/test_apm_config/test_apm_config_cnf_file.py @@ -133,7 +133,6 @@ def test_update_with_cnf_file_all_valid( assert resulting_config.get("enable_sanitize_sql") == True assert resulting_config.get("log_trace_id") == "always" assert resulting_config.get("proxy") == "http://foo-bar" - assert resulting_config.get("is_grpc_clean_hack_enabled") == True # update_transaction_filters was called mock_update_txn_filters.assert_called_once_with(fixture_cnf_dict) @@ -181,7 +180,6 @@ def test_update_with_cnf_file_mostly_invalid( "enableSanitizeSql": "foo", "log_trace_id": "not-never-always-etc", "proxy": "foo", - "isGrpcCleanHackEnabled": "foo", } mock_get_cnf_dict = mocker.patch( "solarwinds_apm.apm_config.SolarWindsApmConfig.get_cnf_dict" @@ -214,7 +212,6 @@ def test_update_with_cnf_file_mostly_invalid( assert resulting_config.get("enable_sanitize_sql") == True assert resulting_config.get("log_trace_id") == "never" assert resulting_config.get("proxy") == "" - assert resulting_config.get("is_grpc_clean_hack_enabled") == False # Meanwhile these are pretty open assert resulting_config.get("collector") == "False" assert resulting_config.get("hostname_alias") == "False" @@ -263,7 +260,6 @@ def test_update_with_cnf_file_and_all_validenv_vars( "SW_APM_ENABLE_SANITIZE_SQL": "false", "SW_APM_LOG_TRACE_ID": "never", "SW_APM_PROXY": "http://other-foo-bar", - "SW_APM_IS_GRPC_CLEAN_HACK_ENABLED": "false", }) mock_update_txn_filters = mocker.patch( "solarwinds_apm.apm_config.SolarWindsApmConfig.update_transaction_filters" @@ -305,7 +301,6 @@ def test_update_with_cnf_file_and_all_validenv_vars( assert resulting_config.get("enable_sanitize_sql") == False assert resulting_config.get("log_trace_id") == "never" assert resulting_config.get("proxy") == "http://other-foo-bar" - assert resulting_config.get("is_grpc_clean_hack_enabled") == False # Restore old collector if old_collector: @@ -347,7 +342,6 @@ def test_update_with_cnf_file_and_several_invalid_env_vars( "SW_APM_ENABLE_SANITIZE_SQL": "other-foo-bar", "SW_APM_LOG_TRACE_ID": "other-foo-bar", "SW_APM_PROXY": "other-foo-bar", - "SW_APM_IS_GRPC_CLEAN_HACK_ENABLED": "other-foo-bar", }) mock_update_txn_filters = mocker.patch( "solarwinds_apm.apm_config.SolarWindsApmConfig.update_transaction_filters" @@ -386,7 +380,6 @@ def test_update_with_cnf_file_and_several_invalid_env_vars( assert resulting_config.get("enable_sanitize_sql") == True assert resulting_config.get("log_trace_id") == "always" assert resulting_config.get("proxy") == "http://foo-bar" - assert resulting_config.get("is_grpc_clean_hack_enabled") == True # These are still valid, so env_var > cnf_file assert resulting_config.get("collector") == "False" From 3f17fa2b9bc1b30b9e709a33488ecdc6fa9483bb Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 8 Jun 2023 12:42:35 -0700 Subject: [PATCH 4/4] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eca3e902..944455b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add log warning when `set_transaction_name` with empty name ([#162](https://github.com/solarwindscloud/solarwinds-apm-python/pull/162)) + +- Removed unused is_grpc_clean_hack_enabled config storage ([#165](https://github.com/solarwindscloud/solarwinds-apm-python/pull/165)) + ## [0.12.0](https://github.com/solarwindscloud/solarwinds-apm-python/releases/tag/rel-0.12.0) - 2023-06-01 ### Changed - Bugfix: APM tracing disabled when platform not supported ([#153](https://github.com/solarwindscloud/solarwinds-apm-python/pull/153))