Repository: phoenixframework/phoenix
Pull request #4210 (2021-02-16 07:19:03)

Add links for Plug callbacks
Make references to Plug callbacks valid links for easier navigation.

![Screenshot from 2021-02-16 16-11-00](https://user-images.githubusercontent.com/11598866/108030419-8bdc8600-7072-11eb-83fc-1f97dbfd55b5.png)


sabiwara: > Hi @sabiwara
> I think it is better to use reference links since they are links that are used repetedly.
> 
> Please read this discussion: [#4202 (review)](https://github.com/phoenixframework/phoenix/pull/4202#pullrequestreview-588497696)

Hi @eksperimental , thank you for the great suggestion, it makes the markdown much cleaner :+1: 

Patch:
diff --git a/guides/plug.md b/guides/plug.md
index 3220c254a..abcab2003 100644
--- a/guides/plug.md
+++ b/guides/plug.md
@@ -73,8 +73,8 @@ Now let's look at the other plug variant, the module plugs.
 
 Module plugs are another type of plug that let us define a connection transformation in a module. The module only needs to implement two functions:
 
-- `init/1` which initializes any arguments or options to be passed to `call/2`
-- `call/2` which carries out the connection transformation. `call/2` is just a function plug that we saw earlier
+- [`init/1`] which initializes any arguments or options to be passed to [`call/2`]
+- [`call/2`] which carries out the connection transformation. [`call/2`] is just a function plug that we saw earlier
 
 To see this in action, let's write a module plug that puts the `:locale` key and value into the connection assign for downstream use in other plugs, controller actions, and our views. Put the contents below in a file named `lib/hello_web/plugs/locale.ex`:
 
@@ -113,7 +113,7 @@ defmodule HelloWeb.Router do
   ...
 ```
 
-In the `init/1` callback, we pass a default locale to use if none is present in the params. We also use pattern matching to define multiple `call/2` function heads to validate the locale in the params, and fall back to `"en"` if there is no match.
+In the [`init/1`] callback, we pass a default locale to use if none is present in the params. We also use pattern matching to define multiple [`call/2`] function heads to validate the locale in the params, and fall back to `"en"` if there is no match.
 
 To see the assign in action, go to the layout in `lib/hello_web/templates/layout/app.html.eex` and add the following close to the main container:
 
@@ -301,3 +301,7 @@ To make this all work, we converted the nested blocks of code and used `halt(con
 At the end of the day, by replacing the nested blocks of code with a flattened series of plug transformations, we are able to achieve the same functionality in a much more composable, clear, and reusable way.
 
 To learn more about plugs, see the documentation for the [Plug project](https://hexdocs.pm/plug), which provides many built-in plugs and functionalities.
+
+
+[`init/1`]: `c:Plug.init/1`
+[`call/2`]: `c:Plug.call/2`
diff --git a/lib/phoenix/controller.ex b/lib/phoenix/controller.ex
index fffe03470..20f9dc131 100644
--- a/lib/phoenix/controller.ex
+++ b/lib/phoenix/controller.ex
@@ -120,8 +120,8 @@ defmodule Phoenix.Controller do
   invoke the inner action plug that dispatches to the `show/2`
   function in the `UserController`.
 
-  As controllers are plugs, they implement both `init/1` and
-  `call/2`, and it also provides a function named `action/2`
+  As controllers are plugs, they implement both [`init/1`](`c:Plug.init/1`) and
+  [`call/2`](`c:Plug.call/2`), and it also provides a function named `action/2`
   which is responsible for dispatching the appropriate action
   after the plug stack (and is also overridable).
 
diff --git a/lib/phoenix/endpoint.ex b/lib/phoenix/endpoint.ex
index a934eac2d..ec1158f63 100644
--- a/lib/phoenix/endpoint.ex
+++ b/lib/phoenix/endpoint.ex
@@ -66,7 +66,7 @@ defmodule Phoenix.Endpoint do
 
   For dynamically configuring the endpoint, such as loading data
   from environment variables or configuration files, Phoenix invokes
-  the `init/2` callback on the endpoint, passing the atom `:supervisor`
+  the `c:init/2` callback on the endpoint, passing the atom `:supervisor`
   as the first argument and the endpoint configuration as second.
 
   All of Phoenix configuration, except the Compile-time configuration
diff --git a/lib/phoenix/router.ex b/lib/phoenix/router.ex
index e272e16e4..9ac2e5e23 100644
--- a/lib/phoenix/router.ex
+++ b/lib/phoenix/router.ex
@@ -359,13 +359,13 @@ defmodule Phoenix.Router do
         rescue
           e in Plug.Conn.WrapperError ->
             measurements = %{duration: System.monotonic_time() - start}
-            metadata = %{conn: conn, kind: :error, reason: e, stacktrace: __STACKTRACE__}
+            metadata = Map.merge(metadata, %{conn: conn, kind: :error, reason: e, stacktrace: __STACKTRACE__})
             :telemetry.execute([:phoenix, :router_dispatch, :exception], measurements, metadata)
             Plug.Conn.WrapperError.reraise(e)
         catch
           kind, reason ->
             measurements = %{duration: System.monotonic_time() - start}
-            metadata = %{conn: conn, kind: kind, reason: reason, stacktrace: __STACKTRACE__}
+            metadata = Map.merge(metadata, %{conn: conn, kind: kind, reason: reason, stacktrace: __STACKTRACE__})
             :telemetry.execute([:phoenix, :router_dispatch, :exception], measurements, metadata)
             Plug.Conn.WrapperError.reraise(piped_conn, kind, reason, __STACKTRACE__)
         end


Test patch:
diff --git a/test/phoenix/router/routing_test.exs b/test/phoenix/router/routing_test.exs
index c76564bc6..c5d1b5933 100644
--- a/test/phoenix/router/routing_test.exs
+++ b/test/phoenix/router/routing_test.exs
@@ -21,6 +21,8 @@ defmodule Phoenix.Router.RoutingTest do
     def image(conn, _params), do: text(conn, conn.params["path"] || "show files")
     def move(conn, _params), do: text(conn, "users move")
     def any(conn, _params), do: text(conn, "users any")
+    def raise(_conn, _params), do: raise("boom")
+    def exit(_conn, _params), do: exit(:boom)
   end
 
   defmodule LogLevel do
@@ -41,6 +43,7 @@ defmodule Phoenix.Router.RoutingTest do
     get "/files/:user_name/*path", UserController, :image
     get "/backups/*path", UserController, :image
     get "/static/images/icons/*image", UserController, :image
+    get "/exit", UserController, :exit
 
     trace("/trace", UserController, :trace)
     options "/options", UserController, :options
@@ -51,6 +54,7 @@ defmodule Phoenix.Router.RoutingTest do
     scope log: :info do
       pipe_through :noop
       get "/plug", SomePlug, []
+      get "/users/:id/raise", UserController, :raise
     end
 
     get "/no_log", SomePlug, [], log: false
@@ -244,6 +248,144 @@ defmodule Phoenix.Router.RoutingTest do
     end
   end
 
+  describe "telemetry" do
+    @router_start_event [:phoenix, :router_dispatch, :start]
+    @router_stop_event [:phoenix, :router_dispatch, :stop]
+    @router_exception_event [:phoenix, :router_dispatch, :exception]
+    @router_events [
+      @router_start_event,
+      @router_stop_event,
+      @router_exception_event
+    ]
+
+    setup context do
+      test_pid = self()
+      test_name = context.test
+
+      :telemetry.attach_many(
+        test_name,
+        @router_events,
+        fn event, measures, metadata, config ->
+          send(test_pid, {:telemetry_event, event, {measures, metadata, config}})
+        end,
+        nil
+      )
+    end
+
+    test "phoenix.router_dispatch.start and .stop are emitted on success" do
+      call(Router, :get, "users/123")
+      assert_received {:telemetry_event, @router_start_event, _event}
+      assert_received {:telemetry_event, @router_stop_event, _event}
+      refute_received {:telemetry_event, @router_exception_event, _event}
+    end
+
+    test "phoenix.router_dispatch.start and .exception are emitted on crash" do
+      assert_raise Plug.Conn.WrapperError, ~r/UndefinedFunctionError/, fn ->
+        call(Router, :get, "route_that_crashes")
+      end
+
+      assert_received {:telemetry_event, @router_start_event, _event}
+      assert_received {:telemetry_event, @router_exception_event, _event}
+      refute_received {:telemetry_event, @router_stop_event, _event}
+    end
+
+    test "phoenix.router_dispatch.start and .exception are emitted on exit" do
+      catch_exit(call(Router, :get, "exit"))
+
+      assert_received {:telemetry_event, @router_start_event, _event}
+      assert_received {:telemetry_event, @router_exception_event, _event}
+      refute_received {:telemetry_event, @router_stop_event, _event}
+    end
+
+    test "phoenix.router_dispatch.start has supported measurements and metadata" do
+      call(Router, :get, "users/123")
+
+      assert_received {:telemetry_event, @router_start_event, {measures, meta, _config}}
+      assert is_integer(measures.system_time)
+
+      assert %{
+               access: :user,
+               conn: %Plug.Conn{state: :unset},
+               log: :debug,
+               path_params: %{"id" => "123"},
+               pipe_through: [],
+               plug: Phoenix.Router.RoutingTest.UserController,
+               plug_opts: :show,
+               route: "/users/:id"
+             } = meta
+    end
+
+    test "phoenix.router_dispatch.stop has supported measurements and metadata" do
+      call(Router, :get, "users/123")
+
+      assert_received {:telemetry_event, @router_stop_event, {measures, meta, _config}}
+      assert is_integer(measures.duration)
+
+      assert %{
+               access: :user,
+               conn: %Plug.Conn{state: :sent},
+               log: :debug,
+               path_params: %{"id" => "123"},
+               pipe_through: [],
+               plug: Phoenix.Router.RoutingTest.UserController,
+               plug_opts: :show,
+               route: "/users/:id"
+             } = meta
+    end
+
+    test "phoenix.router_dispatch.exception has supported measurements and metadata on crash" do
+      assert_raise Plug.Conn.WrapperError, "** (RuntimeError) boom", fn ->
+        call(Router, :get, "users/123/raise")
+      end
+
+      assert_received {:telemetry_event, @router_exception_event, {measures, meta, _config}}
+      assert is_integer(measures.duration)
+
+      assert %{
+               conn: %Plug.Conn{state: :unset},
+               kind: :error,
+               log: :info,
+               path_params: %{"id" => "123"},
+               pipe_through: [:noop],
+               plug: Phoenix.Router.RoutingTest.UserController,
+               plug_opts: :raise,
+               reason: %Plug.Conn.WrapperError{
+                 conn: %Plug.Conn{state: :unset},
+                 kind: :error,
+                 reason: %RuntimeError{message: "boom"},
+                 stack: wrapped_stacktrace
+               },
+               route: "/users/:id/raise",
+               stacktrace: stacktrace
+             } = meta
+
+      assert is_list(wrapped_stacktrace) && length(wrapped_stacktrace) > 0
+      assert is_list(stacktrace) && length(stacktrace) > 0
+    end
+
+    test "phoenix.router_dispatch.exception has supported measurements and metadata on exit" do
+      catch_exit(call(Router, :get, "exit"))
+
+      assert_received {:telemetry_event, @router_exception_event, {measures, meta, _config}}
+      assert is_integer(measures.duration)
+
+      assert %{
+               conn: %Plug.Conn{state: :unset},
+               kind: :exit,
+               log: :debug,
+               path_params: %{},
+               pipe_through: [],
+               plug: Phoenix.Router.RoutingTest.UserController,
+               plug_opts: :exit,
+               reason: :boom,
+               route: "/exit",
+               stacktrace: stacktrace
+             } = meta
+
+      assert is_list(stacktrace) && length(stacktrace) > 0
+    end
+  end
+
   test "route_info returns route string, path params, and more" do
     assert Phoenix.Router.route_info(Router, "GET", "foo/bar/baz", nil) == %{
              log: :debug,