From af5cc38f9e5e40d672f5bca2b07908c8d5988f77 Mon Sep 17 00:00:00 2001 From: Andrew Dryga Date: Thu, 30 Nov 2023 11:52:24 -0600 Subject: [PATCH] Pick latest-versioned gateways (#2739) Closes #2733 --- elixir/README.md | 2 +- elixir/apps/api/lib/api/client/channel.ex | 31 ++++++++++++------- elixir/apps/api/lib/api/client/socket.ex | 2 ++ elixir/apps/api/lib/api/gateway/channel.ex | 18 ++++++----- elixir/apps/domain/lib/domain/gateways.ex | 6 +++- .../lib/domain/jobs/executors/global.ex | 3 +- .../apps/domain/test/domain/gateways_test.exs | 28 ++++++++++++++--- 7 files changed, 63 insertions(+), 27 deletions(-) diff --git a/elixir/README.md b/elixir/README.md index 848a37272..21bb44bcd 100644 --- a/elixir/README.md +++ b/elixir/README.md @@ -278,7 +278,7 @@ iex(api@api-b02t.us-east1-d.c.firezone-staging.internal)1> One-liner to connect to a running application container: ```bash -❯ gcloud compute ssh web-w2f6 --tunnel-through-iap -- '$(docker ps | grep klt- | head -n 1 | awk '\''{split($NF, arr, "-"); print "docker exec -it " $NF " bin/" arr[2] " remote";}'\'')' +❯ gcloud compute ssh $(gcloud compute instances list | grep "web-" | tail -n 1 | awk '{ print $1 }') --tunnel-through-iap -- '$(docker ps | grep klt- | head -n 1 | awk '\''{split($NF, arr, "-"); print "docker exec -it " $NF " bin/" arr[2] " remote";}'\'')' Interactive Elixir (1.15.2) - press Ctrl+C to exit (type h() ENTER for help) iex(web@web-w2f6.us-east1-d.c.firezone-staging.internal)1> diff --git a/elixir/apps/api/lib/api/client/channel.ex b/elixir/apps/api/lib/api/client/channel.ex index bed53bcba..a0126b26a 100644 --- a/elixir/apps/api/lib/api/client/channel.ex +++ b/elixir/apps/api/lib/api/client/channel.ex @@ -82,10 +82,11 @@ defmodule API.Client.Channel do OpenTelemetry.Ctx.attach(opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "client.ice_candidates", %{ - gateway_id: gateway_id, - candidates_length: length(candidates) - } do + OpenTelemetry.Tracer.with_span "client.ice_candidates", + attributes: %{ + gateway_id: gateway_id, + candidates_length: length(candidates) + } do push(socket, "ice_candidates", %{ gateway_id: gateway_id, candidates: candidates @@ -105,7 +106,7 @@ defmodule API.Client.Channel do OpenTelemetry.Ctx.attach(opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "client.connect", %{resource_id: resource_id} do + OpenTelemetry.Tracer.with_span "client.connect", attributes: %{resource_id: resource_id} do reply( socket_ref, {:ok, @@ -125,7 +126,8 @@ defmodule API.Client.Channel do OpenTelemetry.Ctx.attach(socket.assigns.opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(socket.assigns.opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "client.resource_added", %{resource_id: resource_id} do + OpenTelemetry.Tracer.with_span "client.resource_added", + attributes: %{resource_id: resource_id} do with {:ok, resource} <- Resources.fetch_resource_by_id(resource_id, socket.assigns.subject) do push(socket, "resource_added", Views.Resource.render(resource)) end @@ -138,7 +140,8 @@ defmodule API.Client.Channel do OpenTelemetry.Ctx.attach(socket.assigns.opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(socket.assigns.opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "client.resource_updated", %{resource_id: resource_id} do + OpenTelemetry.Tracer.with_span "client.resource_updated", + attributes: %{resource_id: resource_id} do with {:ok, resource} <- Resources.fetch_resource_by_id(resource_id, socket.assigns.subject) do push(socket, "resource_updated", Views.Resource.render(resource)) end @@ -151,7 +154,8 @@ defmodule API.Client.Channel do OpenTelemetry.Ctx.attach(socket.assigns.opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(socket.assigns.opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "client.resource_removed", %{resource_id: resource_id} do + OpenTelemetry.Tracer.with_span "client.resource_removed", + attributes: %{resource_id: resource_id} do push(socket, "resource_removed", resource_id) {:noreply, socket} end @@ -174,7 +178,7 @@ defmodule API.Client.Channel do OpenTelemetry.Ctx.attach(socket.assigns.opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(socket.assigns.opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "client.prepare_connection", attrs do + OpenTelemetry.Tracer.with_span "client.prepare_connection", attributes: attrs do connected_gateway_ids = Map.get(attrs, "connected_gateway_ids", []) with {:ok, resource} <- @@ -191,6 +195,11 @@ defmodule API.Client.Channel do socket.assigns.client.last_seen_remote_ip_location_lon } + OpenTelemetry.Tracer.set_attribute(:relays_length, length(relays)) + OpenTelemetry.Tracer.set_attribute(:gateways_length, length(gateways)) + OpenTelemetry.Tracer.set_attribute(:relay_hosting_type, relay_hosting_type) + OpenTelemetry.Tracer.set_attribute(:relay_connection_type, relay_connection_type) + relays = Relays.load_balance_relays(location, relays) gateway = Gateways.load_balance_gateways(location, gateways, connected_gateway_ids) @@ -234,7 +243,7 @@ defmodule API.Client.Channel do OpenTelemetry.Ctx.attach(socket.assigns.opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(socket.assigns.opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "client.reuse_connection", attrs do + OpenTelemetry.Tracer.with_span "client.reuse_connection", attributes: attrs do with {:ok, gateway} <- Gateways.fetch_gateway_by_id(gateway_id, socket.assigns.subject), {:ok, resource, flow} <- Flows.authorize_flow( @@ -287,7 +296,7 @@ defmodule API.Client.Channel do OpenTelemetry.Ctx.attach(socket.assigns.opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(socket.assigns.opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "client.request_connection", ctx_attrs do + OpenTelemetry.Tracer.with_span "client.request_connection", attributes: ctx_attrs do with {:ok, gateway} <- Gateways.fetch_gateway_by_id(gateway_id, socket.assigns.subject), {:ok, resource, flow} <- Flows.authorize_flow( diff --git a/elixir/apps/api/lib/api/client/socket.ex b/elixir/apps/api/lib/api/client/socket.ex index 93954ef1f..59489c461 100644 --- a/elixir/apps/api/lib/api/client/socket.ex +++ b/elixir/apps/api/lib/api/client/socket.ex @@ -39,6 +39,8 @@ defmodule API.Client.Socket do {:ok, client} <- Clients.upsert_client(attrs, subject) do OpenTelemetry.Tracer.set_attributes(%{ client_id: client.id, + lat: client.last_seen_remote_ip_location_lat, + lon: client.last_seen_remote_ip_location_lon, account_id: subject.account.id }) diff --git a/elixir/apps/api/lib/api/gateway/channel.ex b/elixir/apps/api/lib/api/gateway/channel.ex index ccaee4ac9..47f77d809 100644 --- a/elixir/apps/api/lib/api/gateway/channel.ex +++ b/elixir/apps/api/lib/api/gateway/channel.ex @@ -62,10 +62,11 @@ defmodule API.Gateway.Channel do OpenTelemetry.Ctx.attach(opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "gateway.ice_candidates", %{ - client_id: client_id, - candidates_length: length(candidates) - } do + OpenTelemetry.Tracer.with_span "gateway.ice_candidates", + attributes: %{ + client_id: client_id, + candidates_length: length(candidates) + } do push(socket, "ice_candidates", %{ client_id: client_id, candidates: candidates @@ -104,10 +105,11 @@ defmodule API.Gateway.Channel do OpenTelemetry.Ctx.attach(socket.assigns.opentelemetry_ctx) OpenTelemetry.Tracer.set_current_span(socket.assigns.opentelemetry_span_ctx) - OpenTelemetry.Tracer.with_span "gateway.reject_access", %{ - client_id: client_id, - resource_id: resource_id - } do + OpenTelemetry.Tracer.with_span "gateway.reject_access", + attributes: %{ + client_id: client_id, + resource_id: resource_id + } do push(socket, "reject_access", %{ client_id: client_id, resource_id: resource_id diff --git a/elixir/apps/domain/lib/domain/gateways.ex b/elixir/apps/domain/lib/domain/gateways.ex index b2a2cafe8..617b85c3d 100644 --- a/elixir/apps/domain/lib/domain/gateways.ex +++ b/elixir/apps/domain/lib/domain/gateways.ex @@ -417,7 +417,7 @@ defmodule Domain.Gateways do {gateway.last_seen_remote_ip_location_lat, gateway.last_seen_remote_ip_location_lon} end) |> Enum.map(fn - {{nil, nil}, gateway} -> + {{gateway_lat, gateway_lon}, gateway} when is_nil(gateway_lat) or is_nil(gateway_lon) -> {Geo.fetch_radius_of_earth_km!(), gateway} {{gateway_lat, gateway_lon}, gateway} -> @@ -427,6 +427,10 @@ defmodule Domain.Gateways do |> Enum.sort_by(&elem(&1, 0)) |> Enum.at(0) |> elem(1) + |> Enum.group_by(fn gateway -> gateway.last_seen_version end) + |> Enum.sort_by(&elem(&1, 0), :desc) + |> Enum.at(0) + |> elem(1) |> Enum.random() end diff --git a/elixir/apps/domain/lib/domain/jobs/executors/global.ex b/elixir/apps/domain/lib/domain/jobs/executors/global.ex index b50571a10..4052392a8 100644 --- a/elixir/apps/domain/lib/domain/jobs/executors/global.ex +++ b/elixir/apps/domain/lib/domain/jobs/executors/global.ex @@ -167,8 +167,7 @@ defmodule Domain.Jobs.Executors.Global do Logger.metadata(attributes) - OpenTelemetry.Tracer.with_span job_callback do - OpenTelemetry.Tracer.set_attributes(attributes) + OpenTelemetry.Tracer.with_span job_callback, attributes: attributes do _ = apply(module, function, [config]) end diff --git a/elixir/apps/domain/test/domain/gateways_test.exs b/elixir/apps/domain/test/domain/gateways_test.exs index 89c57205c..71bc9f3ac 100644 --- a/elixir/apps/domain/test/domain/gateways_test.exs +++ b/elixir/apps/domain/test/domain/gateways_test.exs @@ -889,6 +889,22 @@ defmodule Domain.GatewaysTest do assert gateway.id == gateway_1.id end + test "prioritizes gateways of more recent version" do + gateway_1 = + Fixtures.Gateways.create_gateway(last_seen_user_agent: "iOS/12.7 (iPhone) connlib/1.99") + + gateway_2 = + Fixtures.Gateways.create_gateway(last_seen_user_agent: "iOS/12.7 (iPhone) connlib/2.3") + + gateways = [ + gateway_1, + gateway_2 + ] + + assert gateway = load_balance_gateways({32.2029, -80.0131}, gateways) + assert gateway.id == gateway_2.id + end + test "returns gateways in two closest regions to a given location" do # Moncks Corner, South Carolina gateway_us_east_1 = @@ -939,10 +955,14 @@ defmodule Domain.GatewaysTest do ] # multiple attempts are used to increase chances that all gateways in a group are randomly selected - for _ <- 0..3 do - assert gateway = load_balance_gateways({32.2029, -80.0131}, gateways) - assert gateway.id in [gateway_us_east_1.id, gateway_us_east_2.id, gateway_us_east_3.id] - end + selected = + for _ <- 0..12 do + assert gateway = load_balance_gateways({32.2029, -80.0131}, gateways) + assert gateway.id in [gateway_us_east_1.id, gateway_us_east_2.id, gateway_us_east_3.id] + gateway.id + end + + assert selected |> Enum.uniq() |> length() >= 2 for _ <- 0..2 do assert gateway = load_balance_gateways({45.5946, -121.1787}, gateways)