[integration] Remote Pilot Logger to Tornado #6208

martynia · 2022-06-24T12:52:08Z

BEGINRELEASENOTES
This is a draft of a pilot remote logging system to a Tornado Web server - Dirac side.

*Workload Management System
CHANGE: PilotWrapper has been modified to enable sending messages to a Tornado server.
NEW: The remote pilot logger uses a string.io buffer which, when full, flushes messages to Tornado. If enabled, remote logging happens in parallell to the existing pilot logging system. The buffer can be used indirectly by calling log.debug or log.info messages or directly by writing to it. The latter method is used within pilot command subprocesses, where records emitted by Dirac (not Pilot) logger are simply fed into the buffer and sent to Tornado. This requires no changes to logging from those subprocesses.
NEW TornadoPilotLoggingHandler loads a plugin to dispatch messages received by the server. As an example, a FileCacheLoginPlugin is implemented. It writes log records to a file, one per pilot. When a last record is written, or a pilot command exits with a code != 0, a log file is "finalised" and collected by an agent (PilotLoggingAgent).
NEW add a possibility to read a pilot stamp from DIRAC_PILOT_STAMP environment variable if supplied by a CE. This is the same stamp (PilotStamp) as stored in PilotAgents table. This allows to link log files to pilots.
CHANGE PilotCStoJSONSynchronizer to dump an entire Operations section of the CS.

ENDRELEASENOTES

It is foreseen that a MQ logging agent is developed which converts log records into JSON and ship them to a MQ Service.
Notes:
Tornado URL should be provided in Dirac CS.
PilotCStoJSONSynchronizer could also be used to store (parts of ?) the URL in pilot.json

fstagni

This is an initial review.

Apart from the documentation, that can come later, it is missing unit and integration tests.

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

src/DIRAC/WorkloadManagementSystem/Agent/SiteDirector.py

src/DIRAC/WorkloadManagementSystem/Service/TornadoPilotLoggingHandler.py

src/DIRAC/WorkloadManagementSystem/Service/BasicPilotLoggingPlugin.py

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

fstagni · 2022-08-05T15:58:15Z

src/DIRAC/WorkloadManagementSystem/Utilities/PilotCStoJSONSynchronizer.py

@@ -244,6 +244,29 @@ def _getPilotOptionsPerSetup(self, setup, pilotDict):
                    return queueOptionRes
                queuesDict[queue] = queueOptionRes["Value"]
            pilotDict["Setups"][setup]["Logging"]["Queues"] = queuesDict
