Repository navigation
feat: add native NetFlow monitoring with Lucene search and Sankey - #3347
valerypetrov wants to merge 15 commits into
Conversation
🦋 Changeset detectedLatest commit: 99df3ce The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@valerypetrov is attempting to deploy a commit to the HyperDX Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
external
needs-vouch
|
Hi @valerypetrov, thanks for the pull request! Before we review code from a first-time contributor we ask that a maintainer vouches for you, and you're not on our list yet. This PR stays open — it just isn't in the review queue until someone vouches. To get vouched, open an issue saying hello and what you're working on: https://github.com/hyperdxio/hyperdx/issues/new?template=introduce-yourself.md A maintainer will usually reply within a day or two, and then this PR gets picked up as normal. More detail in our contributing guide. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
PR Review
3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor None could be anchored to changed lines; they are listed below. Findings outside the changed lines3 minor
Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
Deep Review✅ No critical issues found. No P0/P1 defects surfaced from the completed reviewers or from direct inspection of the highest-risk paths. SQL injection is not present — filter values pass through 🟡 P2 -- recommended
🔵 P3 nitpicks (3)
Reviewers (5): api-contract, performance, previous-comments, learnings-researcher, orchestrator deep-inspection (security/correctness/SQL-escaping/Sankey/local-mode spot-checks). Testing gaps:
Coverage note: The correctness, security, adversarial, reliability, testing, maintainability, project-standards, kieran-typescript, and agent-native reviewers had not returned results at synthesis time; their lanes were partially covered by direct orchestrator inspection but a full pass on those dimensions did not complete. Treat the "no critical issues" verdict as grounded in the completed reviewers and the highest-risk spot-checks, not an exhaustive multi-persona sign-off. No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Addressed this review round in 9053fcb:
Validation: all app/API/common-utils unit suites passed, plus 58 MCP integration tests, ClickHouse query checks, and browser workflows covering source changes, search, filters, Sankey, axis layout, and time-range recovery. One app suite needed a rerun after a concurrent shared build temporarily removed its dependency; it passed. Repository lint/type/style checks, lint-staged, and Knip passed. For the remaining suggestions, following the repository's review scope rules:
SQL escaping already has unit and real ClickHouse coverage. Added explicit partial-credential routing cases, and documented why NetFlow uses an empty known-column set while restoring exact SQL expressions during serialization. |
|
Addressed the second review round in dee8414:
Validation: 110 backend integration tests and 72 snapshots passed, including real local ClickHouse/Mongo alert evaluation; 43 alert unit tests and 9 focused app tests passed. Both local browser workflows passed, covering drafts, source creation/switching, invalid ranges, search, autocomplete, records, and click filters. Repository lint/type/style/OpenAPI checks, lint-staged, and Knip passed. Historical log-alias fixtures required removing their expired TTL only in the isolated test ClickHouse instance. Remaining review suggestions:
Maintainers: please review the remaining scope and scale-validation items. Per AGENTS.md, “After two rounds of bot findings, stop.” This is the second round; further automated suggestions need maintainer triage. |
|
Addressed the remaining minor findings in a476fa9:
Validation: all 8,597 unit tests passed (4,603 app, 1,157 API, 2,837 common), plus 75 API integration tests and 14 local Playwright tests. The browser suite verifies a single summary request, distinct metric values, source creation/switching, autocomplete, Search, drafts, filters, Sankey, and real axis bounds. Geometry unit coverage uses real Recharts context and bars. Repository lint/type/style/OpenAPI checks, lint-staged, and Knip passed. Local Playwright used a temporary configuration that bypassed only the unrelated global PromQL seeder, which is incompatible with local ClickHouse 26.10; these NetFlow specs seeded their own real data. The committed runner and CI configuration remain unchanged. A separate two-million-row, five-dimension, 30-day Sankey check completed in 0.71 seconds with a 512 MiB memory cap and a 64 MiB spill threshold. The documentation explains that LIMIT bounds returned paths rather than aggregation memory and records the test's scope. |
Deep Review✅ No critical issues found. No P0/P1 defects surfaced across 14 reviewers or from direct inspection of the highest-risk paths. SQL stays parameterized — end-user filter/search values pass through 🟡 P2 -- recommended
🔵 P3 nitpicks (6)
Reviewers (14): correctness, security, adversarial, testing, maintainability, project-standards, performance, api-contract, reliability, kieran-typescript, julik-frontend-races, previous-comments, agent-native, learnings-researcher. Testing gaps:
No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond |
|
Addressed the latest review findings in bbd3e30:
Validation: 2,842 common-unit tests, 280 focused app tests, 68 API unit tests, 112 API integration tests, and all 15 NetFlow Playwright tests passed. Repository lint/type/style/OpenAPI checks, lint-staged, and Knip passed. Local Playwright again used the temporary global-setup bypass for the unrelated PromQL seeder incompatibility with ClickHouse 26.10; NetFlow fixtures seeded and cleaned up their own real ClickHouse data. For the repeated items in Deep Review, a476fa9 already includes the sampling multiplier/default-one description, SQL-metacharacter execution coverage in The two Vercel statuses require deployment authorization. Follow-up adc1e10 consolidates the 23 NetFlow query/hydration regressions into the existing test file. Both review jobs had stopped before code review because their |
| (async () => { | ||
| try { | ||
| if (watchedTableName !== prevTableNameRef.current) { | ||
| if ( |
There was a problem hiding this comment.
🟠 major — Schema inference now overwrites saved mappings on any source when the connection, database or kind changes
Inference used to run only when the table name changed. It now also runs on a kind, connection, database or metadata change, for every source kind. It calls resetField(..., { defaultValue }) on every inferred field that isn't dirty, and loaded saved values never count as dirty. So editing an existing Log or Trace source and switching it to another connection (for example a replica with the same table) silently swaps custom implicitColumnExpression, bodyExpression and similar values for the inferred defaults, and Save persists them. Fix: only re-infer on these new triggers when isNew is true or the watched kind is SourceKind.Netflow, or skip fields whose current value differs from their pristine value.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
There was a problem hiding this comment.
Fixed in 7ebbb8d. Context changes now trigger inference only for new sources; saved Log, Trace, and NetFlow mappings survive connection/database/metadata changes. Explicit table/kind changes still infer, and stale responses remain cancelled. Regression tests reproduced the overwrite for all three kinds before the fix; all 19 source-form tests pass.
| ], | ||
| where: '', | ||
| timestampValueExpression: source.timestampValueExpression, | ||
| ...(source.kind === SourceKind.Netflow && { |
There was a problem hiding this comment.
🔵 minor — Bare-term implicit column is limited to NetFlow in the external charts API
getSourceImplicitColumnExpression already returns the right value for every kind (Log/Trace configured expression, NetFlow derived, undefined otherwise). Gating it on source.kind === SourceKind.Netflow adds a special case and leaves Log/Trace series with bare Lucene terms in where without their configured implicit column. Set implicitColumnExpression: getSourceImplicitColumnExpression(source) unconditionally, as the other call sites in this diff do.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Fixed in 7ebbb8d by applying getSourceImplicitColumnExpression unconditionally. The Log/Trace omission predates this feature, but the already-edited builder now handles all three searchable kinds consistently. Real ClickHouse tests through POST /api/v2/charts/series verify bare-term plus numeric Lucene filtering for Log, Trace, and NetFlow (6 query integration tests pass).
| icon: <IconSitemap size={16} />, | ||
| isBeta: true, | ||
| }, | ||
| { |
There was a problem hiding this comment.
🔵 minor — NetFlow nav, spotlight and preset entries are shown to every user unconditionally
The nav link, the Spotlight preset (packages/app/src/Spotlights.tsx:93) and the dashboards-list preset all appear for every deployment, even with no NetFlow source. The neighbouring niche preset (Kubernetes) sits behind IS_K8S_DASHBOARD_ENABLED. Put these behind a flag, or hide them when useSources() has no SourceKind.Netflow source, or at least mark the nav entry isBeta like Service Map.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
There was a problem hiding this comment.
Marked the NetFlow nav entry isBeta in 7ebbb8d using the existing Service Map convention. Kept the entry discoverable when no source exists so users can reach its source-setup flow.
|
Addressed the current Claude, Greptile, and Deep Review findings in 7ebbb8d:
Validation: 173 affected app unit tests, 13 real ClickHouse/Mongo API integration tests, and all 16 NetFlow Playwright tests passed. Integration coverage now includes Log/Trace/NetFlow bare terms through the v2 series endpoint, saved-alert OK/no-match and grouped results, NULL/zero/negative sampling, URL range removal, and source-switch draft cleanup. Repository lint/type/style/OpenAPI checks, lint-staged, and Knip passed. The reported partial-bucket test gap was already covered by the real ClickHouse test in adc1e10 (61 buckets at the expected constant rate, plus exact-range totals). I retained the existing default-SELECT type guards: they already include Log, Trace, and NetFlow, while isSearchableSource returns a boolean rather than narrowing the source union; replacing them is a future refactor, not a current behavior fix. Removed the previous unnecessary Services-dashboard helper edit because that dashboard restricts sources to Trace. The PR remains at 100 files, keeping the existing review jobs' file-list check working. Local Playwright again used isolated NetFlow fixtures and the documented bypass for the unrelated global PromQL seeder on ClickHouse 26.10. Vercel deployment still requires authorization. |
| ] | ||
| .map(expression => expression?.trim()) | ||
| .filter(Boolean); | ||
| return `concatWithSeparator(' ', ${expressions.map(expression => `ifNull(toString(${expression}), '')`).join(', ')})`; |
There was a problem hiding this comment.
🔵 minor — Bare-term protocol search uses raw numbers, but the UI shows names like TCP
The default implicit expression wraps the raw protocolExpression (toString(Proto) gives '6'). The charts, records and pills all show transform(...) names from getNetflowDimensions (packages/app/src/netflow.ts), so a bare TCP becomes ILIKE '%TCP%' and matches nothing. That contradicts docs/netflow.md, which says bare terms search the protocol. It also re-lists the mapped fields separately from getNetflowDimensions. Fix: move the dimension builder (protocol name transform, ::ffff: stripping) into common-utils and build the implicit expression from it, so there is one list of searchable flow dimensions.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| isTraceSource(source) || isLogSource(source) | ||
| ? source.implicitColumnExpression | ||
| : undefined, | ||
| implicitColumnExpression: getSourceImplicitColumnExpression(source), |
There was a problem hiding this comment.
🔵 minor — NetFlow dashboard filters are unreachable from the UI
QUERY_EXPRESSION_FILTER_SOURCE_KINDS (packages/common-utils/src/filters.ts:872) still lists only Log/Trace/Session/Metric. QueryExpressionFilterEditForm and DBDashboardImportPage use it as allowedSourceKinds, so users can never pick a NetFlow source for a dashboard filter. The new hook path and its test only run for hand-crafted or API-created filters. Fix: add SourceKind.Netflow to that list if the changeset's dashboard-filter support is intended, or drop that claim.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| import { useDefaultTimeRange, useNewTimeQuery } from '@/timeQuery'; | ||
| import { parseAsJsonEncoded } from '@/utils/queryParsers'; | ||
|
|
||
| const queryParsers = { |
There was a problem hiding this comment.
🔵 minor — Two separate filter systems for the same flow fields
Exporter, protocol and source/destination IP can each be filtered two ways: the quick-filter text inputs (exporter/protocol/srcAddr/dstAddr URL params, rendered by buildNetflowWhere as raw-column SQL that wants '6') and click include/exclude filters (the filters param, keyed on the canonical dimension expression that wants 'TCP' and stripped IPv4 addresses). They show up separately (inputs vs pills) and use different value formats, and both URL formats now have to be supported. Fix: route the quick inputs through useNetflowFilterState/applyFilter so each field has one persisted representation.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| isNew={mode === 'new'} | ||
| sourceId={mode === 'edit' ? sourceId : undefined} | ||
| defaultName="NetFlow" | ||
| defaultKind={SourceKind.Netflow} |
There was a problem hiding this comment.
🔵 minor — 'Add NetFlow source' modal can create a non-NetFlow source
TableSourceForm always renders the full kind radio group, so in this modal the user can switch to Logs or Traces and save. NetflowPage's onCreate then calls clearFilters(created.id) with an id that is not in netflowSources, and the page shows 'NetFlow source unavailable'. Fix: add a prop to TableSourceForm that locks or hides the kind selector when defaultKind is forced, and pass it from this modal.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| label={SOURCE_KIND_LABELS[SourceKind.Promql]} | ||
| /> | ||
| )} | ||
| <Radio |
There was a problem hiding this comment.
🔵 minor — Beta NetFlow kind is shown without a feature flag
Every other optional source kind is gated (IS_METRICS_ENABLED, IS_SESSIONS_ENABLED, IS_PROMQL_ENABLED), but the NetFlow radio, nav entry (marked isBeta), spotlight action and preset dashboard card are always shown to every deployment. Fix: add an IS_NETFLOW_ENABLED flag in config.ts and gate these entry points with it, as PromQL does.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| <ActiveFilterPills | ||
| searchFilters={state} | ||
| chartConfig={chartConfig} | ||
| enableValueEditing={false} |
There was a problem hiding this comment.
NetFlow now uses ActiveFilterPills, whose range removal calls clearFilter and deletes all selections for that field. For a shared URL containing Bytes BETWEEN 100 AND 200 and Bytes NOT IN ('150'), removing only the range also removes the exclusion. The page then shows more traffic than the user intended.
Remove just range and keep the field’s included and excluded values.
| severityText: source.severityTextExpression, | ||
| traceId: source.traceIdExpression, | ||
| }; | ||
| } else if (source.kind === SourceKind.Netflow) { |
There was a problem hiding this comment.
🔵 minor — NetFlow keyColumns were added to describe_source but not to clickstack_list_sources
packages/api/src/mcp/tools/sources/listSources.ts:274-308 still builds meta.keyColumns only for Trace, Log and Metric, so agents that list sources get no flow mappings for NetFlow sources while describe_source returns them. Add the same SourceKind.Netflow branch there, ideally through one shared helper used by both tools so the two lists can't drift.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| label={SOURCE_KIND_LABELS[SourceKind.Promql]} | ||
| /> | ||
| )} | ||
| <Radio |
There was a problem hiding this comment.
🔵 minor — NetFlow source kind is always shown; Metric, Session and PromQL are each behind a feature flag
Gate the NetFlow radio behind its own flag, the way the sibling kinds use IS_METRICS_ENABLED, IS_SESSIONS_ENABLED and IS_PROMQL_ENABLED. Apply the same flag to the other entry points this PR adds without one: the /netflow nav link (AppNav.tsx), the Spotlight preset (Spotlights.tsx) and the dashboards-list preset (DashboardsListPage.tsx). As written, every deployment gets a beta source kind and page with no way to turn them off.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| @@ -2653,7 +2692,9 @@ export function isPromqlSource(source: TSource): source is TPromqlSource { | |||
| return source.kind === SourceKind.Promql; | |||
| } | |||
| export function isSearchableSource(source: TSource): boolean { | |||
There was a problem hiding this comment.
🔵 minor — Log/Trace/NetFlow kind check is copied by hand to ~6 sites because isSearchableSource is not a type guard
Make isSearchableSource a type guard (source is TLogSource | TTraceSource | TNetflowSource) and use it in resolveSelect (searchChartConfig.ts), AlertPreviewChart.tsx, computeAliasWithClauses (checkAlerts/index.ts), the select fallback in DBSearchPage.tsx, and both ChartEditorControls.tsx checks. Each of those now re-spells the same three-way kind check, so adding the next searchable kind means editing all of them again.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| tabIndex?: number; | ||
| }; | ||
| }; | ||
| type Props = { |
There was a problem hiding this comment.
🔵 minor — memo on NetflowFilterMenu has no effect because its callback props are recreated on every render
onFilter from useNetflowFilterState (NetflowFilterPills.tsx) is a new function on every NetflowPage render. It is passed down through NetflowCharts and NetflowRecords to every cell. NetflowSankeyTable and NetflowSankeyNode also pass inline onSelect arrows. So all 500×4 record cells re-render anyway, despite the comment about thousands of cells. Fix: wrap onFilter/onDimensionFilter in useCallback (with a ref for expressions/dimensions) and pass stable dimension/value props instead of inline closures; otherwise drop the memo.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
Bare Lucene terms searched the raw protocol number, so "TCP" matched no flows while charts, pills and records all show protocol names. The default full-text expression now uses the protocol-name transform from the NetFlow dimensions, moved to common-utils so both share one list. The source modal can save a non-NetFlow source. The page now switches only to a created NetFlow source instead of showing "NetFlow source unavailable". Flow records keyed rows by index behind an eslint-disable, which fails the escape-hatch ratchet. Rows are now keyed by content plus a repeat count. The changeset no longer claims dashboard-filter support, because dashboard filters cannot select NetFlow sources. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The previous commit replaced the protocol number with its name in the default full-text expression, so a bare "17" stopped matching UDP flows. Search both the number and the name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Charts, records and the Sankey show protocols by name, but the Protocol filter only matched the raw number, so typing "TCP" returned no flows. Known names now map to their protocol number before filtering. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
219da28 to
40b3978
Compare
| const emitFilters = (next: FilterState) => | ||
| onChange( | ||
| filtersToQuery( | ||
| Object.fromEntries( | ||
| Object.entries(next).map(([key, selection]) => [ | ||
| expressions.get(key) ?? | ||
| clickedExpressionsRef.current.get(key) ?? | ||
| key, | ||
| selection, | ||
| ]), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
emitFilters drops conditions that parseQuery cannot turn into pills. A shared NetFlow URL with Bytes > 100 initially applies that filter, but clicking Include on an exporter replaces it with only the exporter filter. The page then shows traffic outside the requested search.
Keep untouched conditions alongside the updated pill filters so clicks and removals do not silently widen the search.
Knowledge Base Used: Roll Back Lucene filter handling
… mappings Clicking Include/Exclude rebuilt the search from filter pills only and dropped conditions pills cannot represent (e.g. `Bytes > 100`). Switching to a table without optional columns kept the previous table's mapping because inference omitted absent columns; they are now returned empty. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
| [ | ||
| ['exporter', 'Exporter', 'All exporters'], | ||
| ['protocol', 'Protocol', '6 (TCP), 17 (UDP)'], | ||
| ['srcAddr', 'Source IP', 'IPv4 or IPv6 address'], |
There was a problem hiding this comment.
🔵 minor — Two independent filter systems for the same NetFlow fields, with different matching rules
The exporter/protocol/srcAddr/dstAddr text inputs write their own URL params, and buildNetflowWhere turns them into toIPv6OrNull(...) = ... / toString(...) = ... predicates. The Include/Exclude clicks write to filters, which match with IN on the replaceRegexpOne/ifNull(toString()) dimension expression. So you can set source IP X in the input and click Include on source IP Y. The two conditions are ANDed, the result is empty, and the input value never shows up as a pill. Fix: have the quick inputs write to the same useNetflowFilterState FilterState (one URL param and one predicate builder), or remove the inputs now that click filters exist.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| whereLanguage, | ||
| filters: [ | ||
| ...extraFilters, | ||
| { type: 'sql' as const, condition: buildNetflowWhere(source, filters) }, |
There was a problem hiding this comment.
🔵 minor — Every NetFlow query carries an empty SQL filter, which forces the text-index metadata lookup
buildNetflowWhere returns '' when no quick filter is set, but { type: 'sql', condition: '' } is still added to every config. renderChartConfig.ts:1326 sets hasSqlPredicate when any filter has type === 'sql', so all ~10 NetFlow queries per page load run buildTextIndexInfoLookup metadata calls for nothing. Fix: only add the filter when the condition is non-empty, e.g. ...(cond ? [{ type: 'sql', condition: cond }] : []).
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| ? { timestampValueExpression: 'TimeReceived' } | ||
| : {}), | ||
| defaultTableSelectExpression: selected.join(', ') || '*', | ||
| // A mapping for a column the table lacks is returned empty so that switching tables clears it. |
There was a problem hiding this comment.
🔵 minor — Switching a saved NetFlow source to another table keeps stale full-text and timestamp expressions
The comment says missing mappings come back empty so a table switch clears them. But implicitColumnExpression is not in NETFLOW_COLUMN_EXPRESSIONS, and timestampValueExpression is only returned when TimeReceived (or a primary-key timestamp) exists. Edit a saved source with implicitColumnExpression: 'SearchText' and point it at a table without that column: the old value survives, and every bare Lucene term then fails. Fix: also return implicitColumnExpression: '', and an empty timestamp when none is detected, so a table change resets them like the other mappings.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| label={SOURCE_KIND_LABELS[SourceKind.Promql]} | ||
| /> | ||
| )} | ||
| <Radio |
There was a problem hiding this comment.
🔵 minor — NetFlow source kind is shipped ungated, unlike the other optional kinds
Metric, Session and PromQL radios are behind IS_METRICS_ENABLED/IS_SESSIONS_ENABLED/IS_PROMQL_ENABLED. The beta NetFlow kind is always shown here, and so are its AppNav link, preset dashboard entry and Spotlight action. Fix: add an IS_NETFLOW_ENABLED flag (as IS_PROMQL_ENABLED does, including the setupDefaults.ts skip) and gate the radio, nav link, preset and spotlight on it.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
| | { label: string; onSelect?: (excluded: boolean) => void } | ||
| ); | ||
|
|
||
| export default memo(function NetflowFilterMenu(props: Props) { |
There was a problem hiding this comment.
🔵 minor — memo on NetflowFilterMenu never prevents a re-render
Every caller passes a new callback on each render. useNetflowFilterState returns new onFilter/onDimensionFilter closures every render, NetflowCharts passes an inline renderGroupLabel, and NetflowSankeyTable/NetflowSankeyNode build onSelect inline. The shallow prop comparison therefore always fails across the hundreds of cells in the flow table. Fix: either drop memo, or wrap the handlers in useCallback in useNetflowFilterState and pass (field, value) instead of building closures per cell.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
Conflicts: the time chart keeps the auto-sized y-axis and measured plot width (main's fixed yAxisWidth is dropped with it), the MCP source schema takes main's field descriptions plus the NetFlow wording, and the row side panel takes main's TSource. Follow-ups for main: netflow stays in the save_source kinds when PromQL is off, the NetFlow MCP fields get descriptions, usePageTitle moved to @/brand, LineData has groupValues, and the time display setting is now a time zone.
Summary
Add native NetFlow monitoring for Akvorado tables through HyperDX connections, source configuration, Search, and charts. Integrate shared schemas, persistence, and the external API with configurable mappings and schema detection.
The page includes sampling-aware rates, top talkers, exporter/interface breakdowns, flow details, and a Sankey with two to five ordered dimensions. Lucene supports column suggestions; clickable values provide include/exclude filters. Searches, filters, and visualization settings persist in the URL. Sankey links represent selected top paths with bitrate and byte totals. Fix clipped rate-axis labels and direct ClickHouse requests in local mode.
Validation: unit suites, TypeScript, lint, ClickHouse query checks, and browser workflows passed. Screenshots use 14,400 synthetic records; setup instructions are in
docs/netflow.md.Screenshots
How to test on Vercel preview
Preview routes: /netflow
Proto:6, select Sankey dimensions, and include/exclude a node.Reference
Akvorado provides the schema and visualization reference.