feat: Add support to fetch and match worker logs #183

leongdl · 2025-04-03T20:57:17Z

What was the problem/requirement? (What/Why)

We are adding some new features related to worker features. Currently the test fixtures only have features related to Job logs. I am extending the test fixtures to worker logs.

What was the solution? (How)

Add new fixtures and helpers to get worker log group, log stream.
Small refactor to log class to re-use existing log search logic for test verification.
Allows tests to use assert_pattern_in_log and assert_pattern_not_in_log.

What is the impact of this change?

We will have great test coverage!

How was this change tested?

hatch build
hatch run fmt
hatch run all:lint
Used the new worker log functions in a new E2E test case for worker agent.

Was this change documented?

Not Applicable.

Is this a breaking change?

Not Applicable.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

ttblanchard · 2025-04-03T21:27:44Z

src/deadline_test_fixtures/deadline/worker.py

@@ -252,6 +264,46 @@ def set_stopped_status(self):
            LOG.exception(f"Failed to update worker status: {error}")
            raise

+    def get_worker_logs(self) -> Optional[WorkerLogConfig]:


To make this a little more clear, should we change this method to _get_worker_log_config?

Sure, we can do that.

I was thinking if other tests may want to get the WorkerLogConfig. Both works.

ttblanchard · 2025-04-03T21:33:41Z

src/deadline_test_fixtures/deadline/worker.py

+        # Default, no log group yet.
+        return None
+
+    def get_all_worker_logs(self, *, logs_client: botocore.client.BaseClient) -> WorkerLog:


Perhaps just get_logs? Would be more consistent with our Job implementation

jairaws · 2025-04-04T16:12:05Z

src/deadline_test_fixtures/deadline/worker.py

@@ -252,6 +264,46 @@ def set_stopped_status(self):
            LOG.exception(f"Failed to update worker status: {error}")
            raise

+    def _get_worker_logs(self) -> Optional[WorkerLogConfig]:
+        """Get the log group and log stream for the worker. Retain the API structure"""
+        response = self.deadline_client.get_worker(


Q: do we have a data class for GetWorker response?

We don't have typing in this library for boto classes :(

The data class comes from the design and structure of this library. There's no use of boto3 generated types

Could we type it as Dict[str, Any]?

jairaws · 2025-04-04T16:16:10Z

src/deadline_test_fixtures/deadline/worker.py

+            fleetId=self.configuration.fleet.id,
+            workerId=self.worker_id,
+        )
+        if log_config := response["log"]:


If log isn't a key in response, wouldn't this line throw an exception?

good call, I'll change it to .get()

jairaws · 2025-04-04T16:16:41Z

src/deadline_test_fixtures/deadline/worker.py

+
+    def get_logs(self, *, logs_client: botocore.client.BaseClient) -> WorkerLog:
+        # Get the worker log group and stream from the service.
+        log_config = self._get_worker_logs()


jairaws · 2025-04-04T16:18:13Z

src/deadline_test_fixtures/deadline/worker.py

+                logStreamNames=[log_config.cloudwatch_log_stream],
+            ),
+        )
+        log_events = filter_log_events_pages.build_full_result()


there's no typing for boto in this package.

jairaws · 2025-04-04T16:19:00Z

src/deadline_test_fixtures/deadline/worker.py

+        )
+        log_events = filter_log_events_pages.build_full_result()
+        log_events = [CloudWatchLogEvent.from_api_response(e) for e in log_events["events"]]
+        # For debugging test cases.


Could we remove this comment? Doesn't seem to be adding any value.

We do, otherwise everyone wants to remove LOG.info :)

godobyte · 2025-04-04T16:18:36Z

src/deadline_test_fixtures/deadline/worker.py

@@ -252,6 +264,46 @@ def set_stopped_status(self):
            LOG.exception(f"Failed to update worker status: {error}")
            raise

+    def _get_worker_logs(self) -> Optional[WorkerLogConfig]:


nit - what about get_worker_log_config?

Arrrg, I had it as the a non private name before but went for _ so no :) Not changing it

godobyte · 2025-04-04T16:22:30Z

src/deadline_test_fixtures/deadline/worker.py

+if TYPE_CHECKING:
+    from botocore.paginate import PageIterator, Paginator


curious - why only import those during type checking?

This was done in the rest of the library too, there's some differences in versions of python I think.

godobyte · 2025-04-04T16:24:39Z

src/deadline_test_fixtures/deadline/worker.py

+            workerId=self.worker_id,
+        )
+        if log_config := response["log"]:
+            LOG.info(f"LogGroup structure {log_config}")


nit - you meant LogConfig?

godobyte · 2025-04-04T16:25:14Z

src/deadline_test_fixtures/deadline/worker.py

