From b052f06c0f52bd70c39573a09413f95a9fd3050a Mon Sep 17 00:00:00 2001
From: Anthony Shull <ashull@mbta.com>
Date: Wed, 29 May 2024 09:44:31 -0500
Subject: [PATCH] Track page visits so we can see where users are going (#2077)

* track visits

* ...

* docs

* ignore playwright

* try using cookies

* remove controller

* revert router

* rearrange

* cleanup

* change level back to notice

* user agent in monitor

* allow logs for tests

* move credo file and disable false positive check
---
 config/.credo.exs => .credo.exs |  1 +
 config/deps/logger.exs          | 15 ++++-----------
 integration/monitor/worker.js   |  2 +-
 lib/dotcom_web/plugs/cookies.ex | 22 +++++++++++++++++++---
 playwright.config.js            |  1 -
 5 files changed, 25 insertions(+), 16 deletions(-)
 rename config/.credo.exs => .credo.exs (98%)

diff --git a/config/.credo.exs b/.credo.exs
similarity index 98%
rename from config/.credo.exs
rename to .credo.exs
index e6de95eae1..f3a056cb77 100644
--- a/config/.credo.exs
+++ b/.credo.exs
@@ -138,6 +138,7 @@
         {Credo.Check.Warning.IoInspect, []},
         # false because not compatible with Elixir 1.10.3
         {Credo.Check.Warning.LazyLogging, false},
+        {Credo.Check.Warning.MissedMetadataKeyInLoggerConfig, false},
         {Credo.Check.Warning.OperationOnSameValues, []},
         {Credo.Check.Warning.OperationWithConstantResult, []},
         {Credo.Check.Warning.RaiseInsideRescue, []},
diff --git a/config/deps/logger.exs b/config/deps/logger.exs
index deb5840f70..843f935731 100644
--- a/config/deps/logger.exs
+++ b/config/deps/logger.exs
@@ -1,10 +1,5 @@
 import Config
 
-# Configures Elixir's Logger
-config :logger, :console,
-  format: "$date $time $metadata[$level] $message\n",
-  metadata: [:ip, :request_id]
-
 # Include referrer in Logster request log
 config :logster, :allowed_headers, ["referer"]
 
@@ -18,16 +13,14 @@ if config_env() == :prod do
   config :logger, :console,
     level: :info,
     format: "$dateT$time [$level]$levelpad node=$node $metadata$message\n",
-    metadata: [:request_id, :ip]
+    metadata: [:ip, :mbta_id, :request_id]
 end
 
 if config_env() == :dev do
-  # Do not include metadata nor timestamps in development logs
-  config :logger, :console, format: "[$level] $message\n"
-
-  config :logger,
+  config :logger, :console,
+    format: "$date $time [$level] $metadata$message\n",
     level: :notice,
-    colors: [enabled: true]
+    metadata: [:ip, :mbta_id, :request_id]
 end
 
 if config_env() == :test do
diff --git a/integration/monitor/worker.js b/integration/monitor/worker.js
index e23ddb221a..9fbb3ec780 100644
--- a/integration/monitor/worker.js
+++ b/integration/monitor/worker.js
@@ -17,7 +17,7 @@ parentPort.on("message", async (_) => {
   const { scenario } = require(workerData.path);
 
   const browser = await chromium.launch();
-  const context = await browser.newContext();
+  const context = await browser.newContext({ userAgent: 'Playwright' });
   const page = await context.newPage();
 
   const metric = `${prefix}${workerData.name}`;
diff --git a/lib/dotcom_web/plugs/cookies.ex b/lib/dotcom_web/plugs/cookies.ex
index 2b77c2188e..725af60df1 100644
--- a/lib/dotcom_web/plugs/cookies.ex
+++ b/lib/dotcom_web/plugs/cookies.ex
@@ -3,6 +3,8 @@ defmodule DotcomWeb.Plugs.Cookies do
   A module Plug that creates a cookie with a unique ID if this cookie does not already exist.
   """
 
+  require Logger
+
   alias Plug.Conn
 
   @behaviour Plug
@@ -37,15 +39,29 @@ defmodule DotcomWeb.Plugs.Cookies do
   def route_cookie_name, do: @route_cookie_name
 
   @doc """
-  Sets a unique ID for every visitor. ID is never overwritten once it exists.
+  Sets a unique id cookie if it does not already exist.
+  If the user-agent is not Playwright (meaning it is a real user), then the id is added to the Logger metadata.
   """
   @spec set_id_cookie(Conn.t()) :: Conn.t()
-  def set_id_cookie(%{cookies: %{@id_cookie_name => _mbta_id}} = conn) do
+  def set_id_cookie(%{cookies: %{@id_cookie_name => mbta_id}} = conn) do
     conn
+    |> maybe_set_metadata(mbta_id)
   end
 
   def set_id_cookie(conn) do
-    Conn.put_resp_cookie(conn, @id_cookie_name, unique_id(), @id_cookie_options)
+    mbta_id = unique_id()
+
+    conn
+    |> maybe_set_metadata(mbta_id)
+    |> Conn.put_resp_cookie(@id_cookie_name, mbta_id, @id_cookie_options)
+  end
+
+  defp maybe_set_metadata(conn, mbta_id) do
+    unless Conn.get_req_header(conn, "user-agent") == ["Playwright"] do
+      Logger.metadata(mbta_id: mbta_id)
+    end
+
+    conn
   end
 
   @spec unique_id() :: String.t()
diff --git a/playwright.config.js b/playwright.config.js
index 907fa8e4a8..6738c3b223 100644
--- a/playwright.config.js
+++ b/playwright.config.js
@@ -18,7 +18,6 @@ module.exports = defineConfig({
     baseURL: process.env.HOST ? `https://${process.env.HOST}` : 'http://localhost:4001',
     /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
     trace: 'on-first-retry',
-    userAgent: 'Playwright',
   },
   /* set the expect timeout to 30s */
   expect: { timeout: 30000 },