Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add the otelcol syslog receiver #2263

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

dehaansa
Copy link
Contributor

PR Description

While adding the otelcol.exporter.syslog component, I discovered that there was not a clean way to proxy syslog using the loki.source.syslog component as a source. For the purposes of syslog proxying, the otelcol.receiver.syslog component emits logs in the exact syntax expected by the exporter.

This does not add support for enhancing parsing with the numerous stanza operators available in upstream opentelemetry. Most or all of the use cases that would be supported by those can also be implemented through processors like the otelcol.processor.transform component, and the effort to add them in seemed significant for no known customer requirements at this time.

Which issue(s) this PR fixes

Related to #312

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@dehaansa dehaansa requested review from clayton-cornell and a team as code owners December 11, 2024 18:37
@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Dec 11, 2024
The `location` argument specifies a Time Zone identifier. The available locations depend on the local IANA Time Zone database.
See [this wikipedia entry][tz-wiki] for a non-comprehensive list.

The `non_transparent_framing_trailer` argument must be one of `LF`, `NUL`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some context around this? Like how can the default be nil? Even if its a string it should be "" but that seems to not be LF or NUL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's fair. It's a *string, so it's nil if not set. If it is set, then the character chosen is expected to terminate the message. If not set, it's expected that you're using octet counting or that the syslog frame contains only one message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clayton-cornell I'd appreciate any feedback you have on this docs change as well as Matt's.

| Name | Type | Description | Default | Required |
|---------------------------------|----------|--------------------------------------------------------------------------------------------------------------|---------|----------|
| `listen_address` | `string` | The `<host:port>` address to listen to for syslog messages. | | yes |
| `max_log_size` | `int` | The maximum size of a log entry to read before failing. | `1MiB` | no |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a string.

| Name | Type | Description | Default | Required |
|--------------------|------------|-----------------------------------------------------------------------------------------------------------|--------------|----------|
| `enabled` | `bool` | If true, the receiver will pause reading a file and attempt to resend the current batch of logs on error. | `false` | no |
| `initial_interval` | `duration` | The time to wait after first failure to retry. | `1 second` | no |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Durations should be in formatted to 1s or 5m

@@ -0,0 +1,271 @@
---
canonical: https://grafana.com/docs/alloy/latest/reference/components/otelcol/otelcol.receiver.syslog/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clayton-cornell do we need to add a tag for public preview?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I missed that :-( I saw the shared lookup... forgot to make sure the badge was added.

Parallel to this, we are looking at better ways of adding badges to the docs... so we can avoid HTML spans.

}

// Values taken from tcp input Build function
const defaultMaxLogSize = helper.ByteSize(tcp.DefaultMaxLogSize)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Rename to tcpDefaultMaxLogSize

Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good aside from a few doc changes.

@dehaansa dehaansa requested a review from mattdurham December 12, 2024 15:59
Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated description makes sense when I read it.

In reviewing again, I found a couple other minor points I missed in the first pass.

@@ -0,0 +1,271 @@
---
canonical: https://grafana.com/docs/alloy/latest/reference/components/otelcol/otelcol.receiver.syslog/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I missed that :-( I saw the shared lookup... forgot to make sure the badge was added.

Parallel to this, we are looking at better ways of adding badges to the docs... so we can avoid HTML spans.

@clayton-cornell
Copy link
Contributor

Ooops double posted my review. I was logged out Git right when I submitted the review.

Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving for docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants