feat(loki.source.syslog): support cisco-specific syslog fields#5165
feat(loki.source.syslog): support cisco-specific syslog fields#5165
Conversation
|
💻 Deploy preview available (feat(loki.source.syslog): cisco IOS fields parsing): |
|
💻 Deploy preview deleted (feat(loki.source.syslog): support cisco-specific syslog fields). |
There was a problem hiding this comment.
Pull request overview
This PR extends the loki.source.syslog component to support parsing Cisco IOS-specific syslog fields by exposing configuration options from the underlying go-syslog library. This enables parsing of non-standard Cisco extensions to RFC3164 syslog messages, such as message counters, sequence numbers, hostname fields, and millisecond-precision timestamps.
Key changes:
- Added
rfc3164_cisco_componentsconfiguration block with options to enable parsing of Cisco-specific fields - Implemented validation to ensure the configuration is only used with RFC3164 format
- Added experimental feature gating for the new functionality
- Updated parser implementation to pass Cisco component options through the parsing pipeline
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
internal/loki/promtail/scrapeconfig/config.go |
Added SyslogRFC3164CiscoComponents struct for Cisco component configuration |
internal/converter/internal/promtailconvert/internal/build/syslog.go |
Updated converter to map Cisco component options from Promtail to Alloy format |
internal/component/loki/source/syslog/types_test.go |
Added comprehensive validation tests for Cisco components configuration |
internal/component/loki/source/syslog/types.go |
Implemented RFC3164CiscoComponents type with validation logic |
internal/component/loki/source/syslog/syslog_test.go |
Refactored experimental feature tests to include Cisco components |
internal/component/loki/source/syslog/syslog.go |
Added experimental feature gate check for Cisco components |
internal/component/loki/source/syslog/internal/syslogtarget/transport.go |
Refactored to use StreamParseConfig and pass Cisco component options to parser |
internal/component/loki/source/syslog/internal/syslogtarget/syslogtarget_test.go |
Added integration tests for RFC3164 Cisco component parsing |
internal/component/loki/source/syslog/internal/syslogtarget/syslogtarget.go |
Added label handling for Cisco-specific message counter and sequence fields |
internal/component/loki/source/syslog/internal/syslogtarget/syslogparser/syslogparser_test.go |
Updated tests to use new StreamParseConfig structure |
internal/component/loki/source/syslog/internal/syslogtarget/syslogparser/syslogparser.go |
Introduced StreamParseConfig struct and Cisco component option handling |
internal/component/loki/source/syslog/config/config.go |
Added RFC3164CiscoComponents struct for component-level configuration |
docs/sources/reference/components/loki/loki.source.syslog.md |
Added comprehensive documentation for Cisco components with device configuration examples and limitations |
internal/component/loki/source/syslog/internal/syslogtarget/syslogtarget_test.go
Outdated
Show resolved
Hide resolved
|
|
||
| isEmpty := !sc.Hostname && !sc.MessageCounter && !sc.SecondFractions && !sc.SequenceNumber | ||
| if isEmpty { | ||
| return errors.New("at least one option in rfc3164_cisco_components has to be enabled") |
There was a problem hiding this comment.
Could we tweak this error to better align with either of the user actions that are required? They must either enable one of these options or set enable_all to true.
Do we have precedence in Alloy for the enable_all behavior like this? I know for something like the unix exporter, we have a default set of things that get used if nothing is provided, which can be overridden in a couple of different ways.
If we can, it would be great to provide a set of sensible defaults for users. I don't have the domain knowledge to know if "all" of them being on by default would be appropriate, but perhaps something to consider.
There was a problem hiding this comment.
Do we have precedence in Alloy for the enable_all behavior like this? I know for something like the unix exporter, we have a default set of things that get used if nothing is provided, which can be overridden in a couple of different ways.
Yes, enable_all takes priority over all other flags:
parseCfg.RFC3164CiscoComponents = &syslogparser.RFC3164CiscoComponents{
MessageCounter: ciscoCfg.EnableAll || ciscoCfg.MessageCounter,
SequenceNumber: ciscoCfg.EnableAll || ciscoCfg.SequenceNumber,
CiscoHostname: ciscoCfg.EnableAll || ciscoCfg.Hostname,
SecondFractions: ciscoCfg.EnableAll || ciscoCfg.SecondFractions,
}If we can, it would be great to provide a set of sensible defaults for users. I don't have the domain knowledge to know if "all" of them being on by default would be appropriate, but perhaps something to consider.
The problem is that parser settings are very config-specific and should match the IOS configuration (this mentioned in the parser docs):
Important: Your parser configuration must match your Cisco device configuration. The parser cannot auto-detect which components are present because they share similar formats (mostly digits followed by colon).
There was a problem hiding this comment.
As per our discussion on Slack, I understand this better and think this aligns well with the device configuration semantics. enable_all is a convenience for users who use everything in the supported set, and if this set becomes large in the future, we can always revisit the way these are specified.
2b882cb to
73b8004
Compare
73b8004 to
8362ca8
Compare
8362ca8 to
4463604
Compare
This is a followup PR to a [previous PR](#5140) to extend `loki.source.syslog` component to enable parsing IOS-specific syslog fields. This PR exposes existing cisco-specific go-syslog [parser options](https://github.com/leodido/go-syslog/tree/develop/rfc3164#cisco-device-configuration) to component arguments.
This is a followup PR to a [previous PR](#5140) to extend `loki.source.syslog` component to enable parsing IOS-specific syslog fields. This PR exposes existing cisco-specific go-syslog [parser options](https://github.com/leodido/go-syslog/tree/develop/rfc3164#cisco-device-configuration) to component arguments.
This is a followup PR to a [previous PR](#5140) to extend `loki.source.syslog` component to enable parsing IOS-specific syslog fields. This PR exposes existing cisco-specific go-syslog [parser options](https://github.com/leodido/go-syslog/tree/develop/rfc3164#cisco-device-configuration) to component arguments.
PR Description
Which issue(s) this PR fixes
This is a followup PR to a previous PR to extend
loki.source.syslogcomponent to enable parsing IOS-specific syslog fields.This PR exposes existing cisco-specific go-syslog parser options to component arguments.
Notes to the Reviewer
PR Checklist
BEGIN_COMMIT_OVERRIDE
feat(loki.source.syslog): Support cisco-specific syslog fields (#5165)
END_COMMIT_OVERRIDE