+                    return WorkerLogConfig(
+                        cloudwatch_log_group=log_group_name, cloudwatch_log_stream=log_stream_name
+                    )
+        # Default, no log group yet.


nit - you meant log config? log group should always exist as it's at fleet level.

godobyte · 2025-04-04T16:29:26Z

src/deadline_test_fixtures/deadline/worker.py

+        if not log_config:
+            return WorkerLog(worker_id=self.worker_id, logs=[])  # type: ignore[arg-type]
+
+        filter_log_events_paginator: Paginator = logs_client.get_paginator("filter_log_events")


Please make sure caller has the logs:FilterLogEvents permission to perform this operation. Local test and automated tests use different infrastructure setup.

This is the same in the rest of the library and we don't check

This introduce FilterLogEvents call to fleet logs while the prior code filter on session logs no? The permission for CWL is configured at log group level, that's why I suggested checking.

Took a quick look seems we have have this permission on all deadline log groups log-group:/aws/deadline/* , should be good then.

Signed-off-by: David Leong <[email protected]>

sonarqubecloud · 2025-04-04T18:28:49Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

leongdl force-pushed the workerlog branch 2 times, most recently from ed174a7 to aee1f33 Compare April 3, 2025 21:09

leongdl marked this pull request as ready for review April 3, 2025 21:10

leongdl requested a review from a team as a code owner April 3, 2025 21:10

leongdl force-pushed the workerlog branch from 4d71d60 to aee1f33 Compare April 3, 2025 21:13

ttblanchard reviewed Apr 3, 2025

View reviewed changes

leongdl force-pushed the workerlog branch 2 times, most recently from 6b44091 to 2e47d5b Compare April 3, 2025 22:05

ttblanchard previously approved these changes Apr 4, 2025

View reviewed changes

jairaws reviewed Apr 4, 2025

View reviewed changes

godobyte reviewed Apr 4, 2025

View reviewed changes

leongdl dismissed ttblanchard’s stale review via dd4da06 April 4, 2025 16:56

leongdl force-pushed the workerlog branch from 2e47d5b to dd4da06 Compare April 4, 2025 16:56

godobyte previously approved these changes Apr 4, 2025

View reviewed changes

leongdl dismissed godobyte’s stale review via 1950ee0 April 4, 2025 18:20

leongdl force-pushed the workerlog branch from dd4da06 to 1950ee0 Compare April 4, 2025 18:20

godobyte approved these changes Apr 4, 2025

View reviewed changes

ttblanchard approved these changes Apr 4, 2025

View reviewed changes

leongdl enabled auto-merge (squash) April 4, 2025 18:27

feat: Add support to fetch and match worker logs

899d158

Signed-off-by: David Leong <[email protected]>

leongdl force-pushed the workerlog branch from 1950ee0 to 899d158 Compare April 4, 2025 18:28

leongdl merged commit b920331 into aws-deadline:mainline Apr 4, 2025
16 checks passed

client-software-ci mentioned this pull request Apr 4, 2025

chore(release): 0.17.5 #184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support to fetch and match worker logs #183

feat: Add support to fetch and match worker logs #183

leongdl commented Apr 3, 2025 •

edited

Loading

ttblanchard Apr 3, 2025

leongdl Apr 3, 2025

ttblanchard Apr 3, 2025

leongdl Apr 3, 2025

jairaws Apr 4, 2025

jairaws Apr 4, 2025

leongdl Apr 4, 2025

leongdl Apr 4, 2025

jairaws Apr 4, 2025

jairaws Apr 4, 2025

leongdl Apr 4, 2025

jairaws Apr 4, 2025

leongdl Apr 4, 2025

jairaws Apr 4, 2025

leongdl Apr 4, 2025

jairaws Apr 4, 2025

leongdl Apr 4, 2025

godobyte Apr 4, 2025

leongdl Apr 4, 2025

godobyte Apr 4, 2025

leongdl Apr 4, 2025

godobyte Apr 4, 2025

leongdl Apr 4, 2025

godobyte Apr 4, 2025

leongdl Apr 4, 2025

godobyte Apr 4, 2025

leongdl Apr 4, 2025

godobyte Apr 4, 2025

sonarqubecloud bot commented Apr 4, 2025

		if TYPE_CHECKING:
		from botocore.paginate import PageIterator, Paginator

feat: Add support to fetch and match worker logs #183

feat: Add support to fetch and match worker logs #183

Conversation

leongdl commented Apr 3, 2025 • edited Loading

What was the problem/requirement? (What/Why)

What was the solution? (How)

What is the impact of this change?

How was this change tested?

Was this change documented?

Is this a breaking change?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 4, 2025

Quality Gate passed

leongdl commented Apr 3, 2025 •

edited

Loading