open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
2.73k stars 2.16k forks source link

[receiver/syslog] Automatically detect protocol #30328

Open llamafilm opened 6 months ago

llamafilm commented 6 months ago

Component(s)

receiver/syslog

Is your feature request related to a problem? Please describe.

Some of my syslog sender devices use RFC 5424 and others use RFC 3164 so I get errors like this:

Dec 21 07:32:38 i-0c7c2f22bd9881703 startup.sh[51677]: 2023-12-21T07:32:38.492Z error helper/transformer.go:98 Failed to process entry {"kind": "receiver", "name": "syslog", "data_type": "logs", "operator_id": "syslog_input_internal_parser", "operator_type": "syslog_parser", "error": "expecting a version value in the range 1-999 [col 4]", "action": "send", "entry": {"observed_timestamp":"2023-12-21T07:32:38.492596111Z","timestamp":"0001-01-01T00:00:00Z","body":"<30>Dec 20 23:32:38 ibootpdu iocontrold[247]: PDU_Event 3c1c1a5fEV002003[Ad-Hoc]~Group (Ad-Hoc) Cycle by admin via Web","severity":0,"scope_name":""}}

It would be nice if this receiver could automatically recognize both formats. Some devices are hardcoded to use UDP port 514 so there is no way to separate the 2 protocols on different ports.

Describe the solution you'd like

I'd like to make the protocol configuration property optional or add a new possible value of auto where it recognizes the format automatically.

Describe alternatives you've considered

I'm currently using rsyslog to parse and forward messages to otel, but it would be nice to remove that additional dependency.

Additional context

No response

github-actions[bot] commented 6 months ago

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

crobert-1 commented 6 months ago

I think the general idea makes sense.

I'm a bit concerned about the performance impact of auto-detect. Depending on how much work is necessary to determine which format the log messages are, it may end up duplicating a lot of the work. This would only be relevant when the auto-detect functionality is enabled, but it's still something to keep in mind.

Another concern I have is that this may make it harder to know if logs are coming in with the wrong format. We'd have to still log for invalid formats to make sure it's not missed.

A side effect of this is that there are a few configuration options that are only relevant to one protocol or the other. This allows the collector to validate the config more thoroughly on startup, but this proposal would potentially allow invalid configurations to run. location, enable_octet_counting, and non_transparent_framing_trailer are all protocol specific values. This doesn't break any existing functionality, but users may get unexpected results that would have been communicated sooner and more clearly without auto-detect.

An alternative to adding another config option would be to add another valid value for the protocol option called automatic or auto, something along those lines. I think if we document the behavior clearly it would make more sense than two separate configuration options that override each other.

As far as feasibility, this receiver is built on top of the stanza package, so any new functionality has to be added there. This may mean that adding this would impact other receivers or features, and is therefore not acceptable.

I'll remove needs triage for now, but we'll have to hear back from the code owner for a more complete answer.

djaglowski commented 6 months ago

I share @crobert-1's concerns about performance and protocol-specific config values.

Given the concerns, I think we should look at a proof of concept implementation before deciding whether the feature will be supported.

github-actions[bot] commented 3 months ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

llamafilm commented 3 months ago

this is still an issue

github-actions[bot] commented 1 month ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.