Skip to content

fix(basket): correct geo resolution telemetry + drop redis geoip cache #589

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

Merged
izadoesdev merged 2 commits into from
Aug 4, 2026
+12 −26
Merged

fix(basket): correct geo resolution telemetry + drop redis geoip cache #589

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9230dc0
fix(basket): log geo resolution source on maxmind and unresolved paths
izadoesdev Aug 4, 2026
1aa2b32
perf(basket): drop redis geoip cache in favor of in-process maxmind l…
izadoesdev Aug 4, 2026
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
38 changes: 12 additions & 26 deletions apps/basket/src/utils/ip-geo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { createHash } from "node:crypto";
import { cacheable } from "@databuddy/redis/cacheable";
import { captureError, mergeWideEvent, record } from "@lib/tracing";
import type { City } from "@maxmind/geoip2-node";
import {
Expand Down Expand Up @@ -169,30 +168,6 @@ function lookupGeoLocation(ip: string): Promise<{
});
}

function coarsenIpForCache(ip: string): string {
if (ip.includes(":")) {
const groups = ip.split(":");
const head = groups.slice(0, 4).join(":");
return `${head}::`;
}
const octets = ip.split(".");
if (octets.length === 4) {
return `${octets[0]}.${octets[1]}.${octets[2]}.0`;
}
return ip;
}

const getCachedGeoLocation = cacheable(lookupGeoLocation, {
expireInSec: 86_400 * 7,
prefix: "geoip_location",
staleWhileRevalidate: true,
staleTime: 86_400,
});

function getGeoLocation(ip: string) {
return getCachedGeoLocation(coarsenIpForCache(ip));
}

export function anonymizeIp(ip: string): string {
if (!ip) {
return "";
Expand All @@ -218,7 +193,7 @@ export function getGeo(ip: string, request?: Request) {
};
}

const geo = await getGeoLocation(ip);
const geo = await lookupGeoLocation(ip);

if (!geo.country && request?.headers) {
const cfCountry = getCloudflareCountry(request.headers);
Expand All @@ -238,6 +213,17 @@ export function getGeo(ip: string, request?: Request) {
}
}

mergeWideEvent({
geo: geo.country
? {
source: "maxmind",
country: geo.country,
region: geo.region,
city: geo.city,
}
: { source: "unresolved" },
});
Comment on lines +216 to +225

@greptile-apps greptile-apps Bot Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Telemetry expands precise geo data

The new MaxMind merge sends user-derived city and region data to operational telemetry even for callers that never persist an analytics event. These fields are unnecessary for measuring resolution source and broaden the retention and audience exposure of precise location data.

Suggested change
mergeWideEvent({
geo: geo.country
? {
source: "maxmind",
country: geo.country,
region: geo.region,
city: geo.city,
}
: { source: "unresolved" },
});
mergeWideEvent({
geo: geo.country
? {
source: "maxmind",
country: geo.country,
}
: { source: "unresolved" },
});

Knowledge Base Used: Basket Ingestion Flow

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


return {
anonymizedIP: anonymizeIp(ip),
country: geo.country,
Expand Down