Piotr Kliczewski has uploaded a new change for review.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
stomp: reducing log footprint from stomp lib
There was bunch of not needed information about being not able to process disconnect frame (part of the spec and expected) as well as a lot of entries about messages being sent.
Bug-Url: https://bugzilla.redhat.com/1239062 Change-Id: I98f0c443fd50ec9e05dcf71ee1c7baabd044f48d Signed-off-by: pkliczewski piotr.kliczewski@gmail.com --- M lib/yajsonrpc/stomp.py M lib/yajsonrpc/stompreactor.py 2 files changed, 29 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/46931/1
diff --git a/lib/yajsonrpc/stomp.py b/lib/yajsonrpc/stomp.py index 7436984..633f307 100644 --- a/lib/yajsonrpc/stomp.py +++ b/lib/yajsonrpc/stomp.py @@ -55,12 +55,15 @@ CONNECTED = "CONNECTED" ERROR = "ERROR" RECEIPT = "RECEIPT" + DISCONNECT = "DISCONNECT"
class Headers(object): CONTENT_LENGTH = "content-length" CONTENT_TYPE = "content-type" SUBSCRIPTION = "subscription" + RECEIPT = "receipt" + RECEIPT_ID = "receipt-id" DESTINATION = "destination" ACCEPT_VERSION = "accept-version" REPLY_TO = "reply-to" @@ -430,6 +433,7 @@ Command.MESSAGE: self._process_message, Command.RECEIPT: self._process_receipt, Command.ERROR: self._process_error, + Command.DISCONNECT: self._process_disconnect }
@property @@ -488,7 +492,7 @@ sub.handle_message(frame)
def _process_receipt(self, frame, dispatcher): - self.log.warning("Receipt frame received and ignored") + self.log.debug("Receipt frame received")
def _process_error(self, frame, dispatcher): raise StompError(frame) @@ -528,6 +532,16 @@ self.queue_frame(Frame(Command.UNSUBSCRIBE, {"id": sub.id}))
+ def _process_disconnect(self, frame, dispatcher): + r_id = frame.headers[Headers.RECEIPT] + if not r_id: + self.log.debug("No receipt id for disconnect frame") + # it is not mandatory to send receipt frame + return + + headers = {Headers.RECEIPT_ID: r_id} + self.queue_frame(Frame(Command.RECEIPT, headers)) +
class _Subscription(object):
diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py index 71fd2ab..66a096d 100644 --- a/lib/yajsonrpc/stompreactor.py +++ b/lib/yajsonrpc/stompreactor.py @@ -75,7 +75,8 @@ stomp.Command.CONNECT: self._cmd_connect, stomp.Command.SEND: self._cmd_send, stomp.Command.SUBSCRIBE: self._cmd_subscribe, - stomp.Command.UNSUBSCRIBE: self._cmd_unsubscribe} + stomp.Command.UNSUBSCRIBE: self._cmd_unsubscribe, + stomp.Command.DISCONNECT: self._cmd_disconnect}
@property def has_outgoing_messages(self): @@ -165,6 +166,18 @@ return else: self._remove_subscription(subscription) + + def _cmd_disconnect(self, dispatcher, frame): + self.log.info("Disconnect command received") + r_id = frame.headers[stomp.Headers.RECEIPT] + if not r_id: + self.log.debug("No receipt id for disconnect frame") + # it is not mandatory to send receipt frame + return + + headers = {stomp.Headers.RECEIPT_ID: r_id} + dispatcher.connection.send_raw(stomp.Frame(stomp.Command.RECEIPT, + headers))
def _remove_subscription(self, subscription): subs = self._sub_dests[subscription.destination] @@ -301,7 +314,6 @@ Sends message to all subscribes that subscribed to destination. """ def send(self, message, destination=stomp.LEGACY_SUBSCRIPTION_ID_RESPONSE): - self.log.debug("Sending response") try: resp = json.loads(message) destination = self._req_dest[resp.get("id")]
automation@ovirt.org has posted comments on this change.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
Patch Set 1:
* Update tracker::#1239062::OK * Check Bug-Url::OK * Check Public Bug::#1239062::OK, public bug * Check Product::#1239062::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
Patch Set 1: Verified+1
Verified by running UT and host deploying vdsm and seeing that communication is OK. I removed a host from the engine to see that processing of DISCONNECT frame is OK.
Yaniv Bronhaim has posted comments on this change.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
Patch Set 1:
(2 comments)
https://gerrit.ovirt.org/#/c/46931/1//COMMIT_MSG Commit Message:
Line 3: AuthorDate: 2015-10-02 11:27:23 +0200 Line 4: Commit: pkliczewski piotr.kliczewski@gmail.com Line 5: CommitDate: 2015-10-02 11:27:23 +0200 Line 6: Line 7: stomp: reducing log footprint from stomp lib this patch basically adds RECEIPT frame handling. its less about reducing logs Line 8: Line 9: There was bunch of not needed information about being not able to Line 10: process disconnect frame (part of the spec and expected) as well as a Line 11: lot of entries about messages being sent.
https://gerrit.ovirt.org/#/c/46931/1/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py:
Line 300 Line 301 Line 302 Line 303 Line 304 this log removal is less related
Piotr Kliczewski has posted comments on this change.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
Patch Set 1:
(2 comments)
https://gerrit.ovirt.org/#/c/46931/1//COMMIT_MSG Commit Message:
Line 3: AuthorDate: 2015-10-02 11:27:23 +0200 Line 4: Commit: pkliczewski piotr.kliczewski@gmail.com Line 5: CommitDate: 2015-10-02 11:27:23 +0200 Line 6: Line 7: stomp: reducing log footprint from stomp lib
this patch basically adds RECEIPT frame handling. its less about reducing l
In the logs we stated that the frame is not recognized even though it is part of spec. In order to reduce wrong/unneeded information we need to introduce new frame type to fix it. Line 8: Line 9: There was bunch of not needed information about being not able to Line 10: process disconnect frame (part of the spec and expected) as well as a Line 11: lot of entries about messages being sent.
https://gerrit.ovirt.org/#/c/46931/1/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py:
Line 300 Line 301 Line 302 Line 303 Line 304
this log removal is less related
This is the log statement which appears the most in the logs. it has no value for us.
automation@ovirt.org has posted comments on this change.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
Patch Set 2:
* Update tracker::#1239062::OK * Check Bug-Url::OK * Check Public Bug::#1239062::OK, public bug * Check Product::#1239062::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Yaniv Bronhaim has posted comments on this change.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
Patch Set 2: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
Patch Set 2: Code-Review+2
raising Yaniv's score
Dan Kenigsberg has submitted this change and it was merged.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
stomp: reducing log footprint from stomp lib
There was bunch of not needed information about being not able to process disconnect frame (part of the spec and expected). We made vdsm aware of disconnect frame and few headers which are required to validate the frame.
We removed log statement which generate the most information about message being sent but it do not bring any value when debugging issues.
Bug-Url: https://bugzilla.redhat.com/1239062 Change-Id: I98f0c443fd50ec9e05dcf71ee1c7baabd044f48d Signed-off-by: pkliczewski piotr.kliczewski@gmail.com Reviewed-on: https://gerrit.ovirt.org/46931 Continuous-Integration: Jenkins CI Reviewed-by: Yaniv Bronhaim ybronhei@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M lib/yajsonrpc/stomp.py M lib/yajsonrpc/stompreactor.py 2 files changed, 29 insertions(+), 3 deletions(-)
Approvals: Piotr Kliczewski: Verified Yaniv Bronhaim: Looks good to me, but someone else must approve Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved
automation@ovirt.org has posted comments on this change.
Change subject: stomp: reducing log footprint from stomp lib ......................................................................
Patch Set 3:
* Update tracker::#1239062::OK * Set MODIFIED::bug 1239062::::#1239062::::IGNORE, not oVirt prod but Red Hat Enterprise Virtualization Manager
vdsm-patches@lists.fedorahosted.org