Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.
### Fixed
- Fixed weekly/monthly e-mail report [rendering issues](https://github.com/plausible/analytics/issues/284)
- Fixed [long URLs display](https://github.com/plausible/analytics/issues/3158) in Outbound Link breakdown view
- Fixed [Sentry reports](https://github.com/plausible/analytics/discussions/3166) for ingestion requests plausible/analytics#3182

## v2.0.0 - 2023-07-12

Expand Down
14 changes: 14 additions & 0 deletions lib/plausible/ingestion/request.ex
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ defmodule Plausible.Ingestion.Request do

@max_url_size 2_000

@primary_key false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise {"id": null} is added to all requests during encoding.

embedded_schema do
field :remote_ip, :string
field :user_agent, :string
Expand Down Expand Up @@ -312,3 +313,16 @@ defmodule Plausible.Ingestion.Request do
nil
end
end

defimpl Jason.Encoder, for: URI do
def encode(uri, _opts), do: [?", URI.to_string(uri), ?"]
end

defimpl Jason.Encoder, for: Plausible.Ingestion.Request do
@fields Plausible.Ingestion.Request.__schema__(:fields)
def encode(request, opts) do
request
|> Map.take(@fields)
|> Jason.Encode.map(opts)
end
end
2 changes: 1 addition & 1 deletion lib/plausible_web/controllers/api/external_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ defmodule PlausibleWeb.Api.ExternalController do

def event(conn, _params) do
with {:ok, request} <- Ingestion.Request.build(conn),
_ <- Sentry.Context.set_extra_context(%{request: inspect(request)}) do
_ <- Sentry.Context.set_extra_context(%{request: request}) do
case Ingestion.Event.build_and_buffer(request) do
{:ok, %{dropped: [], buffered: _buffered}} ->
conn
Expand Down
4 changes: 4 additions & 0 deletions lib/sentry_filter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ defmodule Plausible.SentryFilter do
%{event | fingerprint: ["db_connection", reason]}
end

def before_send(%{extra: %{request: %Plausible.Ingestion.Request{}}} = event) do
%{event | fingerprint: ["ingestion_request"]}
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll need to try that out...

Copy link
Contributor Author

@ruslandoga ruslandoga Jul 24, 2023

Choose a reason for hiding this comment

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

Sentry.capture_message("oof", extra: %{request: %Plausible.Ingestion.Request{}})

https://eh-uo.sentry.io/share/issue/1cb9ceadb737491f8f230c1ec391e0bf/

🥲 "shared" issue doesn't include fingerprint info...

Here's what I'm seeing on my dashboard:

Screenshot 2023-07-24 at 18 38 51


def before_send(event) do
event
end
Expand Down
39 changes: 39 additions & 0 deletions test/plausible/ingestion/request_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -422,4 +422,43 @@ defmodule Plausible.Ingestion.RequestTest do
assert {:error, changeset} = Request.build(conn)
assert changeset.errors[:request]
end

test "encodable" do
params = %{
name: "pageview",
domain: "dummy.site",
url: "https://dummy.site/pictures/index.html?foo=bar&baz=bam",
referrer: "https://example.com",
props: %{"abc" => "qwerty", "hello" => "world"},
hashMode: 1,
revenue: %{
"amount" => "12.3",
"currency" => "USD"
}
}

assert {:ok, request} =
build_conn(:post, "/api/events", params)
|> put_req_header("user-agent", "Mozilla")
|> Request.build()

request = request |> Jason.encode!() |> Jason.decode!()

assert Map.drop(request, ["timestamp"]) == %{
"domains" => ["dummy.site"],
"event_name" => "pageview",
"hash_mode" => 1,
"hostname" => "dummy.site",
"pathname" => "/pictures/index.html",
"props" => %{"abc" => "qwerty", "hello" => "world"},
"query_params" => %{"baz" => "bam", "foo" => "bar"},
"referrer" => "https://example.com",
"remote_ip" => "127.0.0.1",
"revenue_source" => %{"amount" => "12.3", "currency" => "USD"},
"uri" => "https://dummy.site/pictures/index.html?foo=bar&baz=bam",
"user_agent" => "Mozilla"
}

assert %NaiveDateTime{} = NaiveDateTime.from_iso8601!(request["timestamp"])
end
end
15 changes: 15 additions & 0 deletions test/plausible/sentry_filter_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
defmodule Plausible.SentryFilterTest do
use ExUnit.Case

test "enforces fingerprint for ingestion request" do
event =
Sentry.Event.create_event(
message: "oof",
Copy link
Member

Choose a reason for hiding this comment

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

beautiful :)

fingerprint: ["to be", " replaced"],
extra: %{request: %Plausible.Ingestion.Request{}}
)

assert %Sentry.Event{fingerprint: ["ingestion_request"]} =
Plausible.SentryFilter.before_send(event)
end
end