fix: demote geolocation request logs #29

graham · 2026-09-14T23:38:34-05:00

graham commented

2026-09-14 23:38:34 -05:00

Fixes W13 from bugs.md.

Demotes all four IPGeolocation log lines from :info to :debug. The plug runs on every anonymous browser request and wrote 2-3 lines each — pure per-request noise in production logs.

Verification: focused plug tests passed (93), full suite passed (2235), mix credo --strict, and commit hooks including Dialyzer passed.

Fixes W13 from bugs.md. Demotes all four `IPGeolocation` log lines from `:info` to `:debug`. The plug runs on every anonymous browser request and wrote 2-3 lines each — pure per-request noise in production logs. Verification: focused plug tests passed (93), full suite passed (2235), `mix credo --strict`, and commit hooks including Dialyzer passed.

graham added 1 commit

2026-09-14 23:38:34 -05:00

fix: demote geolocation request logs

Elixir CI / Dialyzer (pull_request) Successful in 37s

Details

skippy-bot/review Skippy review: clean — 1 non-blocking note open

Details

Elixir CI / Build and test (pull_request) Successful in 1m39s

Details

Elixir CI / Build and Push Docker Image (pull_request) Has been skipped

Details

a97b37fb73

skippy-bot reviewed

2026-09-14 23:39:22 -05:00

skippy-bot left a comment

🤖 Skippy PR review

2 findings — none blocking.

Severity	Location	Issue
🔵 Suggestion	`lib/aprsme_web/plugs/ip_geolocation.ex:51`	Untrusted-peer line is the CF-bypass signal, not per-request noise
🔵 Suggestion	`lib/aprsme_web/live/map_live/navigation.ex:26`	Same geolocation logs still at :info one layer down (W13 only half fixed)

Findings outside the commentable diff

🔵 Suggestion — lib/aprsme_web/live/map_live/navigation.ex:26 — Same geolocation logs still at :info one layer down (W13 only half fixed)
MapLive.Navigation.determine_map_location/2 logs the identical thing at :info, and it is called on every map mount: line 26 dumps the session value, then one of line 35 (URL params), 42 (Using IP geolocation: lat=…, lng=…) or 48 fires — so a map page load still writes 2 :info lines per anonymous visitor in prod, including the coordinates. Demoting the plug's lines leaves the noise (and the coords-in-logs) that W13 is about. Same one-line change here: Logger.debug at 26/35/42/48.

_{Reviewed a97b37fb73f6. Comment skippy review to re-run.}

### 🤖 Skippy PR review **2 findings** — none blocking. | Severity | Location | Issue | | --- | --- | --- | | 🔵 Suggestion | `lib/aprsme_web/plugs/ip_geolocation.ex:51` | Untrusted-peer line is the CF-bypass signal, not per-request noise | | 🔵 Suggestion | `lib/aprsme_web/live/map_live/navigation.ex:26` | Same geolocation logs still at :info one layer down (W13 only half fixed) | #### Findings outside the commentable diff - **🔵 Suggestion — `lib/aprsme_web/live/map_live/navigation.ex:26` — Same geolocation logs still at :info one layer down (W13 only half fixed)** `MapLive.Navigation.determine_map_location/2` logs the identical thing at `:info`, and it is called on every map mount: line 26 dumps the session value, then one of line 35 (URL params), 42 (`Using IP geolocation: lat=…, lng=…`) or 48 fires — so a map page load still writes 2 `:info` lines per anonymous visitor in prod, including the coordinates. Demoting the plug's lines leaves the noise (and the coords-in-logs) that W13 is about. Same one-line change here: `Logger.debug` at 26/35/42/48. <sub>Reviewed `a97b37fb73f6`. Comment `skippy review` to re-run.</sub>

lib/aprsme_web/plugs/ip_geolocation.ex

					
				@ -49,3 +49,3 @@

				      end

				    else

				      Logger.info("Ignoring Cloudflare geolocation headers from untrusted peer")

				      Logger.debug("Ignoring Cloudflare geolocation headers from untrusted peer")

🔵 Suggestion — Untrusted-peer line is the CF-bypass signal, not per-request noise

This branch only fires when the raw TCP peer is NOT in TRUSTED_PROXY_CIDRS/the Cloudflare ranges — i.e. something reached the origin without passing through Cloudflare, or the trusted-proxy list regressed (which silently disables session geolocation for every visitor). It is rare by construction, so it isn't part of the 2-3 lines/request you're fixing; at :debug with config :logger, level: :info in prod it becomes invisible. Consider leaving this one line at :info (or :warning) and demoting only lines 34/43/47.

**🔵 Suggestion — Untrusted-peer line is the CF-bypass signal, not per-request noise** This branch only fires when the raw TCP peer is NOT in `TRUSTED_PROXY_CIDRS`/the Cloudflare ranges — i.e. something reached the origin without passing through Cloudflare, or the trusted-proxy list regressed (which silently disables session geolocation for every visitor). It is rare by construction, so it isn't part of the 2-3 lines/request you're fixing; at `:debug` with `config :logger, level: :info` in prod it becomes invisible. Consider leaving this one line at `:info` (or `:warning`) and demoting only lines 34/43/47.

graham added 1 commit

2026-09-15 11:59:00 -05:00

fix: demote remaining map navigation info logs to debug

skippy-bot/review Skippy review: clean — 1 non-blocking note open

Details

Elixir CI / Build and test (pull_request) Successful in 58s

Details

Elixir CI / Dialyzer (pull_request) Successful in 2m52s

Details

Elixir CI / Build and Push Docker Image (pull_request) Has been skipped

Details

d4eb8be2be

skippy-bot commented

2026-09-15 11:59:32 -05:00

First-time contributor

Resolved the second earlier finding: MapLive.Navigation lines 26/35/42/48 are Logger.debug now, so W13's other half is done. That one had no inline thread to close, so 1 of 2 findings is cleared by this push.

Still open (suggestion, non-blocking): lib/aprsme_web/plugs/ip_geolocation.ex:51 is still :debug. Leaving it there looks deliberate, so no blocker from me, just noting the flag stands.

Nothing new in a97b37f..d4eb8be; the push is a pure log-level change.

Resolved the second earlier finding: `MapLive.Navigation` lines 26/35/42/48 are `Logger.debug` now, so W13's other half is done. That one had no inline thread to close, so 1 of 2 findings is cleared by this push. Still open (suggestion, non-blocking): `lib/aprsme_web/plugs/ip_geolocation.ex:51` is still `:debug`. Leaving it there looks deliberate, so no blocker from me, just noting the flag stands. Nothing new in `a97b37f..d4eb8be`; the push is a pure log-level change.

graham merged commit dc551a9040 into main

2026-09-15 16:33:43 -05:00

graham deleted branch fix/w13-geolocation-logs

2026-09-15 16:33:43 -05:00

graham referenced this pull request from a commit

2026-09-15 16:33:44 -05:00

fix: demote geolocation request logs (#29)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project