-
Notifications
You must be signed in to change notification settings - Fork 24
feat: Add pilot pilot logging (legacy and DiracX) #260
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
base: master
Are you sure you want to change the base?
feat: Add pilot pilot logging (legacy and DiracX) #260
Conversation
25e0ce8
to
53e8238
Compare
Pilot/dirac-pilot.py
Outdated
wnVO=pilotParams.wnVO, | ||
jwt=pilotParams.jwt | ||
) | ||
log.info(str(pilotParams.jwt)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leak
self._timer.start() | ||
else: | ||
self._timer = None | ||
self.output = StringIO() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was an idea behind the buffer being a StringIO
object. The buffer was first created in the pilot wrapper (https://github.com/DIRACGrid/DIRAC/pull/6208/files#diff-bbbbde6222aeb320aa12b3df745daf44810996d2bf93c760e10625e1f6e4ed59)
and passed to the pilot. A string appender was also added to a (classic) dirac logger). This served 2 purposes:
- accumulate logs before the pilot was started.
- enable to log from Dirac (not pilot) w/o any changes to Dirac code, which from now on would log to
stdout
and a string buffer.
I cannot tell at this level if your code allows remote logging from Dirac (and DiracX) using loggers they have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing from StringIO()
to a list serves one purpose: having a formatted output.
The goal is to avoid a big bulky buffer, by having a structured data that we can easily process in DiracX. See format_to_json
. As DiracX is ment to be strictly type, appending a list with an object instead of text with an object shape will reduce the amount of bug that can happen. If we have to process billions of lines of code in DiracX and DIRAC to transform from an unstructured line to a meaningful data ("when", "why", "how", "where"), it will cost a lost of processing.
But I would need indeed to adapt the wrapper to have my object-shaped log, or just re-format it at the beginning. This is I guess the downside of my code.
I cannot tell at this level if your code allows remote logging from Dirac (and DiracX) using loggers they have.
For DIRAC, the initial purpose discussed with Federico was to rewrite the logging system to support fully and only DiracX. And for DiracX it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so how does DiracX get hold of the buffer ? Dirac got it via a string appender to a global Logger object. Which DiracX PR is it ?
Pilot/pilotCommands.py
Outdated
except Exception as exc: | ||
self.log.error("Remote logger couldn't be finalised %s " % str(exc)) | ||
else: | ||
# No force here because there's no remote logger if we're here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean here. You are clearly within remote logger (if the remote logger is not enabled the wrapper returns at line 80). And the classic logger has no buffer...
if hasattr(self.log, "buffer") and self.log.isPilotLoggerOn: | ||
self.log.buffer.write(outChunk) | ||
self.log.buffer.write(self.log.format_to_json( | ||
"COMMAND", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tricky bit. In Dirac, whatever the chunks contain, they would be sent eventually, with the last line possibly truncated (w/o \n) the remainder of which would eventually come with the new buffer content. Furthermore the lines would contain header information, like a debug level and timestamp. You marked the data with "COMMAND", are you planning to tokenise the message and fill in fields in format_to_json
as intended?
0f746c8
to
3b4878a
Compare
86b73d0
to
db4a1bf
Compare
Splitted and based on #248