+        elif "loggingRESTService" in pilotDict["Setups"][setup]:
+            self.log.debug(
+                "Getting option of ", "/DIRAC/Setups/%s/%s" % (setup, pilotDict["Setups"][setup]["loggingRESTService"])


Does not look like a suitable location.

I'm wondering if we really need this (a section in pilot.json). It was there in the original code. Now we are sending logs to Tornado, and in the second step are selecting a plugin by name (TornadoPilotLoggingHandler does this) from the CS directly (MQ or File cache) and we don't need this section. I would revert to the original code here.

We might need it for logging before DIRAC is installed.

src/DIRAC/WorkloadManagementSystem/Utilities/PilotCStoJSONSynchronizer.py

andresailer · 2022-08-18T09:12:38Z

Getting a proxy on demand and executing a function or code block:
https://dirac.readthedocs.io/en/latest/CodeDocumentation/Core/Utilities/Proxy.html

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

fstagni · 2022-10-19T08:29:42Z

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

+        self.setup = gConfig.getValue("/DIRAC/Setup", None)
+        if self.setup is None:
+            self.log.error("Setup is not defined in the configuration")
+            return S_ERROR("Setup is not defined in the configuration")


I don't think this test should be here.

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

fstagni · 2022-10-19T08:39:13Z

src/DIRAC/WorkloadManagementSystem/Agent/SiteDirector.py

-            pilotOptions.append("-z ")
+        # Pilot Logging defined? This enables the extended (possibly remote) logger.
+        # We need the URL and an optional flag which allows fine tuning on VO by VO basis:
+        pilotLogging = opsHelper.getValue("/Pilot/RemoteLogging", True)


Again I would start with

Suggested change

pilotLogging = opsHelper.getValue("/Pilot/RemoteLogging", True)

pilotLogging = opsHelper.getValue("/Pilot/RemoteLogging", False)

src/DIRAC/WorkloadManagementSystem/Agent/SiteDirector.py

src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_PilotLoggingAgent.py

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

src/DIRAC/WorkloadManagementSystem/Client/TornadoPilotLoggingClient.py

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

src/DIRAC/WorkloadManagementSystem/Client/TornadoPilotLoggingClient.py

src/DIRAC/WorkloadManagementSystem/ConfigTemplate.cfg

src/DIRAC/WorkloadManagementSystem/Service/BasicPilotLoggingPlugin.py

src/DIRAC/WorkloadManagementSystem/Service/FileCacheLoggingPlugin.py

src/DIRAC/WorkloadManagementSystem/Service/TornadoPilotLoggingHandler.py

fstagni · 2022-11-04T14:36:12Z

For the failing PilotWrapper tests: you need to change this line https://github.com/DIRACGrid/DIRAC/blob/integration/tests/Integration/WorkloadManagementSystem/Test_GenerateAndExecutePilotWrapper.py#L56 from

    location="diracproject.web.cern.ch/diracproject/tars/Pilot/DIRAC/master/,wrong.cern.ch",

to

    location="diracproject.web.cern.ch/diracproject/tars/Pilot/DIRAC/devel/,wrong.cern.ch",

and then we would need to merge your Pilot PR (to devel branch of course). This might break this test and the DIRAC certification setup but that's not a big deal.

martynia · 2022-11-04T16:12:56Z

Well, it takes pilot code from tars not from git. How would I make refresh the tars ? It's still the old code.

fstagni · 2022-11-07T06:55:15Z

The tars are updated every night by this action: https://github.com/DIRACGrid/Pilot/actions/workflows/nightly.yml

martynia · 2022-11-18T10:21:16Z

There is something strange happening here. When examining the pilot.tar I can clearly see the new code. This tar does not have (among other things) MessageSender.py, since this file is now obsolete. By when un-taring the bundle the file is listed. Is it possible that we are still unpacking the old file ?? but are claiming otherwise ...

fstagni · 2022-11-18T10:39:37Z

There is something strange happening here. When examining the pilot.tar I can clearly see the new code. This tar does not have (among other things) MessageSender.py, since this file is now obsolete. By when un-taring the bundle the file is listed. Is it possible that we are still unpacking the old file ?? but are claiming otherwise ...

I don't see anything wrong ...

martynia · 2022-11-18T10:45:21Z

Well, so why in the (failing) wrapper test the untar lists more files than there actually are in the tar which is listed in that test and is here: https://diracproject.web.cern.ch/diracproject/tars/Pilot/DIRAC/devel/ ?

fstagni · 2023-06-26T15:02:21Z

There are still test failures after rebasing. Not sure they are related to my changes, though.

Run these locally and you should them failing:

src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py::test_InputData PASSED [ 90%]
src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py::test_performChecks PASSED [ 90%]
src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py::test_execute[/bin/ls-None-None-Application Finished Successfully] PASSED [ 90%]
src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py::test_execute[script-OK.sh-None-src/DIRAC/WorkloadManagementSystem/JobWrapper/test/-Application Finished Successfully] FAILED [ 90%]
src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py::test_execute[script.sh-111-src/DIRAC/WorkloadManagementSystem/JobWrapper/test/-Application Finished With Errors0] FAILED [ 90%]
src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py::test_execute[script.sh-111-src/DIRAC/WorkloadManagementSystem/JobWrapper/test/-Application Finished With Errors1] FAILED [ 90%]
src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py::test_execute[script-RESC.sh-None-src/DIRAC/WorkloadManagementSystem/JobWrapper/test/-Going to reschedule job] FAILED [ 90%]
src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py::test_execute[src/DIRAC/WorkloadManagementSystem/scripts/dirac_jobexec.py-src/DIRAC/WorkloadManagementSystem/JobWrapper/test/jobDescription.xml -o /DIRAC/Setup=Test-None-Application Finished Successfully] FAILED [ 90%]

martynia · 2023-06-27T16:39:29Z

Hi,
The errors I can see when run locally in a Docker container, following the manual (https://dirac.readthedocs.io/en/latest/DeveloperGuide/CodeTesting/index.html) are the same as on GitHub. The test cannot locate a file (multiple files in fact) under src/DIRAC/WorkloadManagementSystem/JobWrapper/test/. The files are there. The test itself hasn't changed for a couple of months and it is not my code. There must be a fundamental issue with paths setting, not the code itself. Even the test in 8.0 looks the same.

martynia · 2023-06-28T05:31:38Z

The cwd the test is running in is /home/dirac. I would like to know if it is intended in this test, before I prepend src/ with DIRAC/.

martynia · 2023-06-28T14:58:54Z

@aldbr is this your code ? Can you help me with failing tests ?

fstagni · 2023-06-28T16:04:21Z

I checked out your branch, and, if I run only this (py)test:

pytest src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py

it works fine.

If instead I run pytest src/DIRAC/WorkloadManagementSystem/ (so all the tests found in this package) it fails with

FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpw43m4wms/src/DIRAC/Core/Base/API.py

This is because in one of the unit tests you have added in this PR you are moving to a temporary directory. Please fix it one way or the other.

martynia · 2023-06-29T09:04:17Z

I fixed the code avoiding using os.chdir(). Now pylint is complaining in AuthHandler:
https://github.com/martynia/DIRAC/blob/87acd4748909128d96c527f56f1e4a61395281b7/src/DIRAC/FrameworkSystem/API/AuthHandler.py#L247

fstagni · 2023-06-29T11:01:28Z

I fixed the code avoiding using os.chdir(). Now pylint is complaining in AuthHandler: https://github.com/martynia/DIRAC/blob/87acd4748909128d96c527f56f1e4a61395281b7/src/DIRAC/FrameworkSystem/API/AuthHandler.py#L247

Yes that's independent from you!

marianne013 · 2023-07-19T11:29:20Z

@fstagni @atsareg @cburr Are any of you able to re-trigger the CI test on this request, please?

andresailer · 2023-07-19T11:39:24Z

Sure

andresailer · 2023-07-19T11:40:17Z

PS: you mean all or just the failed ones?

marianne013 · 2023-07-19T11:42:03Z

PS: you mean all or just the failed ones?

Didn't even know you could do that :-D

andresailer · 2023-07-19T11:51:51Z

You mean that I could do that, or that one could to that?
And I still don't know if all or just the failed ones. But the failed ones failed again.

marianne013 · 2023-07-19T12:07:53Z

Sowohl als auch ....
Just realized that @martynia hasn't rebased, so the test indeed fail again. Sorry about that.

martynia · 2023-07-19T14:30:42Z

I have rebased and tests are still failing, in another place, though.

marianne013 · 2023-07-19T16:18:37Z

I have rebased and tests are still failing, in another place, though.

Yes, this looks like a failure elsewhere in the DIRAC code. I can't tell though whose it is, though I feel an urge to tag arbitrary Chrises....

marianne013 · 2023-07-20T11:43:23Z

@chrisburr I think the test failure is due to an upgrade to python 3.11:
https://docs.python.org/3/library/importlib.resources.html
read_text is now deprecated, so maybe that's the issue ?
Tagging the other Chris @chaen as well, as I don't have everyone's holiday schedules in my head.

#7112 is me trying to fix this.

cburr · 2023-07-20T13:05:52Z

Hi, you all have the wrong Chris Burr, this is the guy you want: @chrisburr

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

fstagni · 2023-07-26T11:09:23Z

OK. Now just add a note in https://github.com/DIRACGrid/DIRAC/wiki/DIRAC-8.1 on what is needed to activate this and we are ready to merge this PR, and to test it in the DIRAC certification setup.

martynia · 2023-07-26T13:10:13Z

I have added a preliminary configuration info to the Wiki.

martynia changed the title ~~[integration] Remote Pilot Logger to Tonado~~ [integration] Remote Pilot Logger to Tornado Jun 24, 2022

martynia mentioned this pull request Jun 27, 2022

[integration] Remote Pilot Logger #5961

Closed

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch from 405b86a to 5a6f587 Compare June 27, 2022 08:32

fstagni reviewed Jun 28, 2022

View reviewed changes

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch 2 times, most recently from 571c1c9 to 50ce8c9 Compare July 11, 2022 13:06

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch from 50ce8c9 to 94a4a63 Compare August 5, 2022 10:07

fstagni reviewed Aug 5, 2022

View reviewed changes

fstagni changed the base branch from integration to rel-v8r0 September 15, 2022 07:51

fstagni changed the title ~~[integration] Remote Pilot Logger to Tornado~~ [8.0] Remote Pilot Logger to Tornado Sep 15, 2022

DIRACGridBot added the alsoTargeting:integration Cherry pick this PR to integration after merge label Sep 21, 2022

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch from 28ea921 to f203f0e Compare October 18, 2022 15:06

fstagni reviewed Oct 19, 2022

View reviewed changes

This was referenced Oct 28, 2022

Pilot.json : add a new format #6470

Closed

(for PilotLogging) find the VO from the proxy, and from the token DIRACGrid/Pilot#165

Closed

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch from f203f0e to 010991b Compare October 31, 2022 10:30

martynia changed the base branch from rel-v8r0 to integration October 31, 2022 10:38

martynia changed the title ~~[8.0] Remote Pilot Logger to Tornado~~ [integration] Remote Pilot Logger to Tornado Oct 31, 2022

fstagni reviewed Oct 31, 2022

View reviewed changes

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch from 0a7b90f to 5eb5379 Compare November 3, 2022 08:35

fstagni mentioned this pull request Nov 3, 2022

(for PilotLogging) find the VO from the proxy (and from the token) #6491

Closed

fstagni mentioned this pull request Nov 4, 2022

feat: Enable remote pilot logging using HTTP interface to Tornado. DIRACGrid/Pilot#158

Merged

fstagni mentioned this pull request Nov 15, 2022

[8.1] remove old PilotsLogging machinery #6556

Merged

martynia mentioned this pull request Jul 10, 2023

[Integration] Modify dirac-admin-get-pilot-output to get remote pilot log #7105

Merged

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch from 87acd47 to 8cb436c Compare July 19, 2023 14:10

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch from 8cb436c to 5a2af95 Compare July 21, 2023 09:41

feat: Enabling remote pilot logging to Tornado.

9d437d8

martynia force-pushed the integration_janusz_pilotlogsWrapper_dev branch from 5a2af95 to 9d437d8 Compare July 25, 2023 09:38

fstagni reviewed Jul 25, 2023

View reviewed changes

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py Show resolved Hide resolved

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py Outdated Show resolved Hide resolved

martynia added 3 commits July 25, 2023 15:34

fix: remove refs to setup

ba5da8e

fix: initialize pilot log lifetime in the constructor

c7b7e2a

fix: remove refs to setup in a test

9bce963

fstagni approved these changes Jul 26, 2023

View reviewed changes

fstagni merged commit a18d85f into DIRACGrid:integration Jul 26, 2023
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[integration] Remote Pilot Logger to Tornado #6208

[integration] Remote Pilot Logger to Tornado #6208

martynia commented Jun 24, 2022 •

edited by fstagni

Loading

fstagni left a comment

fstagni Aug 5, 2022

martynia Aug 9, 2022 •

edited

Loading

fstagni Aug 18, 2022 •

edited

Loading

andresailer commented Aug 18, 2022

fstagni Oct 19, 2022

fstagni Oct 19, 2022

fstagni commented Nov 4, 2022

martynia commented Nov 4, 2022 •

edited

Loading

fstagni commented Nov 7, 2022

martynia commented Nov 18, 2022

fstagni commented Nov 18, 2022

martynia commented Nov 18, 2022

fstagni commented Jun 26, 2023

martynia commented Jun 27, 2023

martynia commented Jun 28, 2023

martynia commented Jun 28, 2023

fstagni commented Jun 28, 2023

martynia commented Jun 29, 2023

fstagni commented Jun 29, 2023

marianne013 commented Jul 19, 2023

andresailer commented Jul 19, 2023

andresailer commented Jul 19, 2023

marianne013 commented Jul 19, 2023

andresailer commented Jul 19, 2023

marianne013 commented Jul 19, 2023

martynia commented Jul 19, 2023

marianne013 commented Jul 19, 2023

marianne013 commented Jul 20, 2023 •

edited

Loading

cburr commented Jul 20, 2023

fstagni commented Jul 26, 2023

martynia commented Jul 26, 2023

	pilotLogging = opsHelper.getValue("/Pilot/RemoteLogging", True)
	pilotLogging = opsHelper.getValue("/Pilot/RemoteLogging", False)

[integration] Remote Pilot Logger to Tornado #6208

[integration] Remote Pilot Logger to Tornado #6208

Conversation

martynia commented Jun 24, 2022 • edited by fstagni Loading

fstagni left a comment

Choose a reason for hiding this comment

fstagni Aug 5, 2022

Choose a reason for hiding this comment

martynia Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

fstagni Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

andresailer commented Aug 18, 2022

fstagni Oct 19, 2022

Choose a reason for hiding this comment

fstagni Oct 19, 2022

Choose a reason for hiding this comment

fstagni commented Nov 4, 2022

martynia commented Nov 4, 2022 • edited Loading

fstagni commented Nov 7, 2022

martynia commented Nov 18, 2022

fstagni commented Nov 18, 2022

martynia commented Nov 18, 2022

fstagni commented Jun 26, 2023

martynia commented Jun 27, 2023

martynia commented Jun 28, 2023

martynia commented Jun 28, 2023

fstagni commented Jun 28, 2023

martynia commented Jun 29, 2023

fstagni commented Jun 29, 2023

marianne013 commented Jul 19, 2023

andresailer commented Jul 19, 2023

andresailer commented Jul 19, 2023

marianne013 commented Jul 19, 2023

andresailer commented Jul 19, 2023

marianne013 commented Jul 19, 2023

martynia commented Jul 19, 2023

marianne013 commented Jul 19, 2023

marianne013 commented Jul 20, 2023 • edited Loading

cburr commented Jul 20, 2023

fstagni commented Jul 26, 2023

martynia commented Jul 26, 2023

martynia commented Jun 24, 2022 •

edited by fstagni

Loading

martynia Aug 9, 2022 •

edited

Loading

fstagni Aug 18, 2022 •

edited

Loading

martynia commented Nov 4, 2022 •

edited

Loading

marianne013 commented Jul 20, 2023 •

edited

Loading