From 227947c83836dc302cdf35af77718f5416af48cf Mon Sep 17 00:00:00 2001
From: Lawrence Jones <lawrjone@gmail.com>
Date: Sat, 5 Aug 2017 18:51:21 +0100
Subject: [PATCH 1/2] Use instrument for coach.handler.finish event

This change uses ActiveSupport::Notifications.instrument to time and
publish the event for coach.handler.finish. This makes Coach consistent
with how it handles handler and middleware events. Such a change should
not break any integrations, and can therefore be included in a patch.

The upshot of this is that integrating Coach with third-party
instrumentation tools then becomes a small bit easier. This is because
using #instrument over #publish will trigger start and end events under
the same key, if your subscriber is listening appropriately.
---
 lib/coach/handler.rb | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/lib/coach/handler.rb b/lib/coach/handler.rb
index 786d1a4..980b949 100644
--- a/lib/coach/handler.rb
+++ b/lib/coach/handler.rb
@@ -15,6 +15,7 @@ def initialize(middleware, config = {})
 
     # Run validation on the root of the middleware chain
     delegate :validate!, to: :@root_item
+    delegate :publish, :instrument, to: ActiveSupport::Notifications
 
     # The Rack interface to handler - builds a middleware chain based on
     # the current request, and invokes it.
@@ -23,20 +24,25 @@ def call(env)
       sequence = build_sequence(@root_item, context)
       chain = build_request_chain(sequence, context)
 
-      start_event = start_event(context)
-      start = Time.now
+      event = build_event(context)
 
-      publish('coach.handler.start', start_event.dup)
-
-      begin
-        response = chain.instrument.call
-      ensure
-        status = response.try(:first) || STATUS_CODE_FOR_EXCEPTIONS
-        publish('coach.handler.finish', start, Time.now, nil,
-                start_event.merge(
-                  response: { status: status },
-                  metadata: context.fetch(:_metadata, {})
-                ))
+      publish('coach.handler.start', event.dup)
+      instrument('coach.handler.finish', event) do
+        begin
+          response = chain.instrument.call
+        ensure
+          # We want to populate the response and metadata fields after the middleware
+          # chain has completed so that the end of the instrumentation can see them. The
+          # simplest way to do this is pass the event by reference to ActiveSupport, then
+          # modify the hash to contain this detail before the instrumentation completes.
+          #
+          # This way, the last coach.handler.finish event will have all the details.
+          status = response.try(:first) || STATUS_CODE_FOR_EXCEPTIONS
+          event.merge!(
+            response: { status: status },
+            metadata: context.fetch(:_metadata, {})
+          )
+        end
       end
     end
 
@@ -65,19 +71,14 @@ def inspect
 
     private
 
-    # Trigger ActiveSupport::Notification
-    def publish(name, *args)
-      ActiveSupport::Notifications.publish(name, *args)
-    end
-
     # Remove middleware that have been included multiple times with the same
     # config, leaving only the first instance
     def dedup_sequence(sequence)
       sequence.uniq { |item| [item.class, item.middleware, item.config] }
     end
 
-    # Event to send for start of handler
-    def start_event(context)
+    # Event to send with notifications
+    def build_event(context)
       {
         middleware: @root_item.middleware.name,
         request: context[:request]

From c3b17f99d59bc29ec2d3589d9cbd3a230ec0736e Mon Sep 17 00:00:00 2001
From: Lawrence Jones <lawrjone@gmail.com>
Date: Mon, 7 Aug 2017 10:05:30 +0100
Subject: [PATCH 2/2] Bump to v0.5.0

---
 CHANGELOG.md         | 9 ++++++++-
 lib/coach/version.rb | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fb9b647..3e053cb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,11 @@
-# Unreleased changes
+# CHANGELOG
+
+# 0.5.0 / 2017-08-07
+
+* [https://github.com/gocardless/coach/pull/24](#24) Use
+  ActiveSupport.instrument for coach.handler.finish event. Potentially breaking
+  change as the coach.handler.finish subscribers may trigger twice, depending on
+  how users have subscribed to them.
 
 # 0.4.6
 
diff --git a/lib/coach/version.rb b/lib/coach/version.rb
index 82c28e5..9c79a10 100644
--- a/lib/coach/version.rb
+++ b/lib/coach/version.rb
@@ -1,3 +1,3 @@
 module Coach
-  VERSION = '0.4.6'.freeze
+  VERSION = '0.5.0'.freeze
 end