loki icon indicating copy to clipboard operation
loki copied to clipboard

feat(promtail): Support of RFC3164 aka BSD Syslog

Open catap opened this issue 1 year ago • 17 comments

What this PR does / why we need it:

This PR introduce support of RFC3164 at promtail with additional hacks to support old and wired implementations.

It includes different BSD system such as OpenBSD and some embeded system. Frankly speaking all of them that I have access: from OpenBSD server to Unifi UA and IP Socket and Synology NAS with Nginx, works with this changes.

Both protocol TCP and UDP works.

It based on https://github.com/grafana/loki/pull/7845 and requries some changes inside used go-syslog library which was accomulated as single PR https://github.com/influxdata/go-syslog/pull/55 but upstream of that library seems to be not so active.

It is tested with config like:

server:
  disable: true

positions:
  filename: /tmp/promtail-syslog-positions.yml

clients:
  - url: http://127.0.0.1:3100/loki/api/v1/push

scrape_configs:
  - job_name: syslog
    syslog:
      listen_address: 127.0.0.1:1514
      listen_protocol: tcp # or udp
      is_rfc3164_message: true
      labels:
        job: syslog
    relabel_configs:
      - source_labels: [__syslog_message_severity]
        target_label: level
      - source_labels: [__syslog_message_facility]
        target_label: facility
      - source_labels: [__syslog_connection_hostname]
        target_label: host
      - source_labels: [__syslog_connection_ip_address]
        target_label: ip
      - source_labels: [__syslog_message_hostname]
        target_label: hostname
      - source_labels: [__syslog_message_app_name]
        target_label: app
      - source_labels: [__syslog_message_proc_id]
        target_label: proc
      - source_labels: [__syslog_message_msg_id]
        target_label: msgid

Which issue(s) this PR fixes: A few times was made quite clear that such things aren't supported. So, no issue were opened.

Checklist

  • [x] Reviewed the CONTRIBUTING.md guide (required)
  • [x] Documentation added
  • [x] Tests updated
  • [x] Title matches the required conventional commits format, see here

catap avatar Apr 28 '24 10:04 catap

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 28 '24 10:04 CLAassistant

hey @catap we're technically calling promtail feature complete but I think we can make a case for this really being a fix rather than being a feature if promtail didn't work with BSD syslog before, though we would definitely want to wait for your changes to be upstreamed via https://github.com/influxdata/go-syslog/pull/55 first

cstyan avatar Apr 30 '24 00:04 cstyan

Hey @cstyan!

Yes, BSD syslog were nuke when support of syslog was introduced at https://github.com/grafana/loki/pull/1275 since when was one attempt https://github.com/grafana/loki/pull/7845 to do it which I've reused.

As part of that attempt the PR: https://github.com/influxdata/go-syslog/pull/46 with required changes inside go-syslog were created but nobody had reviewed it.

It was December 2022.

I doubt that my PR will be merged some time soon, let be honest.

But this changes which is localized inside not used before code allows me to introduce support of different devices which sends logs to Loki. From BSD system to Nginx with Unifi Networks and so on.

So, what's a plan here? I see that https://github.com/grafana/go-syslog exists and I may open PR into this repository, but as far as I see nobody uses it.

catap avatar Apr 30 '24 01:04 catap

Just a rebase to the last main.

catap avatar Apr 30 '24 14:04 catap

Hey @cstyan!

Yes, BSD syslog were nuke when support of syslog was introduced at #1275 since when was one attempt #7845 to do it which I've reused.

As part of that attempt the PR: influxdata/go-syslog#46 with required changes inside go-syslog were created but nobody had reviewed it.

It was December 2022.

I doubt that my PR will be merged some time soon, let be honest.

But this changes which is localized inside not used before code allows me to introduce support of different devices which sends logs to Loki. From BSD system to Nginx with Unifi Networks and so on.

So, what's a plan here? I see that https://github.com/grafana/go-syslog exists and I may open PR into this repository, but as far as I see nobody uses it.

Unfortunately I don't have any suggestions here at the moment. Given we're winding down our own work on promtail and (to my knowledge) don't use the syslog target ourselves I don't really want to take on maintenance of this code within the Loki repo. It's likely better to ask the Alloy team to take on maintenance of the code either directly or via a fork, though I doubt that will happen anytime soon either. They have their own open issue tracking support for the same RFC: https://github.com/grafana/alloy/issues/305

My only other suggestion would be opening an issue on the go-syslog repo itself asking if the repo is unmaintained and trying to get in contact with someone from Influx who has worked on it in the past. It's likely it would also get no response, or a response of "yes, unmaintained". In either case, hopefully that would entice someone else in the Go community to take on maintenance of a fork.

