feat: enable remote pilot logging system [MISSING AUTH] by martynia · Pull Request #269 · DIRACGrid/diracx

martynia · 2024-07-01T14:05:11Z

Enable remote logging system with open search. This PR only shows a router in its preliminary form.
The router defines following operations:

@router.post("/") - bulk insert log record sent by a pilot. The method returns a pilotID, so it can use it next time to insert messaged w/o consulting PilotAgentsDB to get PilotID from PilotStamp.
@router.get("/logs") - to retrieve logs for a given pilot_id

Policies allow pilots to store VO-aware logs. Normal users can only retrieve log for their own VOs. diracAdmin can retrieve logs for any VO.

martynia · 2024-07-01T14:45:28Z

@chrisburr I think we need a pilot logging router, even in its basic form. One of the tests fails because it is missing.

martynia · 2024-10-25T09:34:33Z

@chrisburr What is a reason of test failures here ? The demo runs fine locally, can be contacted with a client etc. The test fails with dirac login, waiting for diracX to be installed. Could it be that I missed a setting somewhere ? The failure is persistent.

martynia · 2024-11-01T15:54:23Z

@chrisburr @chaen
I managed to reproduce the problem locally. There is the same failure on GitHub and locally. Since the demo runs fine (the DB is there), is it possible that integration tests fail (NotImplementedError: Cannot enable system_name='pilotlogs' as it requires missing_os_dbs={<class 'diracx.db.os.pilot_logs.PilotLogsDB'>} because of charts being no discoverable by the CI workflow ?

This type of error occurs (in the demo) where charts don't list a relevant DB

Both local and CI tests reach the same point - server installation failure.

chrisburr · 2024-11-14T10:17:34Z

https://github.com/DIRACGrid/DIRAC/blob/7fe654eef5f4b0c23a7047e6e046580b1e354e97/tests/CI/docker-compose.yml#L174-L209

martynia · 2024-11-21T11:03:56Z

@chaen I fixed the database problem, now integration tests are passing, however I'm getting gubbins errors in unit tests. Is it an internal gubbins problem ?

diracx-routers/src/diracx/routers/pilot_logging/__init__.py

diracx-routers/pyproject.toml

diracx-db/src/diracx/db/os/utils.py

aldbr · 2024-12-20T15:01:21Z

diracx-routers/src/diracx/routers/pilot_logging/remote_logger.py

+    # here, users with privileged properties will see logs from all VOs. Is it what we want ?
+    search_params = [{"parameter": "PilotID", "operator": "eq", "value": pilot_id}]
+    if _non_privileged(user_info):
+        search_params.append(
+            {"parameter": "VO", "operator": "eq", "value": user_info.vo}
+        )
+    result = await db.search(
+        ["Message"],
+        search_params,
+        [{"parameter": "LineNumber", "direction": "asc"}],
+    )
+    if not result:
+        return [{"Message": f"No logs for pilot ID = {pilot_id}"}]


Similarly, this bloc of code should be a method of diracx-dbs/src/diracx/db/sql/pilots/pilot_agents.py PilotAgentsDB

diracx-routers/src/diracx/routers/pilot_logging/remote_logger.py

aldbr · 2024-12-20T15:15:52Z

diracx-routers/src/diracx/routers/pilot_logging/remote_logger.py

+def _non_privileged(user_info: AuthorizedUserInfo):
+    return (
+        SERVICE_ADMINISTRATOR not in user_info.properties
+        and OPERATOR not in user_info.properties
+    )


I feel like this should be moved in the access_policies.py function? May be by adding an action type? Or by adding some logic like in:

diracx/diracx-routers/src/diracx/routers/jobs/access_policies.py

Lines 79 to 82 in 320c554

job_owners = await job_db.summary(

["Owner", "VO"],

[{"parameter": "JobID", "operator": "in", "values": job_ids}],

)

I had a logical problem what to do here. I would like to search only once (and delete by query for a delete operation w/o prior searching). This is a reason why I removed the DB argument from a policy call. The problem is more visible in the delete case:

I could search in the policy for a given client VO and if pilot IDs are not from the VO throw an exception (not sure what to do if some IDs are and some are not from the VO the client has - for a bulk PilotID search)

then delete by query (which searches again)

Or could I use the AgentsDB to match the VO ? Still an extra DB query, this time and SQL one.
I opted for a restricted silent delete and and a restricted search by adding the VO, but outside the policy.

aldbr · 2024-12-20T15:23:27Z

diracx-routers/src/diracx/routers/pilot_logging/remote_logger.py

+
+@router.delete("/logs")
+async def delete(
+    pilot_id: int,


We also tend to make bulk requests to the db to improve the performances of the system. I would take a list of pilot_ids as inputs instead of a single pilot_id.

Then you can return sucess and failures as it is done for the jobs for instance:

diracx/diracx-routers/src/diracx/routers/jobs/status.py

Lines 35 to 62 in 320c554

@router.delete("/")

async def remove_bulk_jobs(

job_ids: Annotated[list[int], Query()],

config: Config,

job_db: JobDB,

job_logging_db: JobLoggingDB,

sandbox_metadata_db: SandboxMetadataDB,

task_queue_db: TaskQueueDB,

background_task: BackgroundTasks,

check_permissions: CheckWMSPolicyCallable,

):

"""Fully remove a list of jobs from the WMS databases.

WARNING: This endpoint has been implemented for the compatibility with the legacy DIRAC WMS

and the JobCleaningAgent. However, once this agent is ported to diracx, this endpoint should

be removed, and a status change to Deleted (PATCH /jobs/status) should be used instead for any other purpose.

"""

await check_permissions(action=ActionType.MANAGE, job_db=job_db, job_ids=job_ids)

return await remove_jobs(

job_ids,

config,

job_db,

job_logging_db,

sandbox_metadata_db,

task_queue_db,

background_task,

)

aldbr · 2024-12-20T15:23:56Z

diracx-routers/src/diracx/routers/pilot_logging/remote_logger.py

+
+@router.get("/logs")
+async def get_logs(
+    pilot_id: int,


Similarly,

Suggested change

pilot_id: int,

pilot_ids: List[int],

aldbr · 2025-01-10T16:21:19Z

diracx-routers/src/diracx/routers/pilots/logging.py

+        if _non_privileged(user_info):
+            search_params.append(non_privil_params)
+        await db.delete(search_params)
+        message = f"Logs for for pilots with submission data >='{data.min}' successfully deleted"


Suggested change

message = f"Logs for for pilots with submission data >='{data.min}' successfully deleted"

message = f"Logs for pilots with submission data >='{data.min}' successfully deleted"

diracx-routers/src/diracx/routers/pilots/logging.py

diracx-db/src/diracx/db/os/utils.py

fstagni · 2025-01-15T10:13:23Z

diracx-routers/src/diracx/routers/pilots/logging.py

+    logger.warning(f"Retrieving logs for pilot ID '{pilot_id}'")
+    user_info = await check_permissions(action=ActionType.QUERY)
+
+    # here, users with privileged properties will see logs from all VOs. Is it what we want ?


These are hardly secrets

chaen · 2025-01-17T10:35:18Z

Waiting for the pilot authentication to be ready

…e and fix _get_columns

martynia · 2025-05-05T14:14:16Z

move to Pilot logging PR (#511)

martynia mentioned this pull request Jul 18, 2024

feat: add PilotLogsDB to osDbs DIRACGrid/diracx-charts#108

Merged

martynia force-pushed the main branch from 9f225ae to 02c24a9 Compare July 19, 2024 14:37

martynia force-pushed the main branch from e90d0ea to 8155438 Compare August 9, 2024 16:37

martynia force-pushed the main branch from a6f1eff to a83c39d Compare September 19, 2024 16:13

martynia mentioned this pull request Oct 4, 2024

feat: Implement PilotAgents schema #292

Merged

martynia force-pushed the main branch from c0c4a28 to 43bd81e Compare October 16, 2024 13:54

martynia force-pushed the main branch from 43bd81e to ccf286f Compare October 25, 2024 09:05

martynia force-pushed the main branch from b8c3a42 to 85da2cc Compare November 1, 2024 14:37

martynia force-pushed the main branch from 1c22760 to 6217509 Compare November 21, 2024 10:06

martynia mentioned this pull request Nov 21, 2024

Fix URL definition to adapt to new version of pydantic #326

Closed

martynia force-pushed the main branch from 6217509 to 4209217 Compare November 28, 2024 13:26

martynia force-pushed the main branch from bd838a7 to 921adec Compare December 9, 2024 16:35

martynia force-pushed the main branch 2 times, most recently from 3b0ea4c to 6eb1ebd Compare December 19, 2024 13:47

aldbr requested changes Dec 20, 2024

View reviewed changes

martynia force-pushed the main branch from d65c3bd to e9ad537 Compare January 6, 2025 14:57

aldbr reviewed Jan 10, 2025

View reviewed changes

diracx-routers/src/diracx/routers/pilots/logging.py Outdated Show resolved Hide resolved

aldbr reviewed Jan 10, 2025

View reviewed changes

diracx-routers/src/diracx/routers/pilots/logging.py Outdated Show resolved Hide resolved

fstagni reviewed Jan 15, 2025

View reviewed changes

martynia force-pushed the main branch 4 times, most recently from 5a94535 to 00ef8ed Compare January 17, 2025 09:00

chaen changed the title ~~feat: enable remote pilot logging system~~ feat: enable remote pilot logging system [MISSING AUTH] Jan 17, 2025

chaen marked this pull request as draft January 17, 2025 10:35

martynia force-pushed the main branch from 6220b7e to be6c26a Compare February 27, 2025 10:30

martynia force-pushed the main branch 2 times, most recently from f7e99a2 to 6b6e2c7 Compare March 7, 2025 13:27

martynia added 6 commits March 7, 2025 17:41

feat: enable remote pilot logging system

2a08f00

feat: Restructured pilot logger + test send_message and get_logs

c5a95d6

fix: remove unused PilotNotFoundError exception

6bd60a3

fix: rename _get_columns to get_columns

93078cf

feat: refactor pilot logging to include a business logic layer

3064aca

feat: refactor pilot logging to include a business logic layer, rebas…

eac152f

…e and fix _get_columns

martynia force-pushed the main branch from 5b5094b to eac152f Compare March 7, 2025 17:50

martynia added 2 commits March 10, 2025 15:24

feat: move bulk_insert to logging.py

af854af

fix: remove unused code

78e00aa

martynia closed this May 5, 2025

	job_owners = await job_db.summary(
	["Owner", "VO"],
	[{"parameter": "JobID", "operator": "in", "values": job_ids}],
	)

	@router.delete("/")
	async def remove_bulk_jobs(
	job_ids: Annotated[list[int], Query()],
	config: Config,
	job_db: JobDB,
	job_logging_db: JobLoggingDB,
	sandbox_metadata_db: SandboxMetadataDB,
	task_queue_db: TaskQueueDB,
	background_task: BackgroundTasks,
	check_permissions: CheckWMSPolicyCallable,
	):
	"""Fully remove a list of jobs from the WMS databases.

	WARNING: This endpoint has been implemented for the compatibility with the legacy DIRAC WMS
	and the JobCleaningAgent. However, once this agent is ported to diracx, this endpoint should
	be removed, and a status change to Deleted (PATCH /jobs/status) should be used instead for any other purpose.
	"""
	await check_permissions(action=ActionType.MANAGE, job_db=job_db, job_ids=job_ids)

	return await remove_jobs(
	job_ids,
	config,
	job_db,
	job_logging_db,
	sandbox_metadata_db,
	task_queue_db,
	background_task,
	)

	message = f"Logs for for pilots with submission data >='{data.min}' successfully deleted"
	message = f"Logs for pilots with submission data >='{data.min}' successfully deleted"

Conversation

martynia commented Jul 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martynia commented Jul 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martynia commented Oct 25, 2024

Uh oh!

martynia commented Nov 1, 2024

Uh oh!

chrisburr commented Nov 14, 2024

Uh oh!

martynia commented Nov 21, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaen commented Jan 17, 2025

Uh oh!

martynia commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

martynia commented Jul 1, 2024 •

edited

Loading

martynia commented Jul 1, 2024 •

edited

Loading

martynia commented May 5, 2025 •

edited

Loading