-
Notifications
You must be signed in to change notification settings - Fork 107
Enable debug logging using the PYTHON_ENABLE_DEBUG_LOGGING flag #939
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6c19778
08a5ee6
53ba079
c6573b4
9afced4
b8d0400
cc4a071
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
# Copyright (c) Microsoft Corporation. All rights reserved. | ||
# Licensed under the MIT License. | ||
import typing | ||
import os | ||
from unittest.mock import patch | ||
|
||
from azure_functions_worker import testutils | ||
from azure_functions_worker.constants import PYTHON_ENABLE_DEBUG_LOGGING | ||
from azure_functions_worker.testutils import TESTS_ROOT, remove_path | ||
|
||
HOST_JSON_TEMPLATE_WITH_LOGLEVEL_INFO = """\ | ||
{ | ||
"version": "2.0", | ||
"logging": { | ||
"logLevel": { | ||
"default": "Information" | ||
} | ||
}, | ||
"functionTimeout": "00:05:00" | ||
} | ||
""" | ||
|
||
|
||
class TestDebugLoggingEnabledFunctions(testutils.WebHostTestCase): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checking - didn't you have to update the host.json file for enabling the logging? Or is it enabled by default now? If yes - Do we need another test where it is not enabled by default and we are still enabling the flag and checking that the new debug log doesn't get logged? Ideally - there is a potential issue - There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, will add a test case for that. As discussed, the host.json logging filter is set at function level while this flag is at function app level for the shared root logger. It is not possible to define the flag validity if functions have inconsistent log levels in the host.json. Separate investigation/change will be made to warn customers in case of inconsistent host.json log level and PYTHON_ENABLE_DEBUG_LOGGING value being set. |
||
@classmethod | ||
def setUpClass(cls): | ||
os_environ = os.environ.copy() | ||
os_environ[PYTHON_ENABLE_DEBUG_LOGGING] = '1' | ||
cls._patch_environ = patch.dict('os.environ', os_environ) | ||
cls._patch_environ.start() | ||
super().setUpClass() | ||
|
||
@classmethod | ||
def tearDownClass(self): | ||
super().tearDownClass() | ||
self._patch_environ.stop() | ||
|
||
@classmethod | ||
def get_script_dir(cls): | ||
return testutils.UNIT_TESTS_FOLDER / 'log_filtering_functions' | ||
|
||
def test_debug_logging_enabled(self): | ||
r = self.webhost.request('GET', 'debug_logging') | ||
self.assertEqual(r.status_code, 200) | ||
self.assertEqual(r.text, 'OK-debug') | ||
|
||
def check_log_debug_logging_enabled(self, host_out: typing.List[str]): | ||
self.assertIn('logging info', host_out) | ||
self.assertIn('logging warning', host_out) | ||
self.assertIn('logging debug', host_out) | ||
self.assertIn('logging error', host_out) | ||
|
||
|
||
class TestDebugLoggingDisabledFunctions(testutils.WebHostTestCase): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add PyDoc comments to describe what the tests do? That'll help in the future. |
||
@classmethod | ||
def setUpClass(cls): | ||
os_environ = os.environ.copy() | ||
os_environ[PYTHON_ENABLE_DEBUG_LOGGING] = '0' | ||
cls._patch_environ = patch.dict('os.environ', os_environ) | ||
cls._patch_environ.start() | ||
super().setUpClass() | ||
|
||
@classmethod | ||
def tearDownClass(self): | ||
super().tearDownClass() | ||
self._patch_environ.stop() | ||
|
||
@classmethod | ||
def get_script_dir(cls): | ||
return testutils.UNIT_TESTS_FOLDER / 'log_filtering_functions' | ||
|
||
def test_debug_logging_disabled(self): | ||
r = self.webhost.request('GET', 'debug_logging') | ||
self.assertEqual(r.status_code, 200) | ||
self.assertEqual(r.text, 'OK-debug') | ||
|
||
def check_log_debug_logging_disabled(self, host_out: typing.List[str]): | ||
self.assertIn('logging info', host_out) | ||
self.assertIn('logging warning', host_out) | ||
self.assertIn('logging error', host_out) | ||
self.assertNotIn('logging debug', host_out) | ||
|
||
|
||
class TestLogFilteringFunctions(testutils.WebHostTestCase): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please change the class name to describe what this test is specifically testing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also the same comments of comments. |
||
@classmethod | ||
def setUpClass(cls): | ||
host_json = TESTS_ROOT / cls.get_script_dir() / 'host.json' | ||
|
||
with open(host_json, 'w+') as f: | ||
f.write(HOST_JSON_TEMPLATE_WITH_LOGLEVEL_INFO) | ||
|
||
os_environ = os.environ.copy() | ||
os_environ[PYTHON_ENABLE_DEBUG_LOGGING] = '1' | ||
cls._patch_environ = patch.dict('os.environ', os_environ) | ||
cls._patch_environ.start() | ||
super().setUpClass() | ||
|
||
@classmethod | ||
def tearDownClass(self): | ||
host_json = TESTS_ROOT / self.get_script_dir() / 'host.json' | ||
remove_path(host_json) | ||
|
||
super().tearDownClass() | ||
self._patch_environ.stop() | ||
|
||
@classmethod | ||
def get_script_dir(cls): | ||
return testutils.UNIT_TESTS_FOLDER / 'log_filtering_functions' | ||
|
||
def test_debug_logging_filtered(self): | ||
r = self.webhost.request('GET', 'debug_logging') | ||
self.assertEqual(r.status_code, 200) | ||
self.assertEqual(r.text, 'OK-debug') | ||
|
||
def check_log_debug_logging_filtered(self, host_out: typing.List[str]): | ||
self.assertIn('logging info', host_out) | ||
self.assertIn('logging warning', host_out) | ||
self.assertNotIn('logging debug', host_out) | ||
self.assertIn('logging error', host_out) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,6 @@ | |
import filecmp | ||
import typing | ||
import os | ||
import unittest | ||
import pytest | ||
|
||
from azure_functions_worker import testutils | ||
|
@@ -98,34 +97,22 @@ def check_log_async_logging(self, host_out: typing.List[str]): | |
self.assertIn('hello info', host_out) | ||
self.assertIn('and another error', host_out) | ||
|
||
@unittest.skip("Reverting the debug logs PR as host currently cannot handle" | ||
"apps with lot of debug statements. Reverting PR: " | ||
"azure-functions-python-worker/pull/745") | ||
def test_debug_logging(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need all of these tests if you are already testing this in the test_enable_debug_logging_functions.py? If not, then I guess these can be removed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, this is kept for testing flag unset case |
||
r = self.webhost.request('GET', 'debug_logging') | ||
self.assertEqual(r.status_code, 200) | ||
self.assertEqual(r.text, 'OK-debug') | ||
|
||
@unittest.skip("Reverting the debug logs PR as host currently cannot handle" | ||
"apps with lot of debug statements. Reverting PR: " | ||
"azure-functions-python-worker/pull/745") | ||
def check_log_debug_logging(self, host_out: typing.List[str]): | ||
self.assertIn('logging info', host_out) | ||
self.assertIn('logging warning', host_out) | ||
self.assertIn('logging debug', host_out) | ||
self.assertIn('logging error', host_out) | ||
self.assertNotIn('logging debug', host_out) | ||
|
||
@unittest.skip("Reverting the debug logs PR as host currently cannot handle" | ||
"apps with lot of debug statements. Reverting PR: " | ||
"azure-functions-python-worker/pull/745") | ||
def test_debug_with_user_logging(self): | ||
r = self.webhost.request('GET', 'debug_user_logging') | ||
self.assertEqual(r.status_code, 200) | ||
self.assertEqual(r.text, 'OK-user-debug') | ||
|
||
@unittest.skip("Reverting the debug logs PR as host currently cannot handle" | ||
"apps with lot of debug statements. Reverting PR: " | ||
"azure-functions-python-worker/pull/745") | ||
def check_log_debug_with_user_logging(self, host_out: typing.List[str]): | ||
self.assertIn('logging info', host_out) | ||
self.assertIn('logging warning', host_out) | ||
|
Uh oh!
There was an error while loading. Please reload this page.