cstyan avatar Apr 30 '24 20:04 cstyan

Unfortunately I don't have any suggestions here at the moment. Given we're winding down our own work on promtail and (to my knowledge) don't use the syslog target ourselves I don't really want to take on maintenance of this code within the Loki repo. It's likely better to ask the Alloy team to take on maintenance of the code either directly or via a fork, though I doubt that will happen anytime soon either. They have their own open issue tracking support for the same RFC: grafana/alloy#305

My only other suggestion would be opening an issue on the go-syslog repo itself asking if the repo is unmaintained and trying to get in contact with someone from Influx who has worked on it in the past. It's likely it would also get no response, or a response of "yes, unmaintained". In either case, hopefully that would entice someone else in the Go community to take on maintenance of a fork.

From another hand: do you see any objection to merge? If this code will be removed soon enough, when you decided to nuke promtail, why not keep it with this feature?

It doesn't change any existed code, just add a new one feature which should be enabled by hand.

Upstream project looks like to be forgetten to be archived, but this code increases use case of loki right now, that should attract new users for Loki.

catap avatar May 01 '24 07:05 catap

From another hand: do you see any objection to merge? If this code will be removed soon enough, when you decided to nuke promtail, why not keep it with this feature?

Yes, because we're not just getting rid of promtail. It's going to be maintained with security and bug fixes for (I'm just trying to make a reasonable guess here) a year or two if not more.

What we are not doing is supporting additional features. We could make an argument for this being a fix but the actual changes you're making to go-syslog would need to be present in the upstream repo or a fork that someone is committed to maintaining (which we would then need to switch to using instead of influx's repo).

cstyan avatar May 01 '24 15:05 cstyan

What we are not doing is supporting additional features. We could make an argument for this being a fix but the actual changes you're making to go-syslog would need to be present in the upstream repo or a fork that someone is committed to maintaining (which we would then need to switch to using instead of influx's repo).

I see your point and I haven't got any resources to promise you to support go-syslog for next couple of years.

Frankly speaking after xz-history I understand why you decided to go this way.

But, as I said before, I really think that this PR will benefite Lokie and Grafana ecosystem.

Maybe as better approach you may tag someone from Alloy team? I had quite a fast look to their code and it seems like a fork of promtail.

catap avatar May 01 '24 21:05 catap

I see your point and I haven't got any resources to promise you to support go-syslog for next couple of years. But, as I said before, I really think that this PR will benefite Lokie and Grafana ecosystem.

I don't disagree.

Maybe as better approach you may tag someone from Alloy team?

We can ping them like this @grafana/grafana-alloy-maintainers, but we also had a discussion internally, the consensus was that we don't have the bandwidth to maintain go-syslog ourselves and finding another solution is low priority at the moment.

I had quite a fast look to their code and it seems like a fork of promtail.

It's a hard fork of some of promtails code, yes.

cstyan avatar May 01 '24 22:05 cstyan

Maybe as better approach you may tag someone from Alloy team?

We can ping them like this @grafana/grafana-alloy-maintainers, but we also had a discussion internally, the consensus was that we don't have the bandwidth to maintain go-syslog ourselves and finding another solution is low priority at the moment.

Seems that github included comma into name of group and decided to not ping them. So, here I'm to ping: @grafana/grafana-alloy-maintainers

catap avatar May 03 '24 15:05 catap

@cstyan here an update. @leodido had forked his go-syslog which he decided to continue support, see his response here: https://github.com/influxdata/go-syslog/issues/56#issuecomment-2119171616

After that he had merged the PR, and here I'm back with reduced PR which:

  • switch go-syslog to an actual upstream (and the last release);
  • enable support of old syslog at promtail.

catap avatar May 20 '24 19:05 catap

@cstyan here an update. @leodido had forked his go-syslog which he decided to continue support, see his response here: influxdata/go-syslog#56 (comment)

That's great news :+1:

cstyan avatar May 20 '24 21:05 cstyan

Yup, I decided to keep maintaining it at https://github.com/leodido/go-syslog With write access it's way more easy to maintain stuff :) You can already go get github.com/leodido/go-syslog/v4 (v4.1.0 is the latest atm)

leodido avatar May 21 '24 08:05 leodido

@cstyan to make review easy, I've added all remarks as dedicated commit. Feel free to ask, if I need to squash it.

catap avatar May 22 '24 22:05 catap

okay, I think we're probably good to go

I'll take another look in the morning

cstyan avatar May 23 '24 00:05 cstyan

@cstyan may I ask you to re-run tests, this time with go fmt!

catap avatar May 23 '24 00:05 catap

@cstyan ping

catap avatar May 28 '24 17:05 catap