gigamon: add CEF/UDP input to ami data stream#18402
Conversation
|
💚 CLA has been signed |
ReviewersBuildkite won't run for external contributors automatically; you need to add a comment:
NOTE: https://github.com/elastic/integrations/blob/main/.buildkite/pull-requests.json contains all those details. |
3c3e011 to
df66721
Compare
|
/test |
|
Pinging @elastic/integration-experience (Team:Integration-Experience) |
df66721 to
32d4883
Compare
|
@kcreddy could you please drop a comment to initiate the test ? |
|
/test |
|
@kcreddy we noticed a few fields that still need to be renamed or dropped. I’ll update those early next week, and then you can proceed with merging the PR. |
@sanjay-2307, I've moved this PR to draft til then. You can mark it
GIgamon used to be owned by our team, but recently there was a change in ownership. Once the owned team approves the PR, then only it can be merged. |
32d4883 to
0d24106
Compare
0d24106 to
79a01d8
Compare
|
@kcreddy PR is now ready for review. Could you please trigger the Buildkite test with |
|
/test |
|
@sanjay-2307 , there is an error in the CI: You will need to run |
79a01d8 to
c78a3af
Compare
|
@kcreddy I have added the latest README.md file. |
|
/test |
💚 Build Succeeded
History
|
|
@elastic/integration-experience PTAL at this PR. This integration recently changed ownership from Service Integrations team to IE. |
|
@elastic/integration-experience / @andrewkroh Gentle follow-up on this PR, could you please share the current status when you have a moment? |
|
Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as |
|
👀 I have started reviewing the PR |
| target_field: gigamon.ami | ||
| if: ctx.json != null | ||
| tag: rename_json | ||
| - pipeline: |
There was a problem hiding this comment.
🟡 MEDIUM data_stream/ami/.../default.yml:12
New CEF/UDP processing path ships without pipeline tests
This PR adds a brand-new CEF processing branch here (- pipeline: guarded by if: ctx.cef != null) that calls the new 730-line cef-pipeline.yml. None of it is exercised by a test: the only pipeline fixture, _dev/test/pipeline/test-ami.json, contains JSON-input events only, so every rename in cef-pipeline.yml and the entire cef != null branch run untested. This is a newly introduced code path (not a pre-existing gap), so a regression in CEF field renaming, the cef.extensions -> gigamon.ami mapping, or the new TcpRtt* converts would not be caught.
Recommendation:
Add a CEF pipeline test fixture and expected output covering the cef != null branch, e.g. _dev/test/pipeline/test-cef.log with a matching test-cef.log-expected.json. Configure the CEF test in _dev/test/pipeline/test-common-config.yml so the decode happens correctly:
fields:
tags:
- preserve_original_event
dynamic_fields:
"event.ingested": ".*"The fixture should include at least one raw CEF message that populates gigamon.ami.* extension fields (src/dst IP, ports, the new TcpRtt* and app_tags fields) so the new pipeline and converts are validated.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
I wanted to confirm the right approach, as I hit a constraint while implementing it.
The recommendation was to add a raw test-cef.log fixture and configure decoding in test-common-config.yml. However, in this package CEF decoding is done by the agent-side decode_cef processor (in udp.yml.hbs), and decode_cef is not available as an Elasticsearch ingest processor. When I tried adding decode_cef to the ingest pipeline (default.yml), Put Pipeline fails with:
No processor type exists with name [decode_cef]
Because pipeline tests only exercise the ingest pipeline (not agent processors), a raw test-cef.log never gets decoded during the test, so ctx.cef stays null and the cef != null branch isn’t actually exercised (the generated expected doc only contains the raw message and no gigamon.ami.* fields).
To genuinely cover the branch, I added a pre-decoded CEF fixture (test-cef.json) that mirrors the agent’s decode_cef output (populated cef.extensions), which correctly triggers the cef != null path and validates the cef.extensions → gigamon.ami renames, the new TcpRtt* converts, and app_tags. Both tests pass:
test-ami.json PASS
test-cef.json PASS
Could you please confirm this approach is acceptable, i.e. using a pre-decoded test-cef.json fixture instead of a raw test-cef.log, given that decode_cef can only run agent-side? If you’d prefer a different pattern for covering the CEF branch, happy to adjust.
c78a3af to
5c3e6ee
Compare
5c3e6ee to
087d7ab
Compare
087d7ab to
4b389d5
Compare
630ac1d to
05bbead
Compare
05bbead to
591860c
Compare
591860c to
bfb883f
Compare
|
No issues across the latest commits bfb883f. Review summaryIssues found across earlier commits 591860c — 1 medium, 2 low
Issues found across earlier commits 05bbead — 1 low
Issues found across earlier commits 4b389d5 — 2 high
Issues found across earlier commits 087d7ab — 1 medium, 1 low
Issues found across earlier commits 2a55beb…5c3e6ee (250 commits) — 1 low
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
|
WHAT:
Adds a UDP input for CEF log ingestion to the existing Gigamon AMI
data stream alongside the existing HTTP Endpoint (JSON) input.
WHY:
Gigamon AMI supports two output formats - JSON over HTTP and CEF over
UDP. The existing package only supported JSON. This change adds CEF
support within the same data stream so both inputs share the same
index, ECS mappings, and dashboards.