carverauto/serviceradar

Fork 0

Features/netflow V10 bpg support #2925

Merged

mikemiles-dev merged 4 commits from refs/pull/2925/head into staging

2026-03-16 04:11:03 +00:00

mikemiles-dev commented

2026-02-15 19:03:06 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2841
Original author: @mikemiles-dev
Original URL: https://github.com/carverauto/serviceradar/pull/2841
Original created: 2026-02-15T19:03:06Z
Original updated: 2026-03-16T04:11:03Z
Original head: mikemiles-dev/serviceradar:features/netflow-bpg
Original base: staging
Original merged: 2026-03-16T04:11:03Z by @mfreeman451

IMPORTANT: Please sign the Developer Certificate of Origin

Describe your changes

Extending Netflow Collector to Ingest V10 BGP Fields, Pipeline, Testing, and UI changes.

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

Imported from GitHub pull request. Original GitHub pull request: #2841 Original author: @mikemiles-dev Original URL: https://github.com/carverauto/serviceradar/pull/2841 Original created: 2026-02-15T19:03:06Z Original updated: 2026-03-16T04:11:03Z Original head: mikemiles-dev/serviceradar:features/netflow-bpg Original base: staging Original merged: 2026-03-16T04:11:03Z by @mfreeman451 --- ## IMPORTANT: Please sign the Developer Certificate of Origin ## Describe your changes Extending Netflow Collector to Ingest V10 BGP Fields, Pipeline, Testing, and UI changes. ## Issue ticket number and link ## Code checklist before requesting a review - [x] I have signed the DCO? - [x] The build completes without errors? - [x] All tests are passing when running make test?

mfreeman451 commented

2026-02-15 22:59:19 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810067539
Original created: 2026-02-15T22:59:19Z
Original path: cmd/consumers/netflow-ingestor/main.go
Original line: 1

check out the Broadway (elixir) stuff there, there is an EventWriter.Producer that is sending messages to NATS JetStream should be a similiar pattern for Consumers
https://github.com/carverauto/serviceradar/blob/staging/packaging/zen/rules/netflow_to_ocsf.json

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810067539 Original created: 2026-02-15T22:59:19Z Original path: cmd/consumers/netflow-ingestor/main.go Original line: 1 --- check out the Broadway (elixir) stuff there, there is an EventWriter.Producer that is sending messages to NATS JetStream should be a similiar pattern for Consumers https://github.com/carverauto/serviceradar/blob/staging/packaging/zen/rules/netflow_to_ocsf.json

mfreeman451 commented

2026-02-15 23:00:09 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810068532
Original created: 2026-02-15T23:00:09Z
Original path: pkg/api/netflow_bgp_handler.go
Original line: 1

this seems like it was built around the old golang API, need to re-do this in web-ng where the new API is

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810068532 Original created: 2026-02-15T23:00:09Z Original path: pkg/api/netflow_bgp_handler.go Original line: 1 --- this seems like it was built around the old golang API, need to re-do this in web-ng where the new API is

mfreeman451 commented

2026-02-15 23:00:33 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810068962
Original created: 2026-02-15T23:00:33Z
Original path: pkg/consumers/netflow-ingestor/service.go
Original line: 1

should be able to replace this with an elixir Broadway based consumer

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810068962 Original created: 2026-02-15T23:00:33Z Original path: pkg/consumers/netflow-ingestor/service.go Original line: 1 --- should be able to replace this with an elixir `Broadway` based consumer

mfreeman451 commented

2026-02-15 23:01:14 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810069818
Original created: 2026-02-15T23:01:14Z
Original path: pkg/db/cnpg_netflow.go
Original line: 26

I think all of these changes in pkg/db can be dropped and we just move this all into Elixir/Broadway and re-use existing Elixir models/schema

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810069818 Original created: 2026-02-15T23:01:14Z Original path: pkg/db/cnpg_netflow.go Original line: 26 --- I think all of these changes in `pkg/db` can be dropped and we just move this all into Elixir/Broadway and re-use existing Elixir models/schema

mfreeman451 commented

2026-02-15 23:01:52 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810070504
Original created: 2026-02-15T23:01:52Z
Original path: pkg/models/netflow.go
Original line: 37

need to find the Elixir model and update that instead I think

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810070504 Original created: 2026-02-15T23:01:52Z Original path: pkg/models/netflow.go Original line: 37 --- need to find the Elixir model and update that instead I think

mfreeman451 commented

2026-02-15 23:02:29 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810071175
Original created: 2026-02-15T23:02:29Z
Original path: rust/netflow-collector/src/converter.rs
Original line: 729

should these tests be in their own file?

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810071175 Original created: 2026-02-15T23:02:29Z Original path: rust/netflow-collector/src/converter.rs Original line: 729 --- should these tests be in their own file?

mfreeman451 commented

2026-02-15 23:05:02 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810074295
Original created: 2026-02-15T23:05:02Z
Original path: go.sum
Original line: 203

I'm guessing we can remove this as well

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810074295 Original created: 2026-02-15T23:05:02Z Original path: go.sum Original line: 203 --- I'm guessing we can remove this as well

mikemiles-dev commented

2026-02-15 23:37:06 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mikemiles-dev
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810114486
Original created: 2026-02-15T23:37:06Z
Original path: rust/netflow-collector/src/converter.rs
Original line: 729

I can, not uncommon for rust to put tests on bottom of files as well.

Imported GitHub PR review comment. Original author: @mikemiles-dev Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810114486 Original created: 2026-02-15T23:37:06Z Original path: rust/netflow-collector/src/converter.rs Original line: 729 --- I can, not uncommon for rust to put tests on bottom of files as well.

mikemiles-dev commented

2026-02-15 23:38:40 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mikemiles-dev
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810116948
Original created: 2026-02-15T23:38:40Z
Original path: rust/netflow-collector/src/converter.rs
Original line: 729

The #[cfg(test)] pattern is specifically designed for this use
case. Rust developers expect to find unit tests at the bottom of the source file.

The only reason to move them would be if:

You prefer a cleaner source file (use option 2)
You're writing integration tests that test the public API across modules (use option 3)

Imported GitHub PR review comment. Original author: @mikemiles-dev Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2810116948 Original created: 2026-02-15T23:38:40Z Original path: rust/netflow-collector/src/converter.rs Original line: 729 --- The #[cfg(test)] pattern is specifically designed for this use case. Rust developers expect to find unit tests at the bottom of the source file. The only reason to move them would be if: 1. You prefer a cleaner source file (use option 2) 2. You're writing integration tests that test the public API across modules (use option 3)

mfreeman451 commented

2026-02-24 17:57:17 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848728987
Original created: 2026-02-24T17:57:17Z
Original path: elixir/serviceradar_core/lib/serviceradar/bgp/as_lookup.ex
Original line: 14

i wonder if we could tie this into our ipinfo/geoip databases to lookup against their data instead of this table

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848728987 Original created: 2026-02-24T17:57:17Z Original path: elixir/serviceradar_core/lib/serviceradar/bgp/as_lookup.ex Original line: 14 --- i wonder if we could tie this into our ipinfo/geoip databases to lookup against their data instead of this table

mfreeman451 commented

2026-02-24 18:15:05 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848809651
Original created: 2026-02-24T18:15:05Z
Original path: elixir/serviceradar_core/lib/serviceradar/bgp/as_lookup.ex
Original line: 129

do we need this if we already have an ASN lookup database from ipinfo?

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848809651 Original created: 2026-02-24T18:15:05Z Original path: elixir/serviceradar_core/lib/serviceradar/bgp/as_lookup.ex Original line: 129 --- do we need this if we already have an ASN lookup database from ipinfo?

mfreeman451 commented

2026-02-24 18:16:53 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848817963
Original created: 2026-02-24T18:16:53Z
Original path: elixir/serviceradar_core/lib/serviceradar/application.ex
Original line: 81

see previous comment about ASN lookups using ipinfo/geomax db

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848817963 Original created: 2026-02-24T18:16:53Z Original path: elixir/serviceradar_core/lib/serviceradar/application.ex Original line: 81 --- see previous comment about ASN lookups using ipinfo/geomax db

mfreeman451 commented

2026-02-24 18:19:41 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848829840
Original created: 2026-02-24T18:19:41Z
Original path: web-ng/assets/js/app.js
Original line: 1876

this might be a little ugly to merge in here, i would synchronize this branch against staging here (pull in latest changes) because app.js has been completely refactored, this stuff will just need to go into its own file/directory or whatever (we split app.js up and it is bare bones now as it should be)

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848829840 Original created: 2026-02-24T18:19:41Z Original path: web-ng/assets/js/app.js Original line: 1876 --- this might be a little ugly to merge in here, i would synchronize this branch against staging here (pull in latest changes) because app.js has been completely refactored, this stuff will just need to go into its own file/directory or whatever (we split app.js up and it is bare bones now as it should be)

mfreeman451 commented

2026-02-24 18:21:32 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR review comment.

Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848838297
Original created: 2026-02-24T18:21:32Z
Original path: elixir/web-ng/lib/serviceradar_web_ng_web/router.ex
Original line: 348

should this be a new view or should it live under observability?

Imported GitHub PR review comment. Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2841#discussion_r2848838297 Original created: 2026-02-24T18:21:32Z Original path: elixir/web-ng/lib/serviceradar_web_ng_web/router.ex Original line: 348 --- should this be a new view or should it live under observability?