From 41e4690116e7c5c785d96e5eb2d72e60b687818a Mon Sep 17 00:00:00 2001 From: Vignesh Joglekar Date: Tue, 18 May 2021 07:14:33 -0500 Subject: [PATCH] Adds Time on Page metric to Top Pages report (#1007) * First pass Needs more testing & potentially cleanup * Fixes tests, error handling * Formatting * Removes broken test * Fixes inconsistent test This was due to Clickhouse setup not inserting the sessions with the exact same timestamp consistently and making the test inconsistent * Combines `include` param, asyncs time_on_page and bounce_rate * Fixes CH error when no pageviews exist in period * Format * Changelog * Increases await timeout to accomodate larger data sets * Improves handling of timeout behavior * Fixes session-based filtering on time on page queries * Formatting * Removes old forced entry page modal from top pages report --- CHANGELOG.md | 1 + assets/js/dashboard/stats/modals/pages.js | 30 +++--- .../stats/modals/referrer-drilldown.js | 4 +- assets/js/dashboard/stats/modals/sources.js | 4 +- lib/plausible/stats/clickhouse.ex | 98 +++++++++++++++++-- .../controllers/api/stats_controller.ex | 12 +-- .../api/stats_controller/pages_test.exs | 8 +- .../api/stats_controller/sources_test.exs | 4 +- test/support/clickhouse_setup.ex | 2 +- 9 files changed, 120 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47b7b0564..65a8860c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. - New parameter `metrics` for the `/api/v1/stats/timeseries` endpoint plausible/analytics#952 - CSV export now includes pageviews, bounce rate and visit duration in addition to visitors plausible/analytics#952 - Send stats to multiple dashboards by configuring a comma-separated list of domains plausible/analytics#968 +- Time on Page metric available in detailed Top Pages report plausible/analytics#1007 ### Fixed - Fix weekly report time range plausible/analytics#951 diff --git a/assets/js/dashboard/stats/modals/pages.js b/assets/js/dashboard/stats/modals/pages.js index e4f180da9..cd0c8d4b6 100644 --- a/assets/js/dashboard/stats/modals/pages.js +++ b/assets/js/dashboard/stats/modals/pages.js @@ -4,7 +4,7 @@ import { withRouter } from 'react-router-dom' import Modal from './modal' import * as api from '../../api' -import numberFormatter from '../../number-formatter' +import numberFormatter, {durationFormatter} from '../../number-formatter' import {parseQuery} from '../../query' class PagesModal extends React.Component { @@ -24,24 +24,18 @@ class PagesModal extends React.Component { } loadPages() { - const include = this.showBounceRate() ? 'bounce_rate' : null + const detailed = this.showExtra() const {query, page, pages} = this.state; - const {filters} = query - if (filters.source || filters.referrer) { - api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/entry-pages`, query, {limit: 100, page, include}) - .then((res) => this.setState((state) => ({loading: false, pages: state.pages.concat(res), moreResultsAvailable: res.length === 100}))) - } else { - api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/pages`, query, {limit: 100, page, include}) - .then((res) => this.setState((state) => ({loading: false, pages: state.pages.concat(res), moreResultsAvailable: res.length === 100}))) - } + api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/pages`, query, {limit: 100, page, detailed}) + .then((res) => this.setState((state) => ({loading: false, pages: state.pages.concat(res), moreResultsAvailable: res.length === 100}))) } loadMore() { this.setState({loading: true, page: this.state.page + 1}, this.loadPages.bind(this)) } - showBounceRate() { + showExtra() { return this.state.query.period !== 'realtime' && !this.state.query.filters.goal } @@ -60,6 +54,7 @@ class PagesModal extends React.Component { renderPage(page) { const query = new URLSearchParams(window.location.search) + const timeOnPage = page['time_on_page'] ? durationFormatter(page['time_on_page']) : '-'; query.set('page', page.name) return ( @@ -69,7 +64,8 @@ class PagesModal extends React.Component { {numberFormatter(page.count)} {this.showPageviews() && {numberFormatter(page.pageviews)} } - {this.showBounceRate() && {this.formatBounceRate(page)} } + {this.showExtra() && {this.formatBounceRate(page)} } + {this.showExtra() && {timeOnPage} } ) } @@ -78,11 +74,6 @@ class PagesModal extends React.Component { return this.state.query.period === 'realtime' ? 'Current visitors' : 'Visitors' } - title() { - const {filters} = this.state.query - return (filters.source || filters.referrer) ? 'Entry Pages' : 'Top Pages' - } - renderLoading() { if (this.state.loading) { return
@@ -101,7 +92,7 @@ class PagesModal extends React.Component { if (this.state.pages) { return ( -

{this.title()}

+

Top Pages

@@ -111,7 +102,8 @@ class PagesModal extends React.Component { Page url { this.label() } {this.showPageviews() && Pageviews} - {this.showBounceRate() && Bounce rate} + {this.showExtra() && Bounce rate} + {this.showExtra() && Time on Page} diff --git a/assets/js/dashboard/stats/modals/referrer-drilldown.js b/assets/js/dashboard/stats/modals/referrer-drilldown.js index 50b20608d..7c4ad231b 100644 --- a/assets/js/dashboard/stats/modals/referrer-drilldown.js +++ b/assets/js/dashboard/stats/modals/referrer-drilldown.js @@ -17,9 +17,9 @@ class ReferrerDrilldownModal extends React.Component { } componentDidMount() { - const include = this.showExtra() ? 'bounce_rate,visit_duration' : null + const detailed = this.showExtra() - api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/referrers/${this.props.match.params.referrer}`, this.state.query, {limit: 100, include: include}) + api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/referrers/${this.props.match.params.referrer}`, this.state.query, {limit: 100, detailed}) .then((res) => this.setState({loading: false, referrers: res.referrers, totalVisitors: res.total_visitors})) } diff --git a/assets/js/dashboard/stats/modals/sources.js b/assets/js/dashboard/stats/modals/sources.js index ee41a3b03..bf56277e4 100644 --- a/assets/js/dashboard/stats/modals/sources.js +++ b/assets/js/dashboard/stats/modals/sources.js @@ -31,8 +31,8 @@ class SourcesModal extends React.Component { const {site} = this.props const {query, page, sources} = this.state - const include = this.showExtra() ? 'bounce_rate,visit_duration' : null - api.get(`/api/stats/${encodeURIComponent(site.domain)}/${this.currentFilter()}`, query, {limit: 100, page: page, include: include, show_noref: true}) + const detailed = this.showExtra() + api.get(`/api/stats/${encodeURIComponent(site.domain)}/${this.currentFilter()}`, query, {limit: 100, page, detailed, show_noref: true}) .then((res) => this.setState({loading: false, sources: sources.concat(res), moreResultsAvailable: res.length === 100})) } diff --git a/lib/plausible/stats/clickhouse.ex b/lib/plausible/stats/clickhouse.ex index aebe43bcd..a525799dd 100644 --- a/lib/plausible/stats/clickhouse.ex +++ b/lib/plausible/stats/clickhouse.ex @@ -238,7 +238,7 @@ defmodule Plausible.Stats.Clickhouse do end) end - def top_sources(site, query, limit, page, show_noref \\ false, include \\ []) do + def top_sources(site, query, limit, page, show_noref \\ false, include_details) do offset = (page - 1) * limit referrers = @@ -266,7 +266,7 @@ defmodule Plausible.Stats.Clickhouse do end referrers = - if "bounce_rate" in include do + if include_details do from( s in referrers, select: %{ @@ -441,7 +441,7 @@ defmodule Plausible.Stats.Clickhouse do ) end - def referrer_drilldown(site, query, referrer, include, limit) do + def referrer_drilldown(site, query, referrer, include_details, limit) do referrer = if referrer == @no_ref, do: "", else: referrer q = @@ -455,7 +455,7 @@ defmodule Plausible.Stats.Clickhouse do |> filter_converted_sessions(site, query) q = - if "bounce_rate" in include do + if include_details do from( s in q, select: %{ @@ -585,7 +585,7 @@ defmodule Plausible.Stats.Clickhouse do end end - def top_pages(site, %Query{period: "realtime"} = query, limit, page, _include) do + def top_pages(site, %Query{period: "realtime"} = query, limit, page, _include_details) do offset = (page - 1) * limit q = base_session_query(site, query) |> apply_page_as_entry_page(site, query) @@ -603,7 +603,7 @@ defmodule Plausible.Stats.Clickhouse do ) end - def top_pages(site, query, limit, page, include) do + def top_pages(site, query, limit, page, include_details) do offset = (page - 1) * limit q = @@ -622,9 +622,52 @@ defmodule Plausible.Stats.Clickhouse do pages = ClickhouseRepo.all(q) - if "bounce_rate" in include do - bounce_rates = bounce_rates_by_page_url(site, query) - Enum.map(pages, fn url -> Map.put(url, :bounce_rate, bounce_rates[url[:name]]) end) + if include_details do + [{bounce_state, bounce_result}, {time_state, time_result}] = + Task.yield_many( + [ + Task.async(fn -> bounce_rates_by_page_url(site, query) end), + Task.async(fn -> + {:ok, page_times} = + page_times_by_page_url(site, query, Enum.map(pages, fn p -> p.name end)) + + page_times.rows |> Enum.map(fn [a, b] -> {a, b} end) |> Enum.into(%{}) + end) + ], + 15000 + ) + |> Enum.map(fn {task, response} -> + case response do + nil -> + Task.shutdown(task, :brutal_kill) + {nil, nil} + + {:ok, result} -> + {:ok, result} + + _ -> + response + end + end) + + Enum.map(pages, fn page -> + if bounce_state == :ok, + do: Map.put(page, :bounce_rate, bounce_result[page[:name]]), + else: page + end) + |> Enum.map(fn page -> + if time_state == :ok do + time = time_result[page[:name]] + + Map.put( + page, + :time_on_page, + if(time, do: round(time), else: nil) + ) + else + page + end + end) else pages end @@ -648,6 +691,43 @@ defmodule Plausible.Stats.Clickhouse do |> Enum.into(%{}) end + defp page_times_by_page_url(site, query, page_list) do + q = + from( + e in base_query_w_sessions(site, %Query{ + query + | filters: Map.delete(query.filters, "page") + }), + select: { + fragment("? as p", e.pathname), + fragment("? as t", e.timestamp), + fragment("? as s", e.session_id) + }, + order_by: [e.session_id, e.timestamp] + ) + + {base_query_raw, base_query_raw_params} = ClickhouseRepo.to_sql(:all, q) + + "SELECT + p, + sum(td)/count(case when p2 != p then 1 end) as avgTime + FROM + (SELECT + p, + p2, + sum(t2-t) as td + FROM + (SELECT + *, + neighbor(t, 1) as t2, + neighbor(p, 1) as p2, + neighbor(s, 1) as s2 + FROM (#{base_query_raw})) + WHERE s=s2 AND p IN tuple(?) + GROUP BY p,p2,s) + GROUP BY p" |> ClickhouseRepo.query(base_query_raw_params ++ [page_list ++ ["/"]]) + end + defp add_percentages(stat_list) do total = Enum.reduce(stat_list, 0, fn %{count: count}, total -> total + count end) diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index b8472d7f8..5985e4689 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -140,11 +140,11 @@ defmodule PlausibleWeb.Api.StatsController do def sources(conn, params) do site = conn.assigns[:site] query = Query.from(site.timezone, params) - include = if params["include"], do: String.split(params["include"], ","), else: [] + include_details = params["detailed"] == "true" limit = if params["limit"], do: String.to_integer(params["limit"]) page = if params["page"], do: String.to_integer(params["page"]) show_noref = params["show_noref"] == "true" - json(conn, Stats.top_sources(site, query, limit || 9, page || 1, show_noref, include)) + json(conn, Stats.top_sources(site, query, limit || 9, page || 1, show_noref, include_details)) end def utm_mediums(conn, params) do @@ -203,10 +203,10 @@ defmodule PlausibleWeb.Api.StatsController do def referrer_drilldown(conn, %{"referrer" => referrer} = params) do site = conn.assigns[:site] query = Query.from(site.timezone, params) - include = if params["include"], do: String.split(params["include"], ","), else: [] + include_details = params["detailed"] == "true" limit = params["limit"] || 9 - referrers = Stats.referrer_drilldown(site, query, referrer, include, limit) + referrers = Stats.referrer_drilldown(site, query, referrer, include_details, limit) {_, total_visitors} = Stats.pageviews_and_visitors(site, query) json(conn, %{referrers: referrers, total_visitors: total_visitors}) end @@ -223,11 +223,11 @@ defmodule PlausibleWeb.Api.StatsController do def pages(conn, params) do site = conn.assigns[:site] query = Query.from(site.timezone, params) - include = if params["include"], do: String.split(params["include"], ","), else: [] + include_details = params["detailed"] == "true" limit = if params["limit"], do: String.to_integer(params["limit"]) page = if params["page"], do: String.to_integer(params["page"]) - json(conn, Stats.top_pages(site, query, limit || 9, page || 1, include)) + json(conn, Stats.top_pages(site, query, limit || 9, page || 1, include_details)) end def entry_pages(conn, params) do diff --git a/test/plausible_web/controllers/api/stats_controller/pages_test.exs b/test/plausible_web/controllers/api/stats_controller/pages_test.exs index 69794fb00..4c14d16e4 100644 --- a/test/plausible_web/controllers/api/stats_controller/pages_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/pages_test.exs @@ -16,33 +16,37 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do ] end - test "calculates bounce rate for pages", %{conn: conn, site: site} do + test "calculates bounce rate and time on page for pages", %{conn: conn, site: site} do conn = get( conn, - "/api/stats/#{site.domain}/pages?period=day&date=2019-01-01&include=bounce_rate" + "/api/stats/#{site.domain}/pages?period=day&date=2019-01-01&detailed=true" ) assert json_response(conn, 200) == [ %{ + "time_on_page" => 82800, "bounce_rate" => 33.0, "count" => 3, "pageviews" => 3, "name" => "/" }, %{ + "time_on_page" => 1, "bounce_rate" => nil, "count" => 2, "pageviews" => 2, "name" => "/register" }, %{ + "time_on_page" => nil, "bounce_rate" => nil, "count" => 1, "pageviews" => 1, "name" => "/contact" }, %{ + "time_on_page" => nil, "bounce_rate" => nil, "count" => 1, "pageviews" => 1, diff --git a/test/plausible_web/controllers/api/stats_controller/sources_test.exs b/test/plausible_web/controllers/api/stats_controller/sources_test.exs index 3eb6dc0f0..9759ce91c 100644 --- a/test/plausible_web/controllers/api/stats_controller/sources_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/sources_test.exs @@ -18,7 +18,7 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do conn = get( conn, - "/api/stats/#{site.domain}/sources?period=day&date=2019-01-01&include=bounce_rate,visit_duration" + "/api/stats/#{site.domain}/sources?period=day&date=2019-01-01&detailed=true" ) assert json_response(conn, 200) == [ @@ -143,7 +143,7 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do conn, "/api/stats/#{site.domain}/referrers/10words?period=day&date=2019-01-01&filters=#{ filters - }&include=bounce_rate,visit_duration" + }&detailed=true" ) assert json_response(conn, 200) == %{ diff --git a/test/support/clickhouse_setup.ex b/test/support/clickhouse_setup.ex index ff1d4f5f3..ef4fa03e1 100644 --- a/test/support/clickhouse_setup.ex +++ b/test/support/clickhouse_setup.ex @@ -102,7 +102,7 @@ defmodule Plausible.Test.ClickhouseSetup do pathname: "/irrelevant", domain: "test-site.com", session_id: @conversion_1_session_id, - timestamp: ~N[2019-01-01 23:00:00] + timestamp: ~N[2019-01-01 23:00:01] }, %{ name: "pageview",