Skip to content
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

celery jobs should generate recordings #361

Open
apotterri opened this issue Jul 30, 2024 · 1 comment
Open

celery jobs should generate recordings #361

apotterri opened this issue Jul 30, 2024 · 1 comment
Labels
enhancement New feature or request navie-plan Plan the issue with Navie

Comments

@apotterri
Copy link
Contributor

Each time a celery job runs it should generate AppMap data (analogous to the way a request recording gets generated).

@apotterri apotterri added the enhancement New feature or request label Jul 30, 2024
@kgilpin kgilpin added the navie-plan Plan the issue with Navie label Aug 8, 2024
Copy link

github-actions bot commented Aug 8, 2024

Title

Enable AppMap Recording for Celery Jobs

Problem

Currently, when a Celery job runs in the application, it does not generate AppMap recordings. This behavior results in a lack of visibility into the execution and performance of tasks managed by Celery, which impedes the ability to trace issues or optimize task handling.

Analysis

To enable AppMap recording for Celery jobs, we need to integrate the AppMap recorder into the task execution lifecycle of Celery. Celery tasks go through a specific sequence of states: pending, started, and success (or failure). By hooking into these states, we can start a recording before the task executes and stop the recording once the task completes, successfully or otherwise.

We will extend the Celery task class to include AppMap recording functionality. This involves importing the AppMap recorder, starting the recording upon task initiation, and stopping it upon task completion or failure. The recorded AppMap data should be saved in a specified directory, similarly to how request recordings are handled.

Proposed Changes

  1. appmap/labeling/jobs.yml: Ensure Celery task classes are included in the job creation configuration:

    • Add an entry for celery.app.task.Task.apply_async under job.create.
  2. _appmap/recorder.py: Create helper functions if necessary to standardize the start and stop of recording processes. Confirm that the AppMap recorder follows the lifecycle of the task correctly.

    • Ensure methods like start_recording() and stop_recording() are suitable for re-use in the context of celery tasks.
  3. Create a new file or update an existing integration module (e.g., _appmap/integration/celery.py): Augment celery.Task to include AppMap recording steps:

    • Patch the celery.Task class such that the task lifecycle includes AppMap recording steps.
  4. _appmap/configuration.py: Ensure that the configuration for celery is correctly captured and can be enabled via the AppMap configuration.

  5. _appmap/test/test_celery.py: Add tests to validate that Celery tasks are correctly generating AppMap recordings:

    • Create and run mock Celery tasks, then verify the existence and correctness of the generated AppMaps.

Detailed File Changes

  1. appmap/labeling/jobs.yml:
    Add the Celery task class to the job creation configuration.

    job.create:
      - celery.app.task.Task.apply_async
  2. _appmap/recorder.py:
    Make sure existing methods support celery tasks recording; if not, adjust the existing methods to support additional context if necessary.

  3. Create or update an integration file _appmap/integration/celery.py:

    from celery import Task
    from appmap.recorder import Recorder
    
    class AppMapTask(Task):
        def __call__(self, *args, **kwargs):
            recorder = Recorder()
            recorder.start_recording()
            try:
                result = super().__call__(*args, **kwargs)
            finally:
                recorder.stop_recording()
            return result
  4. _appmap/configuration.py:
    Ensure the configuration loader recognizes celery-related configurations:

    class _ConfigLoader(SafeLoader):
        # existing methods ...
    
        def construct_mapping(self, node, deep=False):
            mapping = super().construct_mapping(node, deep=deep)
            # Add or update relevant config parameters related to celery
            if "enable_celery" in mapping:
                val = mapping["enable_celery"]
                if isinstance(val, str):
                    mapping["enable_celery"] = val.lower() == "true"
            return mapping
  5. _appmap/test/test_celery.py:
    Add tests ensuring that AppMaps are generated when Celery tasks execute.

    import celery
    import pytest
    from _appmap.recorder import Recorder
    
    @pytest.fixture
    def celery_app():
        app = celery.Celery('tasks', broker='memory://')
        return app
    
    def test_celery_task_appmap_generation(celery_app):
        @celery_app.task(bind=True, base=AppMapTask)
        def sample_task(self):
            return "Task completed"
        
        result = sample_task.apply_async()
        assert result.get() == "Task completed"
        # Add assertions to check the appmap file is generated
        # This might involve checking the tmp/appmap directory

By implementing these changes, Celery tasks will each generate their own AppMap recording, capturing execution details and improving observability for Celery-managed tasks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request navie-plan Plan the issue with Navie
Projects
None yet
Development

No branches or pull requests

2 participants