Logging implementation for tendrl #178

GowthamShanmugam · 2017-01-24T15:32:16Z

tendrl-bug-id: Tendrl/specifications#28
tendrl-spec: logging_implementation

Signed-off-by: GowthamShanmugam gshanmug@redhat.com

GowthamShanmugam · 2017-01-24T15:35:03Z

tendrl/node_agent/message/logger.py

+            self.message.payload["job_id"],
+            self.message.__dict__,
+            append=True)
+        log_message = ("%s:%s") % (


@shtripat @brainfunked @r0h4n @nnDarshan @anmolbabu @nthomas-redhat
operation updates are required some sequence key, so we cant use etcdobj save method, and message is an object but is in common so direct write to etcd is used.

GowthamShanmugam · 2017-01-24T15:36:06Z

@shtripat @brainfunked @r0h4n @nnDarshan @anmolbabu @nthomas-redhat please review

GowthamShanmugam · 2017-01-25T06:54:46Z

etc/tendrl/node-agent/logging.yaml.syslog.sample

-formatters:
-    simple:
-        format: "%(asctime)s - %(filename)s:%(lineno)s - %(funcName)20s() - %(levelname)s - %(message)s"
-        datefmt: "%Y-%m-%dT%H:%M:%S%z"



i have removed format because we cant tell function name and line no and file name from logger , it is passed with message and appended with log message automatically.

how is it passed with the message? any examples?

@r0h4n
log_message = Message.to_json(log_message)
message = ("%s - %s - %s:%s - %s - %s - %s") % (
self.message.timestamp,
self.message.publisher,
self.message.caller["filename"],
self.message.caller["line_no"],
self.message.caller["function"],
self.message.priority.upper(),
log_message
)

brainfunked

@r0h4n need you to review.

brainfunked · 2017-01-25T08:01:02Z

doc/source/installation.rst

@@ -40,7 +40,8 @@ For installing node agent following dependencies have to be manually installed:

    $ yum install libffi-devel gcc python-devel openssl-devel
    $ yum install -y http://li.nux.ro/download/nux/dextop/el7/x86_64/nux-dextop-release-0-1.el7.nux.noarch.rpm
-    $ yum install hwinfo 
+    $ yum install hwinfo


Why is hwinfo required for logging implementation?

@brainfunked i am not added hwinfo it comes because of some space change i think

brainfunked · 2017-01-25T08:03:05Z

etc/tendrl/node-agent/node-agent.conf.yaml.sample

+log_level: DEBUG
+tendrl_message_socket_addr: 127.0.0.1


Needs to be a UNIX socket, as commented here: https://github.com/Tendrl/node-agent/pull/67/files#r97720923

brainfunked · 2017-01-25T08:03:33Z

etc/tendrl/node-agent/logging.yaml.syslog.sample

        stream: ext://sys.stdout

    info_file_handler:
        class: logging.handlers.SysLogHandler
+        facility: local0


Log to local5 or some such. Lots of programmes use local0, local1 etc.

brainfunked · 2017-01-25T08:04:23Z

tendrl-node-agent.spec

@@ -24,6 +24,7 @@ Requires: collectd
 Requires: python-jinja2
 Requires: tendrl-commons
 Requires: hwinfo 
+Requires: rsyslog


Not required.

brainfunked · 2017-01-25T08:08:37Z

tendrl/node_agent/message/handler.py

+RECEIVE_DATA_SIZE = 4096
+
+
+class MessageHandler(gevent.greenlet.Greenlet):


Needs to be updated based on Tendrl/commons#134 (comment)

GowthamShanmugam · 2017-01-27T12:06:46Z

@brainfunked @shtripat @nthomas-redhat @anmolbabu @r0h4n @anmolsachan please review this, i have done all the changes

anmolbabu · 2017-01-31T08:56:29Z

tendrl/node_agent/message/logger.py

+class Logger(object):
+    def __init__(self, message):
+        self.message = message
+        self.etcd_orm = tendrl_ns.etcd_orm


Why do you want a copy use the global variable directly tendrl_ns.etcd_orm.....

@anmolbabu i will correct it, no need to take copy

anmolbabu · 2017-01-31T09:04:53Z

tendrl/node_agent/message/logger.py

+    def critical(self, message):
+        LOG.critical(message)
+
+    def notice(self, message):


I see from line 72, that this func name should match priority I think case wise and also exact name should be the func here. In which case is it NOTIF or notice? Applies to all other funcs above as well. Also if this all that needs to be done is there a better way of doing it? Probably have a map from log levels of tendrl to log levels of logger and use it in getattr directly on LOG object instead of on self? Just a thought.

i will modify those line to call the function easily

anmolbabu · 2017-01-31T09:11:42Z

tendrl/node_agent/message/handler.py

+    def bind_unix_listener(self):
+        try:
+            if os.path.exists(PATH):
+                os.remove(PATH)


do you really need to do this?
I feel just if the socket file is not used currently by some other process it suffices... Please check...
I am not sure though...

@anmolbabu i will check it

@anmolbabu If we are not removed this file then i will throw exception like "Address already in use". They have to remove manually to run then node agent again.

r0h4n · 2017-02-01T16:44:18Z

etc/tendrl/node-agent/logging.yaml.syslog.sample

-formatters:
-    simple:
-        format: "%(asctime)s - %(filename)s:%(lineno)s - %(funcName)20s() - %(levelname)s - %(message)s"
-        datefmt: "%Y-%m-%dT%H:%M:%S%z"



how is it passed with the message? any examples?

r0h4n · 2017-02-01T16:53:21Z

tendrl/node_agent/message/handler.py

+
+
+RECEIVE_DATA_SIZE = 4096
+PATH = "/etc/tendrl/.node_agent_socket"


this needs to come from configuration

@r0h4n i will change it

r0h4n · 2017-02-01T16:58:30Z

tendrl/node_agent/message/handler.py

@@ -0,0 +1,53 @@
+import _socket


use http://www.gevent.org/gevent.socket.html instead of _socket

r0h4n · 2017-02-01T16:58:42Z

tendrl/node_agent/message/handler.py

+        try:
+            if os.path.exists(PATH):
+                os.remove(PATH)
+            self.sock = _socket.socket(_socket.AF_UNIX, _socket.SOCK_STREAM)


http://www.gevent.org/gevent.socket.html

GowthamShanmugam · 2017-02-02T06:01:23Z

@r0h4n message contains one field called caller, it contains all the information about where the message is from and which function and line no. these details are captured using getframeinfo. so there information are appended with message and then logged. so no need log format here. because log format will take the details where the message is logged. Here we are logging the all message in node_agent , so it give wrong function name and line no and file name.

GowthamShanmugam · 2017-02-02T06:03:00Z

@r0h4n example log_message = Message.to_json(log_message)
message = ("%s - %s - %s:%s - %s - %s - %s") % (
self.message.timestamp,
self.message.publisher,
self.message.caller["filename"],
self.message.caller["line_no"],
self.message.caller["function"],
self.message.priority.upper(),
log_message
)

GowthamShanmugam · 2017-02-02T13:34:10Z

@r0h4n all changes are done

r0h4n · 2017-02-02T13:45:37Z

etc/tendrl/node-agent/node-agent.conf.yaml.sample

@@ -5,3 +5,4 @@ tendrl_ansible_exec_file: $HOME/.tendrl/node-agent/ansible_exec
 log_cfg_path: /etc/tendrl/node-agent/node-agent_logging.yaml
 log_level: DEBUG
 package_source_type: rpm
+logging_socket_path: "/etc/tendrl/.node_agent_socket"


this default path is not supposed to be in /etc, application sockets can be store in /tmp/node-agent/.node_agent.sock

r0h4n · 2017-02-02T13:49:43Z

tendrl/node_agent/objects/definition/master.py

+          help: "Called details"
+          type: Dict
+      enabled: true
+      list: cluster/$TendrlContext.integration_id/messages


clusters not cluster

r0h4n · 2017-02-02T13:49:56Z

tendrl/node_agent/objects/definition/master.py

+      enabled: true
+      list: cluster/$TendrlContext.integration_id/messages
+      help: "Cluster Messages"
+      value: cluster/$TendrlContext.integration_id/messages


typo: clusters

r0h4n · 2017-02-02T13:50:23Z

tendrl/node_agent/objects/definition/master.py

+          help: "Called details"
+          type: Dict
+      enabled: true
+      list: node/$Node_context.node_id/messages


camel case "Messages"

r0h4n · 2017-02-02T13:50:34Z

tendrl/node_agent/objects/definition/master.py

+          help: "Called details"
+          type: Dict
+      enabled: true
+      list: cluster/$TendrlContext.integration_id/messages


camelcase "Messages"

r0h4n · 2017-02-02T13:51:12Z

tendrl/node_agent/objects/definition/master.py

+          help: "Called details"
+          type: Dict
+      enabled: true
+      list: /messages


GowthamShanmugam · 2017-02-02T14:08:17Z

@r0h4n all changes done

tendrl-spec: logging_implementation Signed-off-by: GowthamShanmugam <gshanmug@redhat.com>

GowthamShanmugam · 2017-02-08T06:52:18Z

@r0h4n changes are done

GowthamShanmugam commented Jan 24, 2017

View reviewed changes

GowthamShanmugam commented Jan 25, 2017

View reviewed changes

brainfunked suggested changes Jan 25, 2017

View reviewed changes

anmolbabu reviewed Jan 31, 2017

View reviewed changes

r0h4n suggested changes Feb 1, 2017

View reviewed changes

r0h4n suggested changes Feb 2, 2017

View reviewed changes

Logging implementation for tendrl

fc8c924

tendrl-spec: logging_implementation Signed-off-by: GowthamShanmugam <gshanmug@redhat.com>

GowthamShanmugam closed this Feb 8, 2017

GowthamShanmugam deleted the logging branch February 8, 2017 07:52

		RECEIVE_DATA_SIZE = 4096


		class MessageHandler(gevent.greenlet.Greenlet):



		RECEIVE_DATA_SIZE = 4096
		PATH = "/etc/tendrl/.node_agent_socket"

Logging implementation for tendrl #178

Logging implementation for tendrl #178

Conversation

GowthamShanmugam commented Jan 24, 2017 • edited Loading

Choose a reason for hiding this comment

GowthamShanmugam commented Jan 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brainfunked left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GowthamShanmugam commented Jan 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GowthamShanmugam commented Feb 2, 2017

GowthamShanmugam commented Feb 2, 2017

GowthamShanmugam commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GowthamShanmugam commented Feb 2, 2017

GowthamShanmugam commented Feb 8, 2017

GowthamShanmugam commented Jan 24, 2017 •

edited

Loading