From 0a883f10e7c72b103fbf29bd9605649420b756d4 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Tue, 7 May 2024 15:03:37 +0300 Subject: [PATCH] Refactor: Use common current_visitors code (#4071) * Use common module for counting current visitors in external stats controller * Refactor spike notifier, remove now-dead code --- lib/plausible/stats/clickhouse.ex | 219 +----------------- .../api/external_stats_controller.ex | 3 +- lib/workers/spike_notifier.ex | 4 +- test/workers/spike_notifier_test.exs | 10 +- 4 files changed, 10 insertions(+), 226 deletions(-) diff --git a/lib/plausible/stats/clickhouse.ex b/lib/plausible/stats/clickhouse.ex index 52d2e889c..f217d64f3 100644 --- a/lib/plausible/stats/clickhouse.ex +++ b/lib/plausible/stats/clickhouse.ex @@ -120,12 +120,8 @@ defmodule Plausible.Stats.Clickhouse do ClickhouseRepo.all(referrers) end - def current_visitors(site, query) do - Plausible.ClickhouseRepo.one( - from(e in base_query(site, query), - select: uniq(e.user_id) - ) - ) + def current_visitors(site) do + Plausible.Stats.current_visitors(site) end def has_pageviews?(site) do @@ -251,140 +247,6 @@ defmodule Plausible.Stats.Clickhouse do end end - defp base_query_bare(site, query) do - {first_datetime, last_datetime} = utc_boundaries(query, site) - - q = - from(e in "events_v2", - where: e.site_id == ^site.id, - where: e.timestamp >= ^first_datetime and e.timestamp < ^last_datetime - ) - - on_ee do - q = Plausible.Stats.Sampling.add_query_hint(q, 10_000_000) - end - - q = - if query.filters["screen"] do - size = query.filters["screen"] - from(e in q, where: e.screen_size == ^size) - else - q - end - - q = - if query.filters["browser"] do - browser = query.filters["browser"] - from(s in q, where: s.browser == ^browser) - else - q - end - - q = - if query.filters["browser_version"] do - version = query.filters["browser_version"] - from(s in q, where: s.browser_version == ^version) - else - q - end - - q = - if query.filters["os"] do - os = query.filters["os"] - from(s in q, where: s.operating_system == ^os) - else - q - end - - q = - if query.filters["os_version"] do - version = query.filters["os_version"] - from(s in q, where: s.operating_system_version == ^version) - else - q - end - - q = - if query.filters["country"] do - country = query.filters["country"] - from(s in q, where: s.country_code == ^country) - else - q - end - - q = - if query.filters["utm_medium"] do - utm_medium = query.filters["utm_medium"] - from(e in q, where: e.utm_medium == ^utm_medium) - else - q - end - - q = - if query.filters["utm_source"] do - utm_source = query.filters["utm_source"] - from(e in q, where: e.utm_source == ^utm_source) - else - q - end - - q = - if query.filters["utm_campaign"] do - utm_campaign = query.filters["utm_campaign"] - from(e in q, where: e.utm_campaign == ^utm_campaign) - else - q - end - - q = - if query.filters["utm_content"] do - utm_content = query.filters["utm_content"] - from(e in q, where: e.utm_content == ^utm_content) - else - q - end - - q = - if query.filters["utm_term"] do - utm_term = query.filters["utm_term"] - from(e in q, where: e.utm_term == ^utm_term) - else - q - end - - q = - if query.filters["referrer"] do - ref = query.filters["referrer"] - from(e in q, where: e.referrer == ^ref) - else - q - end - - q = include_path_filter(q, query.filters[:page]) - - if query.filters["props"] do - [{key, val}] = query.filters["props"] |> Enum.into([]) - - if val == "(none)" do - from( - e in q, - where: not has_key(e, :meta, ^key) - ) - else - from( - e in q, - where: has_key(e, :meta, ^key) and get_by_key(e, :meta, ^key) == ^val - ) - end - else - q - end - end - - defp base_query(site, query) do - base_query_bare(site, query) |> include_goal_conversions(query) - end - defp utc_boundaries(%Query{now: now, period: "30m"}, site) do last_datetime = now |> NaiveDateTime.truncate(:second) @@ -425,83 +287,6 @@ defmodule Plausible.Stats.Clickhouse do {first_datetime, last_datetime} end - defp event_name_for_goal(query) do - case query.filters["goal"] do - "Visit " <> page -> - {"pageview", page} - - goal when is_binary(goal) -> - {goal, nil} - - _ -> - {nil, nil} - end - end - - defp include_goal_conversions(db_query, query) do - {goal_event, path} = event_name_for_goal(query) - - q = - if goal_event do - from(e in db_query, where: e.name == ^goal_event) - else - from(e in db_query, where: e.name == "pageview") - end - - if path do - {contains_regex, path_regex} = convert_path_regex(path) - - if contains_regex do - from(e in q, where: fragment("match(?, ?)", e.pathname, ^path_regex)) - else - from(e in q, where: e.pathname == ^path) - end - else - q - end - end - - defp check_negated_filter(filter) do - negated = String.at(filter, 0) == "!" - updated_filter = if negated, do: String.slice(filter, 1..-1), else: filter - - {negated, updated_filter} - end - - defp convert_path_regex(path) do - contains_regex = String.match?(path, ~r/\*/) - - regex = - "^#{path}\/?$" - |> String.replace(~r/\*\*/, ".*") - |> String.replace(~r/(? "realtime"}) - json(conn, Plausible.Stats.Clickhouse.current_visitors(site, query)) + json(conn, Plausible.Stats.current_visitors(site)) end def aggregate(conn, params) do diff --git a/lib/workers/spike_notifier.ex b/lib/workers/spike_notifier.ex index 3505f5564..57e18c398 100644 --- a/lib/workers/spike_notifier.ex +++ b/lib/workers/spike_notifier.ex @@ -19,10 +19,10 @@ defmodule Plausible.Workers.SpikeNotifier do ) for notification <- notifications do - query = Query.from(notification.site, %{"period" => "realtime"}) - current_visitors = clickhouse.current_visitors(notification.site, query) + current_visitors = clickhouse.current_visitors(notification.site) if current_visitors >= notification.threshold do + query = Query.from(notification.site, %{"period" => "realtime"}) sources = clickhouse.top_sources_for_spike(notification.site, query, 3, 1) notify(notification, current_visitors, sources) end diff --git a/test/workers/spike_notifier_test.exs b/test/workers/spike_notifier_test.exs index 522f05d7e..d24fc7a98 100644 --- a/test/workers/spike_notifier_test.exs +++ b/test/workers/spike_notifier_test.exs @@ -14,7 +14,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do ) clickhouse_stub = - stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 5 end) + stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 5 end) |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end) SpikeNotifier.perform(nil, clickhouse_stub) @@ -32,7 +32,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do ) clickhouse_stub = - stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 10 end) + stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 10 end) |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end) SpikeNotifier.perform(nil, clickhouse_stub) @@ -58,7 +58,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do ) clickhouse_stub = - stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 10 end) + stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 10 end) |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end) SpikeNotifier.perform(nil, clickhouse_stub) @@ -71,7 +71,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do insert(:spike_notification, site: site, threshold: 10, recipients: ["uku@example.com"]) clickhouse_stub = - stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 10 end) + stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 10 end) |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end) SpikeNotifier.perform(nil, clickhouse_stub) @@ -92,7 +92,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do insert(:spike_notification, site: site, threshold: 10, recipients: ["robert@example.com"]) clickhouse_stub = - stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 10 end) + stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 10 end) |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end) SpikeNotifier.perform(nil, clickhouse_stub)