Use 'Exception.__traceback__' for versioned notifications

The 'inspect.trace()' function is expected to be called within the
context of an exception handler. The 'from_exc_and_traceback' class
method of the 'nova.notification.objects.exception.ExceptionPayload'
class uses this to get information about a provided exception, however,
there are cases where this is called from outside of an exception
handler. In these cases, we see an 'IndexError' since we can't get the
last frame of a non-existent stacktrace. The solution to this is to
fallback to using the traceback embedded in the exception. This is a bit
lossy when decorators are involved but for all other cases this will
give us the same information. This also allows us to avoid passing a
traceback argument to the function since we have it to hand already.

Change-Id: I404ca316b1bf2a963106cd34e927934befbd9b12
Signed-off-by: Stephen Finucane <[email protected]>
Closes-Bug: #1881455

Author: stephenfin

nova/compute/manager.py

@@ -2384,42 +2384,38 @@ def _build_and_run_instance(self, context, instance, image, injected_files,
             with excutils.save_and_reraise_exception():
                 self._notify_about_instance_usage(context, instance,
                     'create.error', fault=e)
-                tb = traceback.format_exc()
                 compute_utils.notify_about_instance_create(
                     context, instance, self.host,
                     phase=fields.NotificationPhase.ERROR, exception=e,
-                    bdms=block_device_mapping, tb=tb)
+                    bdms=block_device_mapping)
         except exception.ComputeResourcesUnavailable as e:
             LOG.debug(e.format_message(), instance=instance)
             self._notify_about_instance_usage(context, instance,
                     'create.error', fault=e)
-            tb = traceback.format_exc()
             compute_utils.notify_about_instance_create(
                     context, instance, self.host,
                     phase=fields.NotificationPhase.ERROR, exception=e,
-                    bdms=block_device_mapping, tb=tb)
+                    bdms=block_device_mapping)
             raise exception.RescheduledException(
                     instance_uuid=instance.uuid, reason=e.format_message())
         except exception.BuildAbortException as e:
             with excutils.save_and_reraise_exception():
                 LOG.debug(e.format_message(), instance=instance)
                 self._notify_about_instance_usage(context, instance,
                     'create.error', fault=e)
-                tb = traceback.format_exc()
                 compute_utils.notify_about_instance_create(
                     context, instance, self.host,
                     phase=fields.NotificationPhase.ERROR, exception=e,
-                    bdms=block_device_mapping, tb=tb)
+                    bdms=block_device_mapping)
         except exception.NoMoreFixedIps as e:
             LOG.warning('No more fixed IP to be allocated',
                         instance=instance)
             self._notify_about_instance_usage(context, instance,
                     'create.error', fault=e)
-            tb = traceback.format_exc()
             compute_utils.notify_about_instance_create(
                     context, instance, self.host,
                     phase=fields.NotificationPhase.ERROR, exception=e,
-                    bdms=block_device_mapping, tb=tb)
+                    bdms=block_device_mapping)
             msg = _('Failed to allocate the network(s) with error %s, '
                     'not rescheduling.') % e.format_message()
             raise exception.BuildAbortException(instance_uuid=instance.uuid,
@@ -2434,11 +2430,10 @@ def _build_and_run_instance(self, context, instance, image, injected_files,
                           instance=instance)
             self._notify_about_instance_usage(context, instance,
                     'create.error', fault=e)
-            tb = traceback.format_exc()
             compute_utils.notify_about_instance_create(
                     context, instance, self.host,
                     phase=fields.NotificationPhase.ERROR, exception=e,
-                    bdms=block_device_mapping, tb=tb)
+                    bdms=block_device_mapping)
             msg = _('Failed to allocate the network(s), not rescheduling.')
             raise exception.BuildAbortException(instance_uuid=instance.uuid,
                     reason=msg)
@@ -2457,23 +2452,21 @@ def _build_and_run_instance(self, context, instance, image, injected_files,
                 exception.RequestedVRamTooHigh) as e:
             self._notify_about_instance_usage(context, instance,
                     'create.error', fault=e)
-            tb = traceback.format_exc()
             compute_utils.notify_about_instance_create(
                     context, instance, self.host,
                     phase=fields.NotificationPhase.ERROR, exception=e,
-                    bdms=block_device_mapping, tb=tb)
+                    bdms=block_device_mapping)
             raise exception.BuildAbortException(instance_uuid=instance.uuid,
                     reason=e.format_message())
         except Exception as e:
             LOG.exception('Failed to build and run instance',
                           instance=instance)
             self._notify_about_instance_usage(context, instance,
                     'create.error', fault=e)
