Skip to content

Commit

Permalink
Move log handler call back into main
Browse files Browse the repository at this point in the history
  • Loading branch information
Iain-S committed Feb 12, 2024
1 parent 370a000 commit df90a23
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 15 deletions.
4 changes: 0 additions & 4 deletions status_function/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ status=$((status+$?))

# Run our unit tests with code coverage
echo "Running unit tests..."
export API_URL="http://some.api.url"
export PRIVATE_KEY="-----BEGIN OPENSSH PRIVATE KEY-----mykey1234-----END OPENSSH PRIVATE KEY-----" # gitleaks:allow
export AZURE_TENANT_ID="00000000-0000-0000-0000-000000000000"
export CENTRAL_LOGGING_CONNECTION_STRING="InstrumentationKey=00000000-0000-0000-0000-000000000000"
python -m coverage run \
--omit=".venv/*,tests/*" \
-m unittest discover \
Expand Down
4 changes: 2 additions & 2 deletions status_function/status/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

from status import models, settings
from status.auth import BearerAuth
from status.logutils import set_log_handler
from status.logutils import add_log_handler_once
from status.wrapper import CredentialWrapper

logging.basicConfig(
Expand All @@ -32,7 +32,6 @@
datefmt="%d/%m/%Y %I:%M:%S %p",
)
logger = logging.getLogger(__name__)
set_log_handler(__name__)

# We should only need one set of credentials.
CREDENTIALS = DefaultAzureCredential()
Expand Down Expand Up @@ -330,6 +329,7 @@ def main(mytimer: func.TimerRequest) -> None:
format="%(asctime)s %(message)s",
datefmt="%d/%m/%Y %I:%M:%S %p",
)
add_log_handler_once(__name__)
logger.warning("Status function starting.")

if mytimer.past_due:
Expand Down
8 changes: 3 additions & 5 deletions status_function/status/logutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def filter(self, record: logging.LogRecord) -> bool:
return True


def set_log_handler(name: str = "status") -> None:
def add_log_handler_once(name: str = "status") -> None:
"""Add an Azure log handler to the logger with provided name.
The log data is sent to the Azure Application Insights instance associated
Expand All @@ -47,16 +47,14 @@ def set_log_handler(name: str = "status") -> None:
Args:
name: The name of the logger instance to which we add the log handler.
Returns:
None
"""
logger = logging.getLogger(name)
log_settings = settings.get_settings()
if log_settings.CENTRAL_LOGGING_CONNECTION_STRING:
for handler in logger.handlers:
# Only allow one AzureLogHandler per logger
if isinstance(handler, AzureLogHandler):
raise RuntimeError("AzureLogHandler already added to logger.")
return

custom_dimensions = {"logger_name": f"logger_{name}"}
handler = AzureLogHandler(
Expand Down
14 changes: 10 additions & 4 deletions status_function/tests/test_function_app.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Tests for status package."""
import logging
from datetime import datetime
from types import SimpleNamespace
from unittest import TestCase, main
Expand Down Expand Up @@ -613,10 +614,15 @@ def test_bearer_auth(self):

class TestLoggingUtils(TestCase):
def test_called_twice(self):
"""Adding multiple loggers can cause large storage bills ££££."""
with self.assertRaises(RuntimeError):
status.logutils.set_log_handler("a")
status.logutils.set_log_handler("a")
"""Adding multiple loggers could cause large storage bills."""
with patch("status.settings.get_settings") as mock_get_settings:
mock_get_settings.return_value.CENTRAL_LOGGING_CONNECTION_STRING = "my-str"

with patch("status.logutils.AzureLogHandler", new=MagicMock):
status.logutils.add_log_handler_once("a")
status.logutils.add_log_handler_once("a")
handlers = logging.getLogger("a").handlers
self.assertEqual(1, len(handlers))


if __name__ == "__main__":
Expand Down

0 comments on commit df90a23

Please sign in to comment.