Skip to content

Enhancements : log file for pack actions #2373

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.

Sign up for GitHub

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

Closed
wants to merge 10 commits into from
Closed

Enhancements : log file for pack actions #2373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e9c6268
PXPSE-422-handled to log actions as a seperate log file for each packs
Dec 30, 2015
d12faf8
PXPSE-422 : updated the logging.conf
Dec 30, 2015
a27b7d2
removing the import logging statement
azamsheriff Jan 6, 2016
300b868
PXPSE-422: updated the pythonrunner to get logging from /etc/st2actio…
azamsheriff Jan 12, 2016
7adcbd3
PXPSE-422:updated pythonrunner.py
azamsheriff Jan 12, 2016
869f1c3
PXPSE-422:updated pythonRunner and handled indentation
azamsheriff Jan 12, 2016
d85ba3c
story/MAS/PXPSE-422-create-log-file-for-pack-actio
azamsheriff Jan 13, 2016
93c14e4
PXPSE-422:Handled code to run take logging.conf from st2 and connect
azamsheriff Jan 13, 2016
e601993
PXPSE-422
azamsheriff Jan 13, 2016
5eb1d82
Merge branch 'master' of https://github.com/Plexxi/st2 into story/MAS…
andrew-regan Feb 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions st2actions/conf/logging.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ args=(sys.stdout,)
class=st2common.log.FormatNamedFileHandler
level=INFO
formatter=verboseConsoleFormatter
args=('logs/st2actionrunner.{pid}.log',)
args=('logs/st2actionrunner.{pack}.log',)
Copy link
Contributor

@manasdk manasdk Feb 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


[handler_auditHandler]
class=st2common.log.FormatNamedFileHandler
level=AUDIT
formatter=jsonFormatter
args=('logs/st2actionrunner.{pid}.audit.log',)
args=('logs/st2actionrunner.{pack}.audit.log',)

[formatter_simpleConsoleFormatter]
class=st2common.logging.formatters.ConsoleLogFormatter
Expand All @@ -36,7 +36,7 @@ datefmt=

[formatter_verboseConsoleFormatter]
class=st2common.logging.formatters.ConsoleLogFormatter
format=%(asctime)s %(thread)s %(levelname)s %(module)s [-] %(message)s
format=%(asctime)s %(process)d %(thread)s %(levelname)s %(module)s [-] %(message)s
Copy link
Contributor

@manasdk manasdk Feb 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok with including process in the line number. If we maintain per pid logging will end up being redundant.

datefmt=

[formatter_gelfFormatter]
Expand All @@ -45,4 +45,4 @@ format=%(message)s

[formatter_jsonFormatter]
class=pythonjsonlogger.jsonlogger.JsonFormatter
format=%(asctime) %(thread) %(levelname) %(module) %(message)
format=%(asctime) %(process)d %(thread) %(levelname) %(module) %(message)
16 changes: 16 additions & 0 deletions st2actions/st2actions/runners/pythonrunner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import abc
import json
import uuid
import logging.config

import six
from eventlet.green import subprocess
Expand Down Expand Up @@ -77,6 +78,21 @@ def __init__(self, config=None):
def run(self, **kwargs):
pass

def _set_up_logger(self):
Copy link
Contributor

@manasdk manasdk Feb 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is not being called anywhere. Are we missing some integration code?

Also, similar impl exists in pythonactionwrapper now so not a good idea to override here anyway. See https://github.com/StackStorm/st2/blob/master/st2actions/st2actions/runners/python_action_wrapper.py#L110

"""
Set up a logger which logs all the messages with level INFO
and above to stderr.
"""
if '/opt/' in BASE_DIR:
from st2actions import config as action_config
config_path = action_config.get_logging_config_path()
else:
config_path = os.path.join(BASE_DIR, '../../conf/logging.conf')
logging.config.fileConfig(config_path, disable_existing_loggers=False)
logger_name = 'actions.python.%s' % (self.__class__.__name__)
logger = logging.getLogger(logger_name)
return logger


class PythonRunner(ActionRunner):

Expand Down
4 changes: 3 additions & 1 deletion st2common/st2common/logging/handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import socket
import time
import logging
import sys

from oslo_config import cfg

Expand All @@ -37,7 +38,8 @@ def __init__(self, filename, mode='a', maxBytes=0, backupCount=0, encoding=None,
format_values = {
'timestamp': timestamp,
'ts': isotime_str,
'pid': pid
'pid': pid,
'pack': os.path.abspath(os.path.dirname(sys.argv[2])).split('/')[-2]
Copy link
Member

@Kami Kami Jan 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@azamsheriff azamsheriff Jan 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@Kami Kami Jan 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@azamsheriff azamsheriff Jan 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Kami.

}
filename = filename.format(**format_values)
super(FormatNamedFileHandler, self).__init__(filename, mode=mode, maxBytes=maxBytes,
Expand Down