-            tb = traceback.format_exc()
             compute_utils.notify_about_instance_create(
                     context, instance, self.host,
                     phase=fields.NotificationPhase.ERROR, exception=e,
-                    bdms=block_device_mapping, tb=tb)
+                    bdms=block_device_mapping)
             raise exception.RescheduledException(
                     instance_uuid=instance.uuid, reason=six.text_type(e))
 
@@ -2505,11 +2498,10 @@ def _build_and_run_instance(self, context, instance, image, injected_files,
             with excutils.save_and_reraise_exception():
                 self._notify_about_instance_usage(context, instance,
                     'create.error', fault=e)
-                tb = traceback.format_exc()
                 compute_utils.notify_about_instance_create(
                     context, instance, self.host,
                     phase=fields.NotificationPhase.ERROR, exception=e,
-                    bdms=block_device_mapping, tb=tb)
+                    bdms=block_device_mapping)
 
         self._update_scheduler_instance_info(context, instance)
         self._notify_about_instance_usage(context, instance, 'create.end',
@@ -3280,13 +3272,11 @@ def _rebuild_default_impl(self, context, instance, image_meta,
                               block_device_info=new_block_device_info)
 
     def _notify_instance_rebuild_error(self, context, instance, error, bdms):
-        tb = traceback.format_exc()
         self._notify_about_instance_usage(context, instance,
                                           'rebuild.error', fault=error)
         compute_utils.notify_about_instance_rebuild(
             context, instance, self.host,
-            phase=fields.NotificationPhase.ERROR, exception=error, bdms=bdms,
-            tb=tb)
+            phase=fields.NotificationPhase.ERROR, exception=error, bdms=bdms)
 
     @messaging.expected_exceptions(exception.PreserveEphemeralNotSupported)
     @wrap_exception()
@@ -3793,12 +3783,11 @@ def bad_volumes_callback(bad_devices):
                             instance, error, exc_info)
                     self._notify_about_instance_usage(context, instance,
                             'reboot.error', fault=error)
-                    tb = traceback.format_exc()
                     compute_utils.notify_about_instance_action(
                         context, instance, self.host,
                         action=fields.NotificationAction.REBOOT,
                         phase=fields.NotificationPhase.ERROR,
-                        exception=error, bdms=bdms, tb=tb
+                        exception=error, bdms=bdms
                     )
                     ctxt.reraise = False
                 else:
@@ -5329,7 +5318,7 @@ def _reschedule_resize_or_reraise(self, context, instance, exc_info,
                 action=fields.NotificationAction.RESIZE,
                 phase=fields.NotificationPhase.ERROR,
                 exception=error,
-                tb=','.join(traceback.format_exception(*exc_info)))
+            )
 
         if rescheduled:
             self._log_original_error(exc_info, instance_uuid)
@@ -5342,7 +5331,7 @@ def _reschedule_resize_or_reraise(self, context, instance, exc_info,
                 action=fields.NotificationAction.RESIZE,
                 phase=fields.NotificationPhase.ERROR,
                 exception=exc_info[1],
-                tb=','.join(traceback.format_exception(*exc_info)))
+            )
         else:
             # not re-scheduling
             six.reraise(*exc_info)
@@ -7043,13 +7032,12 @@ def _attach_volume(self, context, instance, bdm):
                                   exc, instance=instance)
                 else:
                     self.volume_api.unreserve_volume(context, bdm.volume_id)
-                tb = traceback.format_exc()
                 compute_utils.notify_about_volume_attach_detach(
                     context, instance, self.host,
                     action=fields.NotificationAction.VOLUME_ATTACH,
                     phase=fields.NotificationPhase.ERROR,
                     exception=e,
-                    volume_id=bdm.volume_id, tb=tb)
+                    volume_id=bdm.volume_id)
 
         info = {'volume_id': bdm.volume_id}
         self._notify_about_instance_usage(
@@ -7250,11 +7238,10 @@ def _swap_volume(self, context, instance, bdm, connector,
         except Exception as ex:
             failed = True
             with excutils.save_and_reraise_exception():
-                tb = traceback.format_exc()
                 compute_utils.notify_about_volume_swap(
                     context, instance, self.host,
                     fields.NotificationPhase.ERROR,
-                    old_volume_id, new_volume_id, ex, tb)
+                    old_volume_id, new_volume_id, ex)
                 if new_cinfo:
                     msg = ("Failed to swap volume %(old_volume_id)s "
                            "for %(new_volume_id)s")
@@ -7563,12 +7550,11 @@ def attach_interface(self, context, instance, network_id, port_id,
                         instance=instance)
             self._deallocate_port_for_instance(context, instance, port_id)
 
-            tb = traceback.format_exc()
             compute_utils.notify_about_instance_action(
                 context, instance, self.host,
                 action=fields.NotificationAction.INTERFACE_ATTACH,
                 phase=fields.NotificationPhase.ERROR,
-                exception=ex, tb=tb)
+                exception=ex)
 
             raise exception.InterfaceAttachFailed(
                 instance_uuid=instance.uuid)

nova/compute/utils.py

@@ -452,22 +452,23 @@ def notify_about_instance_usage(notifier, context, instance, event_suffix,
     method(context, 'compute.instance.%s' % event_suffix, usage_info)
 
 
-def _get_fault_and_priority_from_exc_and_tb(exception, tb):
+def _get_fault_and_priority_from_exception(exception: Exception):
     fault = None
     priority = fields.NotificationPriority.INFO
 
-    if exception:
-        priority = fields.NotificationPriority.ERROR
-        fault = notification_exception.ExceptionPayload.from_exc_and_traceback(
-            exception, tb)
+    if not exception:
+        return fault, priority
+
+    fault = notification_exception.ExceptionPayload.from_exception(exception)
+    priority = fields.NotificationPriority.ERROR
 
     return fault, priority
 
 
 @rpc.if_notifications_enabled
 def notify_about_instance_action(context, instance, host, action, phase=None,
                                  source=fields.NotificationSource.COMPUTE,
-                                 exception=None, bdms=None, tb=None):
+                                 exception=None, bdms=None):
     """Send versioned notification about the action made on the instance
     :param instance: the instance which the action performed on
     :param host: the host emitting the notification
@@ -476,10 +477,9 @@ def notify_about_instance_action(context, instance, host, action, phase=None,
     :param source: the source of the notification
     :param exception: the thrown exception (used in error notifications)
     :param bdms: BlockDeviceMappingList object for the instance. If it is not
-                provided then we will load it from the db if so configured
-    :param tb: the traceback (used in error notifications)
+        provided then we will load it from the db if so configured
     """
-    fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb)
+    fault, priority = _get_fault_and_priority_from_exception(exception)
     payload = instance_notification.InstanceActionPayload(
             context=context,
             instance=instance,
@@ -500,7 +500,7 @@ def notify_about_instance_action(context, instance, host, action, phase=None,
 
 @rpc.if_notifications_enabled
 def notify_about_instance_create(context, instance, host, phase=None,
-                                 exception=None, bdms=None, tb=None):
+                                 exception=None, bdms=None):
     """Send versioned notification about instance creation
 
     :param context: the request context
@@ -510,9 +510,8 @@ def notify_about_instance_create(context, instance, host, phase=None,
     :param exception: the thrown exception (used in error notifications)
     :param bdms: BlockDeviceMappingList object for the instance. If it is not
                 provided then we will load it from the db if so configured
-    :param tb: the traceback (used in error notifications)
     """
-    fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb)
+    fault, priority = _get_fault_and_priority_from_exception(exception)
     payload = instance_notification.InstanceCreatePayload(
         context=context,
         instance=instance,
@@ -558,17 +557,16 @@ def notify_about_scheduler_action(context, request_spec, action, phase=None,
 
 @rpc.if_notifications_enabled
 def notify_about_volume_attach_detach(context, instance, host, action, phase,
-                                      volume_id=None, exception=None, tb=None):
+                                      volume_id=None, exception=None):
     """Send versioned notification about the action made on the instance
     :param instance: the instance which the action performed on
     :param host: the host emitting the notification
     :param action: the name of the action
     :param phase: the phase of the action
     :param volume_id: id of the volume will be attached
     :param exception: the thrown exception (used in error notifications)
-    :param tb: the traceback (used in error notifications)
     """
-    fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb)
+    fault, priority = _get_fault_and_priority_from_exception(exception)
     payload = instance_notification.InstanceActionVolumePayload(
             context=context,
             instance=instance,
@@ -590,17 +588,16 @@ def notify_about_volume_attach_detach(context, instance, host, action, phase,
 @rpc.if_notifications_enabled
 def notify_about_instance_rescue_action(context, instance, host,
                                         rescue_image_ref, phase=None,
-                                        exception=None, tb=None):
+                                        exception=None):
     """Send versioned notification about the action made on the instance
 
     :param instance: the instance which the action performed on
     :param host: the host emitting the notification
     :param rescue_image_ref: the rescue image ref
     :param phase: the phase of the action
     :param exception: the thrown exception (used in error notifications)
-    :param tb: the traceback (used in error notifications)
     """
-    fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb)
+    fault, priority = _get_fault_and_priority_from_exception(exception)
     payload = instance_notification.InstanceActionRescuePayload(
             context=context,
             instance=instance,
@@ -644,8 +641,7 @@ def notify_about_keypair_action(context, keypair, action, phase):
 
 @rpc.if_notifications_enabled
 def notify_about_volume_swap(context, instance, host, phase,
-                             old_volume_id, new_volume_id, exception=None,
-                             tb=None):
+                             old_volume_id, new_volume_id, exception=None):
     """Send versioned notification about the volume swap action
        on the instance
 
@@ -656,9 +652,8 @@ def notify_about_volume_swap(context, instance, host, phase,
     :param old_volume_id: the ID of the volume that is copied from and detached
     :param new_volume_id: the ID of the volume that is copied to and attached
     :param exception: an exception
-    :param tb: the traceback (used in error notifications)
     """
-    fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb)
+    fault, priority = _get_fault_and_priority_from_exception(exception)
     payload = instance_notification.InstanceActionVolumeSwapPayload(
         context=context,
         instance=instance,
@@ -877,7 +872,7 @@ def notify_about_instance_rebuild(context, instance, host,
                                   action=fields.NotificationAction.REBUILD,
                                   phase=None,
                                   source=fields.NotificationSource.COMPUTE,
-                                  exception=None, bdms=None, tb=None):
+                                  exception=None, bdms=None):
     """Send versioned notification about instance rebuild
 
     :param instance: the instance which the action performed on
@@ -888,9 +883,8 @@ def notify_about_instance_rebuild(context, instance, host,
     :param exception: the thrown exception (used in error notifications)
     :param bdms: BlockDeviceMappingList object for the instance. If it is not
                 provided then we will load it from the db if so configured
-    :param tb: the traceback (used in error notifications)
     """
-    fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb)
+    fault, priority = _get_fault_and_priority_from_exception(exception)
     payload = instance_notification.InstanceActionRebuildPayload(
             context=context,
             instance=instance,
@@ -938,15 +932,14 @@ def notify_about_metrics_update(context, host, host_ip, nodename,
 
 
 @rpc.if_notifications_enabled
-def notify_about_libvirt_connect_error(context, ip, exception, tb):
+def notify_about_libvirt_connect_error(context, ip, exception):
     """Send a versioned notification about libvirt connect error.
 
     :param context: the request context
     :param ip: the IP address of the host
     :param exception: the thrown exception
-    :param tb: the traceback
     """
-    fault, _ = _get_fault_and_priority_from_exc_and_tb(exception, tb)
+    fault, _ = _get_fault_and_priority_from_exception(exception)
     payload = libvirt_notification.LibvirtErrorPayload(ip=ip, reason=fault)
     notification = libvirt_notification.LibvirtErrorNotification(
         priority=fields.NotificationPriority.ERROR,
@@ -984,7 +977,7 @@ def notify_about_volume_usage(context, vol_usage, host):
 
 @rpc.if_notifications_enabled
 def notify_about_compute_task_error(context, action, instance_uuid,
-                                    request_spec, state, exception, tb):
+                                    request_spec, state, exception):
     """Send a versioned notification about compute task error.
 
     :param context: the request context
@@ -1001,7 +994,7 @@ def notify_about_compute_task_error(context, action, instance_uuid,
         request_spec = objects.RequestSpec.from_primitives(
             context, request_spec, {})
 
-    fault, _ = _get_fault_and_priority_from_exc_and_tb(exception, tb)
+    fault, _ = _get_fault_and_priority_from_exception(exception)
     payload = task_notification.ComputeTaskPayload(
         instance_uuid=instance_uuid, request_spec=request_spec, state=state,
         reason=fault)