Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PoC: keep log_comment metadata in ClickHouse queries #3702

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

PoC: keep log_comment metadata in ClickHouse queries #3702

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4d8663
PoC: keep log_comment metadata in ClickHouse queries
aerosol Jan 18, 2024
1567c55
Store flat keys, without the root "log_comment" element
aerosol Jan 18, 2024
8582fc0
Carry log_comment context into parallel queries
aerosol Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 45 additions & 0 deletions lib/plausible/clickhouse_repo.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,51 @@ defmodule Plausible.ClickhouseRepo do
end
end

defmodule KeepRouteContext do
alias Plausible.ClickhouseRepo

def init(_opts), do: nil

def call(conn, _) do
ClickhouseRepo.set_context(%{request_path: conn.request_path})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on what's more suitable, we can either go with route with placeholders, or full request path

conn
end
end

@logger_metadata_key __MODULE__
def set_context(new) when is_map(new) do
metadata =
case :logger.get_process_metadata() do
%{@logger_metadata_key => ctx} -> Map.update(ctx, :log_comment, new, &Map.merge(&1, new))
_ -> %{:log_comment => new}
end

:logger.update_process_metadata(%{@logger_metadata_key => metadata})
end

def get_context() do
case :logger.get_process_metadata() do
%{@logger_metadata_key => ctx} ->
ctx

%{} ->
%{}

:undefined ->
%{}
end
end

@impl true
def prepare_query(_operation, query, opts) do
setting = {:log_comment, Jason.encode!(get_context())}

opts =
Keyword.update(opts, :settings, [setting], fn settings -> [setting | settings] end)

{query, opts}
end

@task_timeout 60_000
def parallel_tasks(queries) do
ctx = OpenTelemetry.Ctx.get_current()
Expand Down
6 changes: 6 additions & 0 deletions lib/plausible/stats.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,38 @@ defmodule Plausible.Stats do

def breakdown(site, query, prop, metrics, pagination) do
include_sentry_replay_info()
Plausible.ClickhouseRepo.set_context(%{query_type: :breakdown})
Breakdown.breakdown(site, query, prop, metrics, pagination)
end

def aggregate(site, query, metrics) do
Plausible.ClickhouseRepo.set_context(%{query_type: :aggregate})
include_sentry_replay_info()
Aggregate.aggregate(site, query, metrics)
end

def timeseries(site, query, metrics) do
Plausible.ClickhouseRepo.set_context(%{query_type: :timeseries})
include_sentry_replay_info()
Timeseries.timeseries(site, query, metrics)
end

def current_visitors(site) do
Plausible.ClickhouseRepo.set_context(%{query_type: :current_visitors})
include_sentry_replay_info()
CurrentVisitors.current_visitors(site)
end

on_full_build do
def funnel(site, query, funnel) do
Plausible.ClickhouseRepo.set_context(%{query_type: :funnel})
include_sentry_replay_info()
Plausible.Stats.Funnel.funnel(site, query, funnel)
end
end

def filter_suggestions(site, query, filter_name, filter_search) do
Plausible.ClickhouseRepo.set_context(%{query_type: :filter_suggestions})
include_sentry_replay_info()
FilterSuggestions.filter_suggestions(site, query, filter_name, filter_search)
end
Expand Down
2 changes: 2 additions & 0 deletions lib/plausible/stats/query.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ defmodule Plausible.Stats.Query do
@type t :: %__MODULE__{}

def from(site, params) do
Plausible.ClickhouseRepo.set_context(params)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a great idea perhaps, since params is external and at this point, untrusted, input.

We could probably sanitize it a bit, or serialize the result of Map.from_struct(query) but there's a bit more work required, depending on what is really useful to keep - just flagging it.

Similarly, there's more places when the %Query{} is updated - some changes are made via this module, but others operate on the struct directly, so there's probably a good chunk of sensible info missing unless we keep track of it too. Maybe a good opportunity to make sure the struct is opaque and is never changed outside of its API.


query =
query_by_period(site, params)
|> put_interval(params)
Expand Down
4 changes: 4 additions & 0 deletions lib/plausible_web/router.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ defmodule PlausibleWeb.Router do
plug PlausibleWeb.SessionTimeoutPlug, timeout_after_seconds: @two_weeks_in_seconds
plug PlausibleWeb.AuthPlug
plug PlausibleWeb.LastSeenPlug
plug Plausible.ClickhouseRepo.KeepRouteContext
end

pipeline :shared_link do
Expand All @@ -38,17 +39,20 @@ defmodule PlausibleWeb.Router do
plug :accepts, ["json"]
plug :fetch_session
plug PlausibleWeb.AuthPlug
plug Plausible.ClickhouseRepo.KeepRouteContext
end

pipeline :internal_stats_api do
plug :accepts, ["json"]
plug :fetch_session
plug PlausibleWeb.AuthorizeSiteAccess
plug PlausibleWeb.Plugs.NoRobots
plug Plausible.ClickhouseRepo.KeepRouteContext
end

pipeline :public_api do
plug :accepts, ["json"]
plug Plausible.ClickhouseRepo.KeepRouteContext
end

on_full_build do
Expand Down