From 9c15d34c8adc006427d558fa622095627037eae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kan=20Nyl=C3=A9n?= Date: Sat, 4 Jul 2020 11:39:11 +0200 Subject: [PATCH] fixed bounce rate to calculate right (#71) * fixed bounce rate to calculate right * fixed so you can set mark if needed for tests * fixed tests --- .../20200704085318_fixed_sessions_mark.cr | 19 +++++++++++ spec/requests/api/events/create_spec.cr | 3 ++ spec/services/event_handler_spec.cr | 3 ++ spec/services/metrics_spec.cr | 33 +++++++++++-------- spec/services/timer_worker_spec.cr | 3 ++ src/services/add_clickhouse.cr | 5 +-- src/services/event_handler.cr | 4 +-- src/services/metrics_new.cr | 2 +- tasks/add_clickhouse.cr | 5 +-- 9 files changed, 57 insertions(+), 20 deletions(-) create mode 100644 db/migrations/20200704085318_fixed_sessions_mark.cr diff --git a/db/migrations/20200704085318_fixed_sessions_mark.cr b/db/migrations/20200704085318_fixed_sessions_mark.cr new file mode 100644 index 0000000..dd5e8ee --- /dev/null +++ b/db/migrations/20200704085318_fixed_sessions_mark.cr @@ -0,0 +1,19 @@ +class FixedSessionsMark::V20200704085318 < Avram::Migrator::Migration::V1 + def migrate + client = Clickhouse.new(host: ENV["CLICKHOUSE_HOST"]?.try(&.strip), port: 8123) + + sql = <<-SQL + ALTER TABLE kindmetrics.sessions ADD COLUMN mark UInt8 + SQL + client.execute sql + + sql = <<-SQL + ALTER TABLE kindmetrics.sessions UPDATE mark=1 WHERE length IS NOT NULL + SQL + client.execute sql + end + + def rollback + # drop table_for(Thing) + end +end diff --git a/spec/requests/api/events/create_spec.cr b/spec/requests/api/events/create_spec.cr index 6f740c8..4da03e7 100644 --- a/spec/requests/api/events/create_spec.cr +++ b/spec/requests/api/events/create_spec.cr @@ -4,6 +4,9 @@ describe Events::Create do before_each do AddClickhouse.clean_database end + after_each do + AddClickhouse.clean_database + end it "domain not set" do response = AppClient.exec(Events::Create, test: "") response.status_code.should eq(404) diff --git a/spec/services/event_handler_spec.cr b/spec/services/event_handler_spec.cr index 2dbdc14..0919a99 100644 --- a/spec/services/event_handler_spec.cr +++ b/spec/services/event_handler_spec.cr @@ -1,6 +1,9 @@ require "../spec_helper" describe EventHandler do + before_each do + AddClickhouse.clean_database + end after_each do AddClickhouse.clean_database end diff --git a/spec/services/metrics_spec.cr b/spec/services/metrics_spec.cr index 69f224e..2d90aab 100644 --- a/spec/services/metrics_spec.cr +++ b/spec/services/metrics_spec.cr @@ -4,6 +4,9 @@ describe Metrics do before_each do AddClickhouse.clean_database end + after_each do + AddClickhouse.clean_database + end it "unique calculation" do domain = DomainBox.create @@ -31,25 +34,29 @@ describe Metrics do it "bounce calculation" do domain = DomainBox.create - EventHandler.create_session(user_id: "53443534", name: "pageview", referrer: "https://indiehackers.com/amazing", referrer_domain: "indiehackers.com", url: "https://test.com/test/rrr", path: "/test/rrr", referrer_source: nil, device: "Android", browser_name: "Chrome", operative_system: "Android", country: "SE", length: 0, is_bounce: 1, domain_id: domain.id) - EventHandler.create_session(user_id: "2423432", name: "pageview", referrer: "https://indiehackers.com/amazing", referrer_domain: "indiehackers.com", url: "https://test.com/test/rrr", path: "/test/rrr", referrer_source: nil, device: "Android", browser_name: "Chrome", operative_system: "Android", country: "SE", length: 0, is_bounce: 1, domain_id: domain.id) + EventHandler.create_session(user_id: "53443534", name: "pageview", referrer: "https://indiehackers.com/amazing", referrer_domain: "indiehackers.com", url: "https://test.com/test/rrr", path: "/test/rrr", referrer_source: nil, device: "Android", browser_name: "Chrome", operative_system: "Android", country: "SE", length: 0, is_bounce: 1, mark: 1, domain_id: domain.id) + EventHandler.create_session(user_id: "2423432", name: "pageview", referrer: "https://indiehackers.com/amazing", referrer_domain: "indiehackers.com", url: "https://test.com/test/rrr", path: "/test/rrr", referrer_source: nil, device: "Android", browser_name: "Chrome", operative_system: "Android", country: "SE", length: 0, is_bounce: 1, mark: 1, domain_id: domain.id) metrics = Metrics.new(domain, "7d") bounce_rate = metrics.bounce_query bounce_rate.should eq(100) end - # it "bounce with 50/50 calculation" do - # domain = DomainBox.create - # - # EventHandler.create_session(user_id: "1573435124370987", name: "pageview", referrer: "https://indiehackers.com/amazing", referrer_domain: "indiehackers.com", url: "https://test.com/test/rrr", path: "/test/rrr", referrer_source: nil, device: "Android", browser_name: "Chrome", operative_system: "Android", country: "SE", length: 0, is_bounce: 1, domain_id: domain.id) - # EventHandler.create_session(user_id: "12441241565512", name: "pageview", referrer: "https://indiehackers.com/amazing", referrer_domain: "indiehackers.com", url: "https://test.com/test/rrr", path: "/test/rrr", referrer_source: nil, device: "Android", browser_name: "Chrome", operative_system: "Android", country: "SE", length: 234, is_bounce: 0, domain_id: domain.id) - # - # metrics = Metrics.new(domain, "7d") - # bounce_rate = metrics.bounce_query - # # It push down the bounce_rate, that's why it is 30 and not 50. - # bounce_rate.should eq(30) - # end + it "bounce with 50/50 calculation" do + domain = DomainBox.create + + EventHandler.create_session(user_id: "1573435124370987", name: "pageview", referrer: "https://indiehackers.com/amazing", referrer_domain: "indiehackers.com", url: "https://test.com/test/rrr", path: "/test/rrr", referrer_source: nil, device: "Android", browser_name: "Chrome", operative_system: "Android", country: "SE", length: 0, is_bounce: 1, mark: 1, domain_id: domain.id, created_at: 1.minutes.ago) + EventHandler.create_session(user_id: "12441241565512", name: "pageview", referrer: "https://indiehackers.com/amazing", referrer_domain: "indiehackers.com", url: "https://test.com/test/rrr", path: "/test/rrr", referrer_source: nil, device: "Android", browser_name: "Chrome", operative_system: "Android", country: "SE", length: 234, is_bounce: 0, mark: 1, domain_id: domain.id, created_at: 3.minutes.ago) + + sessions = AddClickhouse.get_domain_sessions(domain.id) + events = AddClickhouse.get_domain_events(domain.id) + sessions.size.should eq(2) + events.size.should eq(2) + + metrics = Metrics.new(domain, "7d") + bounce_rate = metrics.bounce_query + bounce_rate.should eq(50) + end it "7 days calculation" do domain = DomainBox.create diff --git a/spec/services/timer_worker_spec.cr b/spec/services/timer_worker_spec.cr index 92a8dc8..2fd7a58 100644 --- a/spec/services/timer_worker_spec.cr +++ b/spec/services/timer_worker_spec.cr @@ -4,6 +4,9 @@ describe TimeWorker do before_each do AddClickhouse.clean_database end + after_each do + AddClickhouse.clean_database + end it "no events attached to session" do user_id = "evwesafsafas" AddClickhouse.session_insert(user_id: user_id, length: nil, is_bounce: 1, referrer: "indiehacker.com", url: "https://kindmetrics.com/aaadsad", referrer_source: "indiehacker.com", path: "/asadasd", device: "Desktop", operative_system: "Mac OS", referrer_domain: "indiehacker.com", browser_name: "Chrome", country: "SE", domain_id: DomainBox.create.id) diff --git a/src/services/add_clickhouse.cr b/src/services/add_clickhouse.cr index 79f444d..c8f3f7d 100644 --- a/src/services/add_clickhouse.cr +++ b/src/services/add_clickhouse.cr @@ -28,13 +28,14 @@ class AddClickhouse client.insert buf end - def self.session_insert(user_id, length : Int64?, is_bounce : Int32, referrer, url, referrer_source, path, device, operative_system, referrer_domain, browser_name, country, domain_id, created_at : Time = Time.utc) + def self.session_insert(user_id, length : Int64?, is_bounce : Int32, referrer, url, referrer_source, path, device, operative_system, referrer_domain, browser_name, country, domain_id, created_at : Time = Time.utc, mark : Int8 = 0) client = Clickhouse.new(host: ENV["CLICKHOUSE_HOST"]?.try(&.strip), port: 8123) id = Random.new.rand(0.to_i64..Int64::MAX) json_object = { id: id, + mark: mark, user_id: user_id, length: length, is_bounce: is_bounce, @@ -116,7 +117,7 @@ class AddClickhouse def self.update_session(session_id : Int64, length : Int64, is_bounce : Int32) client = Clickhouse.new(host: ENV["CLICKHOUSE_HOST"]?.try(&.strip), port: 8123) sql = <<-SQL - ALTER TABLE kindmetrics.sessions UPDATE length=#{length}, is_bounce=#{is_bounce} WHERE id=#{session_id} + ALTER TABLE kindmetrics.sessions UPDATE length=#{length}, is_bounce=#{is_bounce}, mark=1 WHERE id=#{session_id} SQL client.execute sql end diff --git a/src/services/event_handler.cr b/src/services/event_handler.cr index e3914c2..06cf994 100644 --- a/src/services/event_handler.cr +++ b/src/services/event_handler.cr @@ -88,8 +88,8 @@ class EventHandler end end - def self.create_session(user_id : String, length : Int64?, name : String, is_bounce : Int32, referrer : String?, url : String?, referrer_source : String?, path : String?, device : String?, operative_system : String?, referrer_domain : String?, browser_name : String?, country : String?, domain_id : Int64, created_at : Time = Time.utc) - AddClickhouse.session_insert(user_id, length, is_bounce, referrer, url, referrer_source, path, device, operative_system, referrer_domain, browser_name, country, domain_id, created_at.to_utc) + def self.create_session(user_id : String, length : Int64?, name : String, is_bounce : Int32, referrer : String?, url : String?, referrer_source : String?, path : String?, device : String?, operative_system : String?, referrer_domain : String?, browser_name : String?, country : String?, domain_id : Int64, created_at : Time = Time.utc, mark : Int8 = 0) + AddClickhouse.session_insert(user_id, length, is_bounce, referrer, url, referrer_source, path, device, operative_system, referrer_domain, browser_name, country, domain_id, created_at.to_utc, mark: mark) session = get_session(user_id) AddClickhouse.event_insert(user_id, name, referrer, url, referrer_source, path, device, operative_system, referrer_domain, browser_name, country, domain_id, session_id: session.not_nil!.id, created_at: created_at.to_utc) end diff --git a/src/services/metrics_new.cr b/src/services/metrics_new.cr index ffbde50..1286d10 100644 --- a/src/services/metrics_new.cr +++ b/src/services/metrics_new.cr @@ -73,7 +73,7 @@ class MetricsNew def bounce_query : Int64 sql = <<-SQL - SELECT round(sum(is_bounce * id) / sum(id) * 100) as bounce_rate + SELECT round(sum(is_bounce * mark) / sum(mark) * 100) as bounce_rate FROM kindmetrics.sessions WHERE domain_id=#{@domain.id} AND created_at > '#{slim_from_date}' AND created_at < '#{slim_to_date}' SQL res = @client.execute(sql) diff --git a/tasks/add_clickhouse.cr b/tasks/add_clickhouse.cr index 536a92f..0aa724e 100644 --- a/tasks/add_clickhouse.cr +++ b/tasks/add_clickhouse.cr @@ -25,7 +25,6 @@ class AddClickhouseTable < LuckyCli::Task `id` UInt64, `user_id` String, `name` String, - `referrer` String, `domain` String, `url` String, `referrer_source` Nullable(String), @@ -33,6 +32,7 @@ class AddClickhouseTable < LuckyCli::Task `device` Nullable(String), `operative_system` Nullable(String), `referrer_domain` Nullable(String), + `referrer` Nullable(String), `browser_name` Nullable(String), `country` Nullable(String), `domain_id` UInt64, @@ -48,8 +48,8 @@ class AddClickhouseTable < LuckyCli::Task buf = <<-SQL CREATE TABLE IF NOT EXISTS kindmetrics.sessions ( `id` UInt64, + `mark` UInt8, `user_id` String, - `referrer` String, `domain` String, `url` String, `referrer_source` Nullable(String), @@ -57,6 +57,7 @@ class AddClickhouseTable < LuckyCli::Task `device` Nullable(String), `operative_system` Nullable(String), `referrer_domain` Nullable(String), + `referrer` Nullable(String), `browser_name` Nullable(String), `country` Nullable(String), `domain_id` UInt64,