From d0a811575beca9fb29f94bc1e203e0c5a8d45384 Mon Sep 17 00:00:00 2001 From: Mirela Popoveniuc Date: Fri, 8 Jan 2021 11:34:08 +0000 Subject: [PATCH 1/5] Multiple minor clean-ups from previous PR and added links to PyPI and demo app. --- README.md | 6 +++ .../aws_lambda/lambda_handler.py | 3 +- codeguru_profiler_agent/model/sample.py | 2 + codeguru_profiler_agent/profiler.py | 1 + setup.py | 4 +- test/conftest.py | 6 +-- .../test_live_backend_reporting.py | 2 - test/pytestutils.py | 54 ------------------- test/pytestutils_focus.py | 8 --- test/unit/model/test_sample.py | 9 ++++ .../test_agent_configuration_merger.py | 2 +- 11 files changed, 24 insertions(+), 73 deletions(-) delete mode 100644 test/pytestutils_focus.py create mode 100644 test/unit/model/test_sample.py diff --git a/README.md b/README.md index 8941610..7bed1f1 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,12 @@ For more details, check the documentation: https://docs.aws.amazon.com/codeguru/latest/profiler-ug/what-is-codeguru-profiler.html +## How to use it + +This package is released to PyPI, so use it as any Python package from PyPI: https://pypi.org/project/codeguru-profiler-agent + +For a demo application that uses this agent, check our [demo application](https://github.com/aws-samples/aws-codeguru-profiler-demo-application). + ## Release to PyPI Use the `setup.py` script to create the archive. diff --git a/codeguru_profiler_agent/aws_lambda/lambda_handler.py b/codeguru_profiler_agent/aws_lambda/lambda_handler.py index aebf76c..f31968c 100644 --- a/codeguru_profiler_agent/aws_lambda/lambda_handler.py +++ b/codeguru_profiler_agent/aws_lambda/lambda_handler.py @@ -22,8 +22,7 @@ def load_handler(bootstrap_module, env=os.environ, original_handler_env_key=HAND if hasattr(bootstrap_module, '_get_handler'): customer_handler_function = bootstrap_module._get_handler(original_handler_name) else: - # Support for python 3.6 bootstrap which is different. It is starting to be quite hacky at this point. - # We will need to discuss if it is worth handling this case and if we can do it in a better way. + # TODO FIXME Review if the support for python 3.6 bootstrap can be improved. # This returns both a init_handler and the function, we apply the init right away as we are in init process init_handler, customer_handler_function = bootstrap_module._get_handlers( handler=original_handler_name, diff --git a/codeguru_profiler_agent/model/sample.py b/codeguru_profiler_agent/model/sample.py index 7b1178f..8108315 100644 --- a/codeguru_profiler_agent/model/sample.py +++ b/codeguru_profiler_agent/model/sample.py @@ -1,4 +1,6 @@ class Sample: + __slots__ = ["stacks", "attempted_sample_threads_count", "seen_threads_count"] + def __init__(self, stacks, attempted_sample_threads_count=0, seen_threads_count=0): """ :param stacks: list of lists; each list is a list of Frame object representing a thread stack in bottom (of thread stack) to top (of thread stack) order diff --git a/codeguru_profiler_agent/profiler.py b/codeguru_profiler_agent/profiler.py index 99b54d0..854f07c 100644 --- a/codeguru_profiler_agent/profiler.py +++ b/codeguru_profiler_agent/profiler.py @@ -31,6 +31,7 @@ # [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory. # https://bandit.readthedocs.io/en/latest/plugins/b108_hardcoded_tmp_directory.html # This file can be used by the customer as kill switch for the Profiler. +# TODO FIXME Consider making this work for Windows. KILLSWITCH_FILEPATH = "/var/tmp/killProfiler" #nosec logger = logging.getLogger(__name__) diff --git a/setup.py b/setup.py index d22074b..2f16743 100755 --- a/setup.py +++ b/setup.py @@ -30,8 +30,8 @@ def find_version(*file_paths): description="The Python agent to be used for Amazon CodeGuru Profiler", long_description="https://docs.aws.amazon.com/codeguru/latest/profiler-ug/what-is-codeguru-profiler.html", author="Amazon Web Services", - url="https://github.com/aws", - download_url="https://github.com/aws", + url="https://github.com/aws/amazon-codeguru-profiler-python-agent", + download_url="https://github.com/aws/amazon-codeguru-profiler-python-agent", classifiers=[ "Programming Language :: Python :: 3", "Development Status :: 5 - Production/Stable", diff --git a/test/conftest.py b/test/conftest.py index a192787..0288c3a 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,4 +1,2 @@ -import pytest - -# Enable pytestutils @focus decoration -pytest_plugins = "test.pytestutils_focus" +# Enable pytestutils decoration +pytest_plugins = "test.pytestutils" diff --git a/test/integration/test_live_backend_reporting.py b/test/integration/test_live_backend_reporting.py index a911239..0051a82 100644 --- a/test/integration/test_live_backend_reporting.py +++ b/test/integration/test_live_backend_reporting.py @@ -18,8 +18,6 @@ @pytest.mark.skipif( - # TODO FIXME Remove the conditions for skipping this on Amazonian fleets when we move fully to GitHub. - socket.gethostname().startswith("pb-worker-prod") or socket.gethostname().startswith("coverlay-") or socket.getfqdn().endswith("internal.cloudapp.net"), # hosts running ubuntu and windows in GitHub socket.getfqdn().endswith("ip6.arpa"), # hosts running macs in GitHub diff --git a/test/pytestutils.py b/test/pytestutils.py index cdb0d0d..2ad5e39 100644 --- a/test/pytestutils.py +++ b/test/pytestutils.py @@ -5,8 +5,6 @@ _pytestutils_before_global_counter = 0 _pytestutils_before_global_lock = threading.Lock() -_pytestutils_is_there_any_test_marked_with_focus = False - def before(before_function): """ @@ -56,55 +54,3 @@ def test_it_does_stuff(self): return pytest.fixture( autouse=True, name=unique_fixture_name)(before_function) - - -def focus(decorated_test): - """ - Decorator for tagging a function or class as being in focus, and to switch the test suite execution to "focused - execution mode". When the test suite is in "focused execution mode", only functions and classes marked with @focus - are run, all others are skipped. - - When there are no functions/classes marked with @focus, the test suite goes back to the usual mode, and all tests - are run. - - The focused execution mode is useful for quickly and without needing to edit any more configuration/files selecting - only a subset of the tests for execution, to speed up testing cycles. - - This decorator is inspired by similar IDE features (e.g. https://blogs.oracle.com/geertjan/run-focused-test-method ) - and other test libraries (e.g. https://medium.com/table-xi/focus-your-rspec-workflow-4cd5798d2a3e ). - - Limitation when used with Pytest < 3.6: @focus does not extend to nested classes, see - https://github.com/pytest-dev/pytest/issues/199 and https://github.com/pytest-dev/pytest/pull/3317 for details. - - This feature is broken into several pieces: - * The @focus decorator sets a global variable to trigger the focused execution mode, and additionally marks any - test method or class with the "pytestutils_focus" marker - * In pytestutils_focus.py a pytest plugin is provided that when in focused execution mode, skips any tests not - marked with the "pytestutils_focus" marker - * In conftest.py we enable the pytest plugin - """ - _validate_focus_enabled() - - global _pytestutils_is_there_any_test_marked_with_focus - - _pytestutils_is_there_any_test_marked_with_focus = True - - return pytest.mark.pytestutils_focus(decorated_test) - - -def _validate_focus_enabled(): - if os.environ.get("PYTESTUTILS_ALLOW_FOCUS") in ("1", "true", "yes"): - return - raise RuntimeError(""" -Found tests annotated with @focus decorator, but the PYTESTUTILS_ALLOW_FOCUS environment variable is not set in the -current environment. - -If you found this error in a CI environment, it means someone committed test code with a @focus annotation -- please -check for and remove it from the codebase. - -If you found this error and you wanted to use @focus for your own development work, please add a PYTESTUTILS_ALLOW_FOCUS -enviroment variable set to 1 (e.g. `export PYTESTUTILS_ALLOW_FOCUS=1`) to your execution environment to make this error -go away. - -Thanks for using @focus! - """) diff --git a/test/pytestutils_focus.py b/test/pytestutils_focus.py deleted file mode 100644 index 55eacb6..0000000 --- a/test/pytestutils_focus.py +++ /dev/null @@ -1,8 +0,0 @@ -import pytest -import test.pytestutils as pytestutils - - -def pytest_runtest_setup(item): - if pytestutils._pytestutils_is_there_any_test_marked_with_focus and not item.get_marker( - name='pytestutils_focus'): - pytest.skip("Test skipped in focus mode") diff --git a/test/unit/model/test_sample.py b/test/unit/model/test_sample.py new file mode 100644 index 0000000..8574f75 --- /dev/null +++ b/test/unit/model/test_sample.py @@ -0,0 +1,9 @@ +from codeguru_profiler_agent.model.sample import Sample +from pympler.asizeof import asizeof + + +class TestSample: + def test_sizeof_sample(self): + sample = Sample(stacks="foo") + assert asizeof(sample) == 136 + diff --git a/test/unit/reporter/test_agent_configuration_merger.py b/test/unit/reporter/test_agent_configuration_merger.py index 2ebcfbe..5c8fb37 100644 --- a/test/unit/reporter/test_agent_configuration_merger.py +++ b/test/unit/reporter/test_agent_configuration_merger.py @@ -87,4 +87,4 @@ def test_it_leaves_other_values_untouched(self): assert AgentConfiguration.get().sampling_interval == timedelta(milliseconds=1) assert AgentConfiguration.get().minimum_time_reporting == timedelta(seconds=1) assert AgentConfiguration.get().reporting_interval == timedelta(minutes=1) - assert AgentConfiguration.get().max_stack_depth == 998 \ No newline at end of file + assert AgentConfiguration.get().max_stack_depth == 998 From b2c4f322910080b5c6d45d8314ad2e511de56a26 Mon Sep 17 00:00:00 2001 From: Mirela Popoveniuc Date: Fri, 8 Jan 2021 11:39:16 +0000 Subject: [PATCH 2/5] Remove test that sometimes fails and does not bring any value. --- test/unit/model/test_sample.py | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 test/unit/model/test_sample.py diff --git a/test/unit/model/test_sample.py b/test/unit/model/test_sample.py deleted file mode 100644 index 8574f75..0000000 --- a/test/unit/model/test_sample.py +++ /dev/null @@ -1,9 +0,0 @@ -from codeguru_profiler_agent.model.sample import Sample -from pympler.asizeof import asizeof - - -class TestSample: - def test_sizeof_sample(self): - sample = Sample(stacks="foo") - assert asizeof(sample) == 136 - From 3329fe2bcbeb1c51fad71ba38682e55c469301f6 Mon Sep 17 00:00:00 2001 From: Mirela Popoveniuc Date: Fri, 8 Jan 2021 11:58:15 +0000 Subject: [PATCH 3/5] Use the existing wait_for from help_utils. --- test/unit/test_profiler_runner.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/test/unit/test_profiler_runner.py b/test/unit/test_profiler_runner.py index f63d883..7a10533 100644 --- a/test/unit/test_profiler_runner.py +++ b/test/unit/test_profiler_runner.py @@ -1,7 +1,7 @@ from datetime import timedelta -import time from codeguru_profiler_agent.reporter.agent_configuration import AgentConfiguration from test.pytestutils import before +from test.help_utils import wait_for from mock import MagicMock from codeguru_profiler_agent.profiler_runner import ProfilerRunner @@ -86,17 +86,7 @@ def test_when_orchestrator_says_no_to_profiler(self): # calling start in this test, it will start the scheduler and because initial delay is 0 it will execute now self.profiler_runner.start() # still it is safer to wait until the new config has been applied - wait_until(lambda: AgentConfiguration.get().should_profile == False) + wait_for(lambda: AgentConfiguration.get().should_profile is False) assert self.profiler_runner.scheduler._get_next_delay_seconds() == 150 self.mock_collector.add.assert_not_called() - - -def wait_until(predicate, max_wait_seconds=1, period_seconds=0.1): - start = time.time() - timeout = start + max_wait_seconds - while time.time() < timeout: - if predicate(): - return True - time.sleep(period_seconds) - raise AssertionError("Predicate was never true after waiting for " + str(max_wait_seconds) + " seconds") From 4a0837648b841c90182c5bc51796bf1b6504c135 Mon Sep 17 00:00:00 2001 From: Mirela Popoveniuc Date: Fri, 8 Jan 2021 12:10:25 +0000 Subject: [PATCH 4/5] Trying to fix a test that fails sometimes. --- test/unit/test_profiler_runner.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/test_profiler_runner.py b/test/unit/test_profiler_runner.py index 7a10533..f4ba436 100644 --- a/test/unit/test_profiler_runner.py +++ b/test/unit/test_profiler_runner.py @@ -82,11 +82,11 @@ def test_when_disabler_say_to_stop(self): def test_when_orchestrator_says_no_to_profiler(self): self.agent_configuration = AgentConfiguration(should_profile=False, sampling_interval=timedelta(seconds=2), - reporting_interval=timedelta(seconds=150)) + reporting_interval=timedelta(seconds=151)) # calling start in this test, it will start the scheduler and because initial delay is 0 it will execute now self.profiler_runner.start() # still it is safer to wait until the new config has been applied - wait_for(lambda: AgentConfiguration.get().should_profile is False) + wait_for(lambda: AgentConfiguration.get().reporting_interval.total_seconds() == 151) - assert self.profiler_runner.scheduler._get_next_delay_seconds() == 150 + assert self.profiler_runner.scheduler._get_next_delay_seconds() == 151 self.mock_collector.add.assert_not_called() From a0739c89cd91f65563cac5d82b2ed7124c06e9f0 Mon Sep 17 00:00:00 2001 From: Mirela Popoveniuc Date: Fri, 8 Jan 2021 13:34:14 +0000 Subject: [PATCH 5/5] Addressed comments from PR. --- README.md | 2 +- test/integration/test_live_backend_reporting.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 7bed1f1..1927ecd 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ For more details, check the documentation: https://docs.aws.amazon.com/codeguru/ This package is released to PyPI, so use it as any Python package from PyPI: https://pypi.org/project/codeguru-profiler-agent -For a demo application that uses this agent, check our [demo application](https://github.com/aws-samples/aws-codeguru-profiler-demo-application). +For a demo application that uses this agent, check our [demo application](https://github.com/aws-samples/aws-codeguru-profiler-python-demo-application). ## Release to PyPI diff --git a/test/integration/test_live_backend_reporting.py b/test/integration/test_live_backend_reporting.py index 0051a82..a79f231 100644 --- a/test/integration/test_live_backend_reporting.py +++ b/test/integration/test_live_backend_reporting.py @@ -18,7 +18,6 @@ @pytest.mark.skipif( - socket.gethostname().startswith("coverlay-") or socket.getfqdn().endswith("internal.cloudapp.net"), # hosts running ubuntu and windows in GitHub socket.getfqdn().endswith("ip6.arpa"), # hosts running macs in GitHub reason="This integration test is skipped on any shared fleet from Amazon or GitHub "