Conversation
✅ Elastic Docs Style Checker (Vale)No issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
…y-token-optional-02024448
|
✅ All changelog entries have the correct PR link. |
🚀 Benchmarks reportTo see the full report comment with |
💚 Build Succeeded
History
cc @haetamoudi |
|
Pinging @elastic/integration-experience (Team:Integration-Experience) |
There was a problem hiding this comment.
🔵 Needs a closer look
One or more issues must be addressed before approval.
Pull request overview
Updates the HPE Aruba CX NTP ingest pipeline to parse 1106 messages without primary, with regression coverage and release metadata.
Changes:
- Makes the
primarytoken optional in the NTP grok pattern. - Adds a failing-format fixture and expected output.
- Bumps package version to 0.4.6 and records the bugfix.
File summaries
| File | Description |
|---|---|
| packages/hpe_aruba_cx/manifest.yml | Updated as part of this pull request. |
| packages/hpe_aruba_cx/data_stream/log/elasticsearch/ingest_pipeline/default.yml | Updated as part of this pull request. |
| packages/hpe_aruba_cx/data_stream/log/_dev/test/pipeline/test-aruba-cx.log-expected.json | Updated as part of this pull request. |
| packages/hpe_aruba_cx/data_stream/log/_dev/test/pipeline/test-aruba-cx.log | Updated as part of this pull request. |
| packages/hpe_aruba_cx/changelog.yml | Updated as part of this pull request. |
Review details
Suppressed comments (2)
packages/hpe_aruba_cx/changelog.yml:2
- The PR description and implementation steps still refer to version 0.4.3, but this package already has 0.4.3 and 0.4.5 entries and the actual release bump here is 0.4.6 (also reflected in
manifest.yml). Please update the PR description/commit plan to match the release metadata so the change is not documented against an old version.
- version: "0.4.6"
packages/hpe_aruba_cx/manifest.yml:4
- The PR description's implementation steps still say this change should publish version 0.4.3 (from 0.4.2), but the package is already at 0.4.5 and this diff correctly moves it to 0.4.6 with a matching changelog entry. Please update the description to reflect the actual release version so the documented upgrade and release notes are not misleading.
version: 0.4.6
- Files reviewed: 4/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review summaryIssues found across the latest commits 1f4013d — 1 lowPackage-level:
🤖 AI-Generated Review | Vera Review Bot - v0.4.2 | 📚 Knowledge base: integration-skills
|
|
Tick the box to add this pull request to the merge queue (same as
|
Executive summary
The grok pattern for NTP events 1105/1106 in the HPE Aruba CX ingest pipeline required the literal word 'primary' before 'server connection', but real-world log messages for event code 1106 can omit that token (e.g. 'NTP server connection lost'). The fix makes 'primary' optional via a non-capturing group
(?:primary )?, allowing both variants to be parsed correctly. A regression test fixture for the previously-failing message format is also added.Proposed commit message
Root cause
The ntp_event_1105_1106 grok processor requires the literal string 'primary' in its pattern, but AOS-CX event 1106 ('connection lost') is emitted in two forms: one with 'primary' (e.g. 'NTP primary server connection lost to ...') and one without (e.g. 'NTP server connection lost to ...'). The current single pattern only covers the 'primary'-present variant, causing a grok failure whenever the 'primary'-absent form is received.
Approach
Make the literal 'primary' token optional in the ntp_event_1105_1106 grok processor by replacing it with a non-capturing optional group. Change the single pattern from '^NTP primary server connection (established|lost) to %{GREEDYDATA:server.address}' to '^NTP (?:primary )?server connection (established|lost) to %{GREEDYDATA:server.address}'. This is fully backward-compatible: existing 1105 and 1106 events that include 'primary' continue to match, and the customer-observed 1106 form ('NTP server connection lost to ...') without 'primary' now also matches. A new pipeline test fixture entry is added for the 1106-without-primary variant.
Implementation
Pipeline changes
Field / mapping changes
—
Sanitized error message
Processor 'grok' with tag 'ntp_event_1105_1106' in pipeline 'logs-hpe_aruba_cx.log-default' failed with message '[on_failure_message]'Sanitized log (
event_sanitizedexcerpt)Reviewer concerns
server.address; trailing whitespace or additional tokens in future message variants would be silently absorbed into the field value, but this matches the pre-existing pattern behavior.Self-review findings
—
Risk and classification
Links
d37eb800a5a939d9