Repository navigation
feat: add ISP, organization, usage type and connection type tracking - #490
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds network intelligence fields (ISP, organisation, usage type, connection type) to IP enrichment, threads them through controllers, transformers, and services, updates ClickHouse schema/migrations, and exposes UI panels, filters, icons, and documentation for the new dimensions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Request
participant Controller as Analytics Controller
participant IP as IP Geolocation Service
participant Transformer as Event Transformer
participant Service as Analytics Service
participant DB as ClickHouse DB
Client->>Controller: send event (with client IP)
Controller->>IP: getIPDetails(ip)
IP-->>Controller: {cc, rg, rgc, ct, isp, organization, userType, connectionType}
Controller->>Transformer: transformEvent({..., cc, rg, rgc, ct, isp, og, ut, ctp})
Transformer-->>Controller: enrichedEvent
Controller->>Service: processEvent(enrichedEvent)
Service->>Service: apply EXCLUDE_NULL_FOR filters (isp, og, ut, ctp, ...)
Service->>DB: insert/enqueue enrichedEvent
DB-->>Service: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Blaumaus
marked this pull request as draft
March 20, 2026 00:39
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/cloud/src/common/utils.ts`:
- Around line 269-307: getGeoDetails currently extracts
ISP/organization/userType/connectionType traits (isp, organization, userType,
connectionType) but the deployed DB-IP City Lite (dbip-free-sync.js) does not
contain those traits so they'll always be null; fix by either switching the sync
script to dbip-commercial-sync.js and wiring the DB-IP API key so the commercial
DB with traits is used, or remove the trait extraction and related return fields
from getGeoDetails (and any upstream consumers) so the function only returns
country, city, region, and regionCode when using the free DB; update
callers/types accordingly (refer to getGeoDetails and the trait variables
isp/organization/userType/connectionType).
In `@web/public/locales/en.json`:
- Around line 1579-1583: Add the missing locale key project.network to the
English locale (web/public/locales/en.json) so ViewProject.helpers.tsx's
t('project.network') resolves; mirror the existing dashboard.network value
(e.g., "Network") to keep labels consistent and prevent fallback to the key
text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a4491ee-4071-47e4-b63e-3b46abf7c20a
📒 Files selected for processing (16)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/utils/transformers.tsbackend/apps/cloud/src/common/constants.tsbackend/apps/cloud/src/common/utils.tsbackend/meta/dbip-commercial-sync.jsbackend/migrations/clickhouse/2026_03_19_isp_network.jsbackend/migrations/clickhouse/initialise_database.jsweb/app/lib/constants/index.tsweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/utils/filters.tsxweb/app/pages/Project/tabs/Errors/ErrorsView.tsxweb/app/pages/Project/tabs/Performance/PerformanceView.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/ui/FilterValueInput.tsxweb/public/locales/en.json
Blaumaus
marked this pull request as ready for review
April 27, 2026 22:43
There was a problem hiding this comment.
🧹 Nitpick comments (4)
backend/apps/community/src/common/utils.ts (1)
1146-1161: Optional: Reuse the localtraitsvariable for the new field reads.
traitsis already destructured on line 1146 and used on line 1148 forisHosting. The four new lookups on lines 1158–1161 redundantly traversedata?.traits?.…again — usingtraits.isp,traits.organization,traits.user_type,traits.connection_typewould be slightly more consistent and avoids the extra optional-chaining hops.♻️ Proposed refactor
- const isp = data?.traits?.isp || null - const organization = data?.traits?.organization || null - const userType = data?.traits?.user_type || null - const connectionType = data?.traits?.connection_type || null + const isp = traits.isp || null + const organization = traits.organization || null + const userType = traits.user_type || null + const connectionType = traits.connection_type || null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/community/src/common/utils.ts` around lines 1146 - 1161, The new lookups for isp, organization, userType, and connectionType repeat optional-chaining into data?.traits; reuse the already-declared local variable traits instead: replace data?.traits?.isp, data?.traits?.organization, data?.traits?.user_type, and data?.traits?.connection_type with traits.isp, traits.organization, traits.user_type, and traits.connection_type respectively (preserving the null fallback behavior used elsewhere), so the isHosting logic and subsequent reads all use the same traits reference.backend/apps/cloud/src/common/utils.ts (1)
283-296: Optional: Reuse the localtraitsvariable for the new field reads.
traitsis already extracted on line 283. Lines 293–296 re-traversedata?.traits?.…; usingtraits.isp/traits.organization/traits.user_type/traits.connection_typewould be more consistent with line 285 and slightly cheaper.♻️ Proposed refactor
- const isp = data?.traits?.isp || null - const organization = data?.traits?.organization || null - const userType = data?.traits?.user_type || null - const connectionType = data?.traits?.connection_type || null + const isp = traits.isp || null + const organization = traits.organization || null + const userType = traits.user_type || null + const connectionType = traits.connection_type || null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/common/utils.ts` around lines 283 - 296, The code extracts const traits = data?.traits ?? {} but then redundantly reads data?.traits for isp, organization, user_type and connection_type; update the assignments for isp, organization, userType and connectionType to read from the local traits variable (e.g., const isp = traits.isp || null, const organization = traits.organization || null, const userType = traits.user_type || null, const connectionType = traits.connection_type || null) so they mirror the earlier isHosting/traits usage and avoid re-traversing data.docs/content/docs/api/stats.mdx (1)
1471-1551: Optional: Update the common request example response to include the new params keys.The
/v1/logexample response at the top (lines 220-248) was extended withisp,og,ut, andctparrays, but the second example response under "Common request examples" still only shows the legacy keys (ending at"co": []). For consistency between the two example responses for the same endpoint, consider appending emptyisp/og/ut/ctparrays here as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/api/stats.mdx` around lines 1471 - 1551, The "Common request examples" response params object is missing the new keys added to the top /v1/log example; update the second example under "Common request examples" by adding empty arrays for the new params keys isp, og, ut, and ctp inside the params object so both examples match; locate the params JSON in the docs/content/docs/api/stats.mdx example (the object containing "cc","rg","ct",...,"co") and append isp: [], og: [], ut: [], ctp: [] to that object.backend/apps/community/src/analytics/utils/transformers.ts (1)
26-256: Consider refactoring to an options-object signature.These transformers already had long positional parameter lists, and this PR pushes them further (e.g.,
trafficTransformerto 26 positional args,customEventTransformer/errorEventTransformerto 27). Inserting four new geo-adjacent parameters (isp,og,ut,ctp) at a specific position relies on every call site being updated in the exact same order — a silent off-by-one (e.g., swappingut/ctpor shiftingisppastct) would corrupt analytics rows without any type-system protection since all the new params share the samestring | nulltype.Switching to a single typed parameter object (e.g.,
trafficTransformer({ psid, profileId, …, isp, og, ut, ctp, meta })) would eliminate this category of bug and make future column additions safer. Not blocking for this PR, but recommended as a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/community/src/analytics/utils/transformers.ts` around lines 26 - 256, The transformer functions (trafficTransformer, customEventTransformer, errorEventTransformer, performanceTransformer) use long positional parameter lists which are error-prone; refactor each to accept a single typed options object (e.g., trafficTransformer(opts: { psid: string; profileId: string; pid: string; host?: string | null; ... isp?: string | null; og?: string | null; ut?: string | null; ctp?: string | null; meta?: Record<string,string> | null })) and then destructure inside the function, preserving the current null-coalescing behavior and use of processMetaKV(meta) and dayjs.utc() for created; update all call sites to pass a single object with named properties to avoid positional mistakes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/apps/cloud/src/common/utils.ts`:
- Around line 283-296: The code extracts const traits = data?.traits ?? {} but
then redundantly reads data?.traits for isp, organization, user_type and
connection_type; update the assignments for isp, organization, userType and
connectionType to read from the local traits variable (e.g., const isp =
traits.isp || null, const organization = traits.organization || null, const
userType = traits.user_type || null, const connectionType =
traits.connection_type || null) so they mirror the earlier isHosting/traits
usage and avoid re-traversing data.
In `@backend/apps/community/src/analytics/utils/transformers.ts`:
- Around line 26-256: The transformer functions (trafficTransformer,
customEventTransformer, errorEventTransformer, performanceTransformer) use long
positional parameter lists which are error-prone; refactor each to accept a
single typed options object (e.g., trafficTransformer(opts: { psid: string;
profileId: string; pid: string; host?: string | null; ... isp?: string | null;
og?: string | null; ut?: string | null; ctp?: string | null; meta?:
Record<string,string> | null })) and then destructure inside the function,
preserving the current null-coalescing behavior and use of processMetaKV(meta)
and dayjs.utc() for created; update all call sites to pass a single object with
named properties to avoid positional mistakes.
In `@backend/apps/community/src/common/utils.ts`:
- Around line 1146-1161: The new lookups for isp, organization, userType, and
connectionType repeat optional-chaining into data?.traits; reuse the
already-declared local variable traits instead: replace data?.traits?.isp,
data?.traits?.organization, data?.traits?.user_type, and
data?.traits?.connection_type with traits.isp, traits.organization,
traits.user_type, and traits.connection_type respectively (preserving the null
fallback behavior used elsewhere), so the isHosting logic and subsequent reads
all use the same traits reference.
In `@docs/content/docs/api/stats.mdx`:
- Around line 1471-1551: The "Common request examples" response params object is
missing the new keys added to the top /v1/log example; update the second example
under "Common request examples" by adding empty arrays for the new params keys
isp, og, ut, and ctp inside the params object so both examples match; locate the
params JSON in the docs/content/docs/api/stats.mdx example (the object
containing "cc","rg","ct",...,"co") and append isp: [], og: [], ut: [], ctp: []
to that object.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5bdd546d-7bbf-4983-8055-6397f346e062
📒 Files selected for processing (14)
README.mdbackend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/common/utils.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/utils/transformers.tsbackend/apps/community/src/common/constants.tsbackend/apps/community/src/common/utils.tsbackend/migrations/clickhouse/initialise_database.jsdocs/content/docs/analytics-dashboard/performance.mdxdocs/content/docs/analytics-dashboard/traffic.mdxdocs/content/docs/api/stats.mdxdocs/content/docs/selfhosting/configuring.mdx
✅ Files skipped from review due to trivial changes (3)
- docs/content/docs/selfhosting/configuring.mdx
- README.md
- backend/migrations/clickhouse/initialise_database.js
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/cloud/src/analytics/analytics.controller.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/apps/community/src/analytics/utils/transformers.ts (1)
26-53: AlignprocessMetaKVsignature with nullablemetacontract.The three transformer option types declare
meta?: Record<string, string> | null, butprocessMetaKVaccepts only non-nullableRecord<string, string>. While strict mode is not currently enabled in this module'stsconfig, the type mismatch should be resolved for clarity—the implementation already safely handles null and undefined.♻️ Proposed fix
const processMetaKV = ( - rawMeta: Record<string, string>, + rawMeta?: Record<string, string> | null, ): { 'meta.key': string[] | null; 'meta.value': string[] | null } => {Applies to: lines 10–24, and all three call sites at 109, 200, 291.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/community/src/analytics/utils/transformers.ts` around lines 26 - 53, The transformer option types (e.g., TrafficTransformerOptions) declare meta?: Record<string,string> | null but processMetaKV currently accepts only non-nullable Record<string,string>; update processMetaKV's parameter type to accept Record<string,string> | null | undefined (or make it optional) and keep its existing null/undefined-safe logic, then ensure all call sites (the three transformer functions that pass .meta) compile with the relaxed signature; reference processMetaKV and the transformer option types (e.g., TrafficTransformerOptions) when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/apps/community/src/analytics/utils/transformers.ts`:
- Around line 26-53: The transformer option types (e.g.,
TrafficTransformerOptions) declare meta?: Record<string,string> | null but
processMetaKV currently accepts only non-nullable Record<string,string>; update
processMetaKV's parameter type to accept Record<string,string> | null |
undefined (or make it optional) and keep its existing null/undefined-safe logic,
then ensure all call sites (the three transformer functions that pass .meta)
compile with the relaxed signature; reference processMetaKV and the transformer
option types (e.g., TrafficTransformerOptions) when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08af363b-7e00-47bf-bc60-a2350d5ec211
📒 Files selected for processing (7)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/utils/transformers.tsbackend/apps/cloud/src/common/utils.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/utils/transformers.tsbackend/apps/community/src/common/utils.tsdocs/content/docs/api/stats.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/content/docs/api/stats.mdx
- backend/apps/community/src/common/utils.ts
- backend/apps/cloud/src/analytics/utils/transformers.ts
3 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
Documentation