Skip to content

Commit

Permalink
1259 remove custom telemetry plumbing (#1528)
Browse files Browse the repository at this point in the history
* Remove custom telemetry plumbing

* Removing telemtry makes handle_create superfluous

* Update CHANGELOG
  • Loading branch information
rorymckinley authored Dec 5, 2023
1 parent 2b4ca94 commit 604e4dd
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 117 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ and this project adheres to

- Limit entries count on term work orders search
[#1461](https://github.com/OpenFn/Lightning/issues/1461)
- Remove custom telemetry plumbing.
[1259](https://github.com/OpenFn/Lightning/issues/1259)

### Fixed

Expand Down
19 changes: 1 addition & 18 deletions lib/lightning_web/controllers/webhooks_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,7 @@ defmodule LightningWeb.WebhooksController do
alias Lightning.WorkOrders

@spec create(Plug.Conn.t(), %{path: binary()}) :: Plug.Conn.t()
def create(conn, %{"path" => path}) do
path = Enum.join(path, "/")
start_opts = %{path: path}

:telemetry.span([:lightning, :workorder, :webhook], start_opts, fn ->
{conn, metadata} =
OpenTelemetry.Tracer.with_span "lightning.api.webhook", %{
attributes: start_opts
} do
conn = handle_create(conn)
{conn, %{status: Plug.Conn.Status.reason_atom(conn.status)}}
end

{conn, start_opts |> Map.merge(metadata)}
end)
end

defp handle_create(conn) do
def create(conn, _params) do
case conn.assigns.trigger do
nil ->
conn |> put_status(:not_found) |> json(%{"error" => "Webhook not found"})
Expand Down
23 changes: 5 additions & 18 deletions lib/lightning_web/plugs/webhook_auth.ex
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,12 @@ defmodule LightningWeb.Plugs.WebhookAuth do
def call(conn, _opts) do
case conn.path_info do
["i", webhook] ->
start_opts = %{path: webhook}
trigger =
Workflows.get_webhook_trigger(webhook,
include: [:workflow, :edges]
)

:telemetry.span([:lightning, :workorder, :webhook], start_opts, fn ->
{conn, metadata} =
OpenTelemetry.Tracer.with_span "lightning.api.webhook", %{
attributes: start_opts
} do
trigger =
Workflows.get_webhook_trigger(webhook,
include: [:workflow, :edges]
)

conn = validate_auth(trigger, conn)

{conn, %{status: Plug.Conn.Status.reason_atom(conn.status || 202)}}
end

{conn, start_opts |> Map.merge(metadata)}
end)
validate_auth(trigger, conn)

_ ->
conn
Expand Down
81 changes: 0 additions & 81 deletions test/lightning_web/controllers/webhooks_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -33,71 +33,10 @@ defmodule LightningWeb.WebhooksControllerTest do
assert attempt.starting_trigger_id == trigger.id
end

test "triggers a custom telemetry event", %{conn: conn} do
ref =
:telemetry_test.attach_event_handlers(self(), [
[:lightning, :workorder, :webhook, :stop]
])

%{triggers: [trigger]} =
insert(:simple_workflow) |> Lightning.Repo.preload(:triggers)

trigger_id = trigger.id
message = %{"foo" => "bar"}

post(conn, "/i/#{trigger_id}", message)

assert_received {
[:lightning, :workorder, :webhook, :stop],
^ref,
%{},
%{path: ^trigger_id, status: :ok}
}
end

test "executes a custom OpenTelemetry trace", %{conn: conn} do
:otel_simple_processor.set_exporter(:otel_exporter_pid, self())

%{triggers: [trigger]} =
insert(:simple_workflow) |> Lightning.Repo.preload(:triggers)

trigger_id = trigger.id
message = %{"foo" => "bar"}

attributes =
:otel_attributes.new([path: trigger_id], 128, :infinity)

post(conn, "/i/#{trigger_id}", message)

assert_receive {:span,
span(
name: "lightning.api.webhook",
attributes: ^attributes
)}
end

test "with an invalid trigger id returns a 404", %{conn: conn} do
conn = post(conn, "/i/bar")
assert json_response(conn, 404) == %{"error" => "Webhook not found"}
end

test "with an invalid trigger id - indicates this in the telemetry span", %{
conn: conn
} do
ref =
:telemetry_test.attach_event_handlers(self(), [
[:lightning, :workorder, :webhook, :stop]
])

post(conn, "/i/bar")

assert_received {
[:lightning, :workorder, :webhook, :stop],
^ref,
%{},
%{path: "bar", status: :not_found}
}
end
end

describe "a disabled message" do
Expand All @@ -119,25 +58,5 @@ defmodule LightningWeb.WebhooksControllerTest do
assert response_message =~
"Unable to process request, trigger is disabled."
end

test "adjusts the telemetry span status", %{
conn: conn,
trigger_id: trigger_id,
message: message
} do
ref =
:telemetry_test.attach_event_handlers(self(), [
[:lightning, :workorder, :webhook, :stop]
])

post(conn, "/i/#{trigger_id}", message)

assert_received {
[:lightning, :workorder, :webhook, :stop],
^ref,
%{},
%{path: ^trigger_id, status: :forbidden}
}
end
end
end

0 comments on commit 604e4dd

Please sign in to comment.