Enhancements : log file for pack actions

[valarmathip]

Handled pack actions to log to a file in addition to the current logging to stdout. as per the story PXPSE - 422.

[Kami]

Thanks for the contribution.

Can you please provide more details (feel free to update the PR description) on what this PR is supposed to do and solve?

Thanks

[Kami]

I'm not sure I fully understand how this is supposed to work.

It seems like we parse the pack name from the command line arguments? If so, this doesn't seem robust (not to mention, that it relies on an unstable command line arguments order, etc.).

Is there a safer and more robust way to achieve this?

[azamsheriff]

Yes Kami your understanding is correct. we tried to get the pack name from the command line arguments. We tried with different os.path calls but we are not able to get the pack name. Can you please help us on getting the pack name in more safer and robust way

[Kami]

I will look into it, but it will probably require some refactoring.

[azamsheriff]

Thanks Kami.

[andrew-regan]

@Kami
"Can you please provide more details (feel free to update the PR description) on what this PR is supposed to do and solve?"

Today python action logging is collected up into a single line of output in st2actionrunner.[PID].log with embedded '\n' characters instead of newlines. This not very readable when trying to look through the logging of a single action execution. The intent of this PR is to break the action logging out into their own log files per pack with each log message on its own line instead of embedded '\n''s.

This was briefly discussed in stackstorm_community on Slack between ryanms-plexxi, jfryman, and doriftoshoes for reference. :-)

[lakshmi-kannan]

Though I understand the high level need, I am with kami on solution being hacky and not clean. I don't have an alternate proposal but we'll discuss internally first.

[andrew-regan]

Thanks! Let us know.

[andrew-regan]

@lakshmi-kannan @Kami any update on an alternative for retrieving the pack name? One thought is to leverage a similar approach to the one in PR #2396. PythonActionWrapper knows the pack and could set action_instance.pack the same way it is currently setting action_instance.datastore. Thoughts?

[Kami]

@andrew-regan Sorry, no update yet, been busy with 1.3 release and related stuff lately.

[Kami]

@andrew-regan I had a look at the code again and yes, you could handle that in the python action wrapper since the action wrapper is already aware of the pack name (similar to the sensor wrapper).

[andrew-regan]

We'll take a pass at implementing that and see if it looks any better.

[andrew-regan]

@lakshmi-kannan @Kami @manasdk

Any thoughts on the approach in this commit? 8ced5e1

I think this is better aligned with the logging in other st2 components, but doesn't loose the PID information. The PID is part of the log message instead of being in the name of the file.

Thanks!

[manasdk]

ActionRunner itself logs when not in context of running an action in a pack. Per pack logging make very little sense in that case.

[manasdk]

-1

Logs should be independent of pack running the action. I honestly believe PID is the right way to deal with log. We always control the no of action runners but packs are unbounded.

Besides, in production actionrunner logs should be sent to a rsyslog or a similar util. This way all logs can end up in a single file and be easy to search etc. Also, the idea that the process can depending on context end up writing to different log files rubs me the wrong way.

I see we are solving 2 separate issues -

1. break the action logging out into their own log files per pack
2. each log message on its own line instead of embedded '\n''s

-1 to #1.

+1 to #2

Not sure I see how #2 is solved by the PR yet - maybe some code is missing?

[manasdk]

Some offline discussions led us to come to a conclusion that this approach is not going to necessarily get us to the desired solution.

@andrew-regan is investigating alternate way to pretty print logs from action output so that the NLs are properly preserved.