You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Cisco NBAR application reports (maintainer continuation of #295)
This continues #295 by @alcatron. It is the same branch name (feature/cisco-nbar-reports) and keeps all of @alcatron's original commits and history. The maintainer rework is added on top as separate commits, so the original work stays attributed and reviewable.
#295 is left open and untouched so the two can be compared. Once this PR is reviewed, #295 can be closed in favour of it.
Cisco Apps (NBAR) reports for application-bearing NetFlow v9 exports: IPv4/IPv6 application totals, conversations and paged connections, per-account capture selection and local-network preferences, hourly and daily summaries maintained by the Cacti poller, and a separate retention setting.
What the maintainer commits on top change
Structure: library-only files moved to includes/, non-poller/non-service CLIs moved to cli/, requires hoisted to the top of each file and parenthesized.
Style:print instead of echo, tab-indented CSS/JS, Cacti develop php-cs-fixer rules applied (to PR-added lines in files that existed before this PR, to whole new files).
Types and safety: parameter and return types throughout, cacti_count()/cacti_sizeof() instead of count()/sizeof() (the bundled Net/ DNS code is excluded), PHPStan level 8 clean.
Upgrade guard: the background database upgrade is launched only inside register_process_start() with a 4 hour timeout and is released with unregister_process().
Table creation: the multi-layer NBAR schema builder is replaced by standard CREATE TABLE IF NOT EXISTS statements in includes/database.php. This works with the flow tables in an external database.
Partitioning: raw NBAR records are stored in time-sliced tables (plugin_flowview_nbar_data_<year><day>[<hour>]) that follow the Database Partitioning Scheme setting. Slices are named by UTC. Readers union only the slices overlapping the range, and retention drops whole slices.
Bugs found while reworking:
First/latest-record lookups returned the wrong record when daily and hourly slices overlapped after a partition-scheme change.
The report built d.listener_id IN () (a SQL error) when no capture device existed.
The CHANGELOG develop section was rewritten and re-sorted; it is restored and only the NBAR feature entries are added.
Validation
373 Pest tests, 100% coverage of changed lines.
PHPStan level 8 with no errors.
php-cs-fixer is at a fixed point.
Lint is clean on PHP 8.3 (8.4 lint run earlier).
Live runs on MariaDB 10.5: install, ingest, hourly and daily partition schemes, summary builds, retention, the Top apps / Conversations / Connections pages, and an external flow database.
…equires)
- Move the pure-library nbar_*.php files into includes/ and drop their request guards; phpunit.xml coverage includes follow them.
- Move the non-poller, non-service CLI (flowview_upgrade.php) into cli/ and fix its chdir depth, callers, manifest tombstone and patch-coverage allowlist.
- Hoist mid-file require/include statements to the top of each file and parenthesise every require/include path.
- Add parameter and return types to every function and method this PR adds.
- Format css/nbar.css and js/nbar-*.js with tabs.
- Drop the obsolete direct-request guard scenarios for files that no longer carry guards.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…type test stubs
- php-cs-fixer with Cacti develop's rules (echo->print, spacing, blank lines, alignment) over every file this PR adds; for pre-existing files only the lines this PR added are touched.
- Regenerate locales/po/cacti.pot (xgettext step only): same msgids, updated file references for the moved files.
- Type the Pest/DNS/report stub functions and classes in the new tests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
plugin_flowview_check_upgrade() runs on every page load while an upgrade is pending, so each request could spawn another background upgrade. Launch it only when register_process_start() succeeds (four hour timeout). cli/flowview_upgrade.php releases that lock, and its own run lock, however it ends (early exit, success, or fatal error via a shutdown function); its own registration timeout drops from 24 hours to 4 hours to match.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ngs in NBAR code
- Replace count()/sizeof() with cacti_count()/cacti_sizeof() in production code (Net/ excluded); add a cacti_count stub to the unit bootstrap.
- Narrow-width fv_nbar_int() for NetFlow header fields; typed/guarded results for unpack, preg_split, inet_pton/inet_ntop, hex2bin, json_encode, parse_url.
- Declare variables inherited by the included report fragments, initialise by-reference arguments, and pass correctly typed html_start_box() arguments.
- Remove an always-true condition in fv_section_params().
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Guard array|bool database results with is_array() before use as arrays (flowview.php, flowview_schedules.php, run_schedule.php, includes/database.php, includes/functions-pre13.php).
- Initialise the by-reference report tag as a bool for reports_load_format_file().
- Pass the message level to raise_message_javascript() as its level argument instead of as the message body.
- Keep the Query Service request body a string; document the by-reference output text parameter as a string.
- Add regression tests for the changed measured lines and allowlist the pre-1.3 compatibility shim in the patch-coverage gate.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s in includes/database.php
The NBAR schema was built by five layers: a generic definition builder (includes/nbar_database.php), fv_nbar_summary_schema(), fv_nbar_daily_schema(), fv_nbar_seen_schema() and fv_nbar_exporter_schema(), orchestrated by fv_nbar_schema_install(). The layers created tables and then patched them with ALTER TABLE and seed inserts to reach the final shape.
Replace all of that with flowview_db_create_nbar_tables() in includes/database.php: one CREATE TABLE IF NOT EXISTS per table in the same format as flowview_setup_table(), each verified after creation. Verified against a real MariaDB: the 11 tables are identical, column for column and key for key, to what the old chain produced (the only difference is a table comment on each).
Remove includes/nbar_database.php and the schema installers. NBAR has not shipped, so no deprecation shims are kept, and the ALTER/seed steps that only repaired tables from earlier development revisions of this branch are gone. Tests are rewritten against the new function.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Raw NBAR records now go to plugin_flowview_nbar_data_<year><day>[<hour>]
tables, following the Database Partitioning Scheme setting (hourly by
default, daily when set to daily), the same way the raw flow tables are
partitioned.
- Slices are created when the first record for them arrives and are
named by UTC time so the repeated daylight saving hour cannot share a
table.
- Readers list the existing slices and test each slice's time range, so
slices from before a scheme change keep working.
- The summary, daily, connection, seen and feed status readers build a
UNION ALL of per-slice selects, each with the same filter, and fall
back to an empty typed relation when no slice exists yet.
- Retention drops whole expired slices instead of deleting rows.
- The collector batches rows per slice and writes them together with the
hour revision bump in one transaction.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…HANGELOG order
The NBAR report built "d.listener_id IN ()" when no enabled capture
device existed, which is a SQL error. Capture ids start at 1, so the
scope now uses IN (0) and matches nothing.
CHANGELOG.md is restored to the develop ordering and wording so the PR
no longer rewrites unrelated entries; only the NBAR feature entries are
added.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…upgrade launch
- A failed SHOW TABLES for the raw NBAR slices is no longer an empty
list. fv_nbar_data_tables() throws, so hourly builds roll back instead
of publishing an empty hour, retention skips the pass, the first/latest
record lookups report failure, and the report shows an error instead of
treating the history as missing.
- The first/latest record lookup probes the slices in batches of 32
rather than one query per slice, so a capture with no records costs a
handful of queries instead of one per retained slice.
- The deferred connection count stores its selection criteria and
rebuilds its source when it runs, so a slice created after the page
rendered is included. The cache key is derived from the criteria.
- The upgrade CLI takes over the launcher's process registration, so a
timeout recovery can only signal the upgrade and never a reused web
worker.
- The test bootstrap exec_background() stub now has Cacti's real
signature; the two-argument stub made static analysis flag the
plugin's three-argument calls.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Raw-slice expiration runs before the cleanup deadline is created, and fv_nbar_data_drop_expired() checks no deadline between drops. After a retention reduction or polling backlog, dropping many slices can consume the supervisor's 90-second limit before summary maintenance starts. Compute one deadline at cleanup entry, pass it to the raw-drop loop, check it between drops, and reuse it for the summary deletion batches.
Test does not verify future-date row filtering
tests/Unit/NbarRenderingTest.php:49
Queue a raw slice before the future-dated row. The default SHOW TABLES result is empty, so fv_nbar_data_edge() returns without consuming the queued row. This test currently checks an empty capture and still passes if the future-date guard is removed.
…the cleanup deadline
- The chart, the conversations table and the applications table catch a
failed slice listing and show their own error instead of aborting the
report, which matters when the earlier record and status checks
succeeded and the failure comes later in the same request.
- fv_nbar_cleanup() computes one deadline up front and the raw slice drops
honour it, oldest slices first, so dropping a backlog of expired slices
cannot use up the supervisor's time before the summaries are maintained.
- The future-dated feed status test now queues a raw slice so the lookup
actually runs; it previously only exercised an empty capture.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addressed the two findings from the latest review summary that were not inline threads, in 2a82bc4:
Raw-slice cleanup could exceed the supervisor timeout:fv_nbar_cleanup() now creates a single deadline at entry. fv_nbar_data_drop_expired() takes an optional deadline and checks it between drops (oldest slices first, so an interrupted pass leaves the newest expired slices for the next one), and the summary deletion batches reuse the same deadline. Tests cover an already-expired deadline, a shared deadline and the drop loop.
Future-date feed status test did not verify the filter: the test now queues a raw slice so the lookup runs, asserts the lookup happened, and asserts the feed is reported as 'No recent NBAR' rather than 'Receiving' or 'No retained NBAR records'.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Collector, storage and process-supervision changes need final human integration review, particularly because live MySQL 8 validation remains outstanding.
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
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.
Cisco NBAR application reports (maintainer continuation of #295)
This continues #295 by @alcatron. It is the same branch name (
feature/cisco-nbar-reports) and keeps all of @alcatron's original commits and history. The maintainer rework is added on top as separate commits, so the original work stays attributed and reviewable.#295 is left open and untouched so the two can be compared. Once this PR is reviewed, #295 can be closed in favour of it.
What @alcatron's work adds
Cisco Apps (NBAR) reports for application-bearing NetFlow v9 exports: IPv4/IPv6 application totals, conversations and paged connections, per-account capture selection and local-network preferences, hourly and daily summaries maintained by the Cacti poller, and a separate retention setting.
What the maintainer commits on top change
includes/, non-poller/non-service CLIs moved tocli/, requires hoisted to the top of each file and parenthesized.printinstead ofecho, tab-indented CSS/JS, Cacti developphp-cs-fixerrules applied (to PR-added lines in files that existed before this PR, to whole new files).cacti_count()/cacti_sizeof()instead ofcount()/sizeof()(the bundledNet/DNS code is excluded), PHPStan level 8 clean.register_process_start()with a 4 hour timeout and is released withunregister_process().CREATE TABLE IF NOT EXISTSstatements inincludes/database.php. This works with the flow tables in an external database.plugin_flowview_nbar_data_<year><day>[<hour>]) that follow the Database Partitioning Scheme setting. Slices are named by UTC. Readers union only the slices overlapping the range, and retention drops whole slices.d.listener_id IN ()(a SQL error) when no capture device existed.Validation
php-cs-fixeris at a fixed point.Not yet validated