From 790cbc9a729ea29d9d6eb37119fb4cb64596ef2a Mon Sep 17 00:00:00 2001 From: Peder Hovdan Andresen <107681714+pederhan@users.noreply.github.com> Date: Wed, 4 Sep 2024 11:21:40 +0200 Subject: [PATCH] Change log statement levels, change default level to INFO (#85) * Change log statement levels, remove unused * Refactor log level validator and serializer * Add pyright config * Change default log level --- config.sample.toml | 2 +- pyproject.toml | 3 ++ zabbix_auto_config/__init__.py | 2 +- zabbix_auto_config/models.py | 14 ++++++--- zabbix_auto_config/processing.py | 43 ++++++++++++--------------- zabbix_auto_config/pyzabbix/client.py | 6 ++++ 6 files changed, 40 insertions(+), 30 deletions(-) diff --git a/config.sample.toml b/config.sample.toml index 9965d96..67c9132 100644 --- a/config.sample.toml +++ b/config.sample.toml @@ -9,7 +9,7 @@ host_modifier_dir = "path/to/host_modifier_dir/" db_uri = "dbname='zac' user='zabbix' host='localhost' password='secret' port=5432 connect_timeout=2" # Log level for the application. -log_level = "DEBUG" +log_level = "INFO" # Health status for each ZAC process. health_file = "/tmp/zac_health.json" diff --git a/pyproject.toml b/pyproject.toml index a44f65f..f3a238d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -75,3 +75,6 @@ extend-select = [ force-single-line = true # Add annotations import to every file required-imports = ["from __future__ import annotations"] + +[tool.pyright] +pythonVersion = "3.8" diff --git a/zabbix_auto_config/__init__.py b/zabbix_auto_config/__init__.py index 2ec58f8..9d885ad 100644 --- a/zabbix_auto_config/__init__.py +++ b/zabbix_auto_config/__init__.py @@ -280,7 +280,7 @@ def main() -> None: time.sleep(1) - logging.debug( + logging.info( "Queues: %s", ", ".join([str(queue.qsize()) for queue in source_hosts_queues]), ) diff --git a/zabbix_auto_config/models.py b/zabbix_auto_config/models.py index f19a89c..f7076d3 100644 --- a/zabbix_auto_config/models.py +++ b/zabbix_auto_config/models.py @@ -143,7 +143,7 @@ class ZacSettings(ConfigBaseModel): source_collector_dir: str host_modifier_dir: str db_uri: str - log_level: int = Field(logging.DEBUG, description="The log level to use.") + log_level: int = Field(logging.INFO, description="The log level to use.") health_file: Optional[Path] = None failsafe_file: Optional[Path] = None failsafe_ok_file: Optional[Path] = None @@ -164,7 +164,7 @@ def _validate_file_path( return v @field_serializer("log_level") - def _serialize_log_level(self, v: str) -> str: + def _serialize_log_level(self, v: int) -> str: """Serializes the log level as a string. Ensures consistent semantics between loading/storing log level in config. E.g. we dump `"INFO"` instead of `20`. @@ -176,6 +176,13 @@ def _serialize_log_level(self, v: str) -> str: def _validate_log_level(cls, v: Any) -> int: """Validates the log level and converts it to an integer. The log level can be specified as an integer or a string.""" + # NOTE: this is basically an overcomplicated version of + # `logging.getLevelName(v)`, but it's necessary for 2 reasons: + # 1. We want to validate that the level is a valid log level. + # `logging.getLevelName(v)` doesn't raise an error if `v` is invalid. + # It just returns `Level `, which is not helpful. + # 2. `logging.getLevelName(v)` with string arguments + # is deprecated in Python 3.10. if isinstance(v, int): if v not in logging._levelToName: raise ValueError( @@ -184,8 +191,7 @@ def _validate_log_level(cls, v: Any) -> int: return v elif isinstance(v, str): v = v.upper() - level_int = logging._nameToLevel.get(v, None) - if level_int is None: + if (level_int := logging._nameToLevel.get(v)) is None: raise ValueError( f"Invalid log level: {v} is not a valid log level name." ) diff --git a/zabbix_auto_config/processing.py b/zabbix_auto_config/processing.py index b9f3698..8089586 100644 --- a/zabbix_auto_config/processing.py +++ b/zabbix_auto_config/processing.py @@ -87,7 +87,6 @@ def run(self) -> None: break if self.next_update > datetime.datetime.now(): - # logging.debug(f"Waiting for next update {self.next_update.isoformat()}") time.sleep(1) continue @@ -330,7 +329,7 @@ def work(self) -> None: source = source_hosts["source"] hosts = source_hosts["hosts"] - logging.debug( + logging.info( "Handling %d hosts from source, '%s', from queue. Current queue size: %d", len(source_hosts["hosts"]), source, @@ -350,14 +349,12 @@ def handle_source_host( if current_host == host: return HostAction.NO_CHANGE else: - # logging.debug(f"Replaced host <{host['hostname']}> from source <{source}>") cursor.execute( f"UPDATE {self.db_source_table} SET data = %s WHERE data->>'hostname' = %s AND data->'sources' ? %s", [host.model_dump_json(), host.hostname, source], ) return HostAction.UPDATE else: - # logging.debug(f"Inserted host <{host['hostname']}> from source <{source}>") cursor.execute( f"INSERT INTO {self.db_source_table} (data) VALUES (%s)", [host.model_dump_json()], @@ -507,17 +504,14 @@ def handle_host( if current_host: if current_host == host: - # logging.debug(f"Host <{host['hostname']}> from source <{source}> is equal to current host") return HostAction.NO_CHANGE else: - # logging.debug(f"Replaced host <{host['hostname']}> from source <{source}>") cursor.execute( f"UPDATE {self.db_hosts_table} SET data = %s WHERE data->>'hostname' = %s", [host.model_dump_json(), host.hostname], ) return HostAction.UPDATE else: - # logging.debug(f"Inserted host <{host['hostname']}> from source <{source}>") cursor.execute( f"INSERT INTO {self.db_hosts_table} (data) VALUES (%s)", [host.model_dump_json()], @@ -671,6 +665,7 @@ def __init__( ver = self.api.apiinfo.version() self.zabbix_version = Version(ver) + logging.info("Connected to Zabbix API version: %s", ver) def work(self) -> None: start_time = time.time() @@ -1209,18 +1204,18 @@ def do_update(self) -> None: db_hostnames.intersection(zabbix_manual_hostnames) ) - logging.debug("Total in zabbix: %d", len(zabbix_hostnames)) - logging.debug("Total in db: %d", len(db_hostnames)) - logging.debug("Manual in zabbix: %d", len(zabbix_manual_hostnames)) - logging.debug("Manual and in source: %d", len(hostnames_in_manual_and_source)) - logging.debug( + logging.info("Total in zabbix: %d", len(zabbix_hostnames)) + logging.info("Total in db: %d", len(db_hostnames)) + logging.info("Manual in zabbix: %d", len(zabbix_manual_hostnames)) + logging.info("Manual and in source: %d", len(hostnames_in_manual_and_source)) + logging.info( "Manual and in source: %s", " ".join(hostnames_in_manual_and_source[:10]) ) - logging.debug("Only in zabbix: %d", len(hostnames_to_remove)) - logging.debug("Only in zabbix: %s", " ".join(hostnames_to_remove[:10])) - logging.debug("Only in db: %d", len(hostnames_to_add)) - logging.debug("Only in db: %s", " ".join(hostnames_to_add[:10])) - logging.debug("In both: %d", len(hostnames_in_both)) + logging.info("Only in zabbix: %d", len(hostnames_to_remove)) + logging.info("Only in zabbix: %s", " ".join(hostnames_to_remove[:10])) + logging.info("Only in db: %d", len(hostnames_to_add)) + logging.info("Only in db: %s", " ".join(hostnames_to_add[:10])) + logging.info("In both: %d", len(hostnames_in_both)) # Check if we have too many hosts to add/remove check_failsafe(self.settings, hostnames_to_add, hostnames_to_remove) @@ -1315,7 +1310,7 @@ def do_update(self) -> None: or zabbix_interface.port != interface.port or zabbix_interface.useip != useip ): - logging.debug( + logging.info( "DNS interface of type %s for host '%s' is configured wrong", interface.type, db_host.hostname, @@ -1336,7 +1331,7 @@ def do_update(self) -> None: str(details_dict.get(k)) == str(v) for k, v in interface.details.items() ): - logging.debug( + logging.info( "SNMP interface for host '%s' differs from source data. Fixing.", db_host.hostname, ) @@ -1446,7 +1441,7 @@ def __init__( def clear_templates(self, templates: List[Template], host: Host) -> None: if self.config.dryrun: - logging.debug( + logging.info( "DRYRUN: Clearing templates %s on host: %s", ", ".join(t.host for t in templates), host, @@ -1469,7 +1464,7 @@ def set_templates(self, templates: List[Template], host: Host) -> None: to_add = ", ".join(f"{t.host!r}" for t in templates) if self.config.dryrun: - logging.debug("DRYRUN: Setting templates %s on host: %s", to_add, host) + logging.info("DRYRUN: Setting templates %s on host: %s", to_add, host) return try: @@ -1595,7 +1590,7 @@ def set_hostgroups(self, host: Host, hostgroups: List[HostGroup]) -> None: """Set host groups on a host given a list of host groups.""" to_add = ", ".join(f"{hg.name!r}" for hg in hostgroups) if self.config.dryrun: - logging.debug("DRYRUN: Setting hostgroups %s on host: %s", to_add, host) + logging.info("DRYRUN: Setting hostgroups %s on host: %s", to_add, host) return try: self.api.set_host_hostgroups(host, hostgroups) @@ -1606,7 +1601,7 @@ def set_hostgroups(self, host: Host, hostgroups: List[HostGroup]) -> None: def create_hostgroup(self, hostgroup_name: str) -> Optional[str]: if self.config.dryrun: - logging.debug("DRYRUN: Creating hostgroup: '%s'", hostgroup_name) + logging.info("DRYRUN: Creating hostgroup: '%s'", hostgroup_name) return None logging.debug("Creating hostgroup: '%s'", hostgroup_name) @@ -1637,7 +1632,7 @@ def create_extra_hostgroups(self, existing_hostgroups: List[HostGroup]) -> None: def create_templategroup(self, templategroup_name: str) -> Optional[str]: if self.config.dryrun: - logging.debug("DRYRUN: Creating template group: '%s'", templategroup_name) + logging.info("DRYRUN: Creating template group: '%s'", templategroup_name) return None logging.debug("Creating template group: '%s'", templategroup_name) diff --git a/zabbix_auto_config/pyzabbix/client.py b/zabbix_auto_config/pyzabbix/client.py index ea1643e..ed41911 100644 --- a/zabbix_auto_config/pyzabbix/client.py +++ b/zabbix_auto_config/pyzabbix/client.py @@ -257,6 +257,12 @@ def do_request( try: response = self.session.post(self.url, json=request_json) except Exception as e: + logging.error( + "Failed to send request to %s (%s) with params %s", + self.url, + method, + params, + ) raise ZabbixAPIRequestError( f"Failed to send request to {self.url} ({method}) with params {params}", params=params,