feat(inputs.powerdns_recursor): Support for new PowerDNS recursor control protocol #9633

OrfeasZ · 2021-08-16T19:40:03Z

Required for all PRs:

Updated associated README.md.
Wrote appropriate unit tests.
Pull request title or commits are in conventional commit format (e.g. feat: or fix:)

Since version 4.5.0, the PowerDNS recursor control protocol was updated to include a 32-bit status code before each request and response. This commit adds a new configurable option (new_control_protocol) to the PowerDNS Recursor input plugin that can be enabled to allow telegraf to properly communicate with newer servers. These changes preserve backwards compatibility with existing configurations.

fixes: #12252
fixes: #9494

Since version 4.5.0, the PowerDNS recursor control protocol was updated to include a 32-bit status code before each request and response. This commit adds a new configurable option (`new_control_protocol`) that can be enabled to allow telegraf to properly communicate with newer servers.

telegraf-tiger · 2021-08-16T19:40:07Z

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

OrfeasZ · 2021-08-16T19:42:02Z

!signed-cla

telegraf-tiger · 2021-08-16T20:08:36Z

Looks like new artifacts were built from this PR. Get them here!

Artifact URLs

ajoergensen · 2022-01-06T11:21:01Z

Are there any plans to merge the PR into release?

With PowerDNS Recursor v4.6 the error message is different, but I believe it's the same issue

[inputs.powerdns_recursor] Error in plugin: dial unixgram /run/pdns-recursor/pdns_recursor_telegraf2715021145323247347->/run/pdns-recursor/pdns_recursor.controlsocket: connect: protocol wrong type for socket

powersj · 2022-01-06T14:39:29Z

Are there any plans to merge the PR into release?

I resolved the conflict, however, tests and linters are failing.

Once tests are passing, artifacts will attach to this PR which you can download and use. If you could confirm those artifacts resolve the issues on newer versions of PowerDNS Recusrsor that would be very helpful.

OrfeasZ · 2022-01-06T14:54:08Z

@powersj I've pushed some fixes for the tests and linters, and everything should pass checks as expected now 🤞. I currently can't test on PDNS 4.6 but if @ajoergensen can once this is built it would be great!

telegraf-tiger · 2022-01-06T15:07:55Z

👍 This pull request doesn't change the Telegraf binary size

📦 Looks like new artifacts were built from this PR.

Expand this list to get them here ! 🐯

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_i386.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

ajoergensen · 2022-01-06T15:47:10Z

I still get the same error:

telegraf[83421]: 2022-01-06T15:44:20Z E! [inputs.powerdns_recursor] Error in plugin: dial unixgram /run/pdns-recursor/pdns_recursor_telegraf4605115996352163493->/run/pdns-recursor/pdns_recursor.controlsocket: connect: protocol wrong type for socket

Config looks like this

[[inputs.powerdns_recursor]]
    unix_sockets = [ "/run/pdns-recursor/pdns_recursor.controlsocket" ]
    socket_dir = "/run/pdns-recursor"
    socket_mode = "0666"
    new_control_protocol = true

I will try and debug more tomorrow.

ajoergensen · 2022-01-07T10:49:52Z

I have installed the 1.22.0 snapshot on all my PowerDNS Recursor servers; on those running 4.5.x I am now getting data again, so the fix is working as expected.

On my test servers (running 4.6.0) I get the same error as mentioned before:

Jan 07 11:41:20 dnsserver telegraf[13743]: 2022-01-07T10:41:20Z E! [inputs.powerdns_recursor] Error in plugin: dial unixgram /run/pdns-recursor/pdns_recursor_telegraf5411160192932929100->/run/pdns-recursor/pdns_recursor.controlsocket: connect: protocol wrong type for socket

I'm not sure how to debug the issue.

OrfeasZ · 2022-01-07T17:09:38Z

This probably means that they changed something in the protocol again. We'll be updating our servers to 4.6.0 sometime soon so I'll be debugging this then, but until then I can't offer any tips.

OrfeasZ · 2022-01-07T17:11:43Z

Actually, looking at the changelog for 4.6.0 I came across this which would explain this issue.

dmgeurts · 2022-04-21T12:05:14Z

Has any progress been made on this or can Telegraf not poll PowerDNS (v4.7) via unix_socket?

vquie · 2022-04-29T10:52:48Z

Too bad this hasn't made any progress yet.
I switched to the prometheus plugin that queries http://localhost:8082/metrics for the metrics. Though I had to rebuild all dashboards I can now update independently of this plugin.

powersj · 2022-07-11T14:24:50Z

@OrfeasZ I see that you have rebased on master. Is this ready to go? The last conversation with @ajoergensen made it seem that there was still development to do? Thanks!

OrfeasZ · 2022-07-11T14:26:05Z

It is not, I just updated my copy. Currently working on adding support for the new protocol.

OrfeasZ · 2022-07-11T23:39:01Z

This is basically ready but I have only tested it locally. We'll roll it out gradually on our infrastructure and I'll report back / send patches if there's any issues as that happens.

OrfeasZ · 2022-07-11T23:45:39Z

I should note that, in attempting to maintain backwards-compatibility with all versions (including the one this PR was originally targetting), the original description is no longer valid. The newly introduced variable is now control_protocol_version, with accepted values as seen below:

PowerDNS Version	Protocol Version
<4.5.0	`1` (default)
4.5.0 - 4.5.9	`2`
>=4.6.0	`3`

These versions are arbitrary since this protocol is internal and the powerdns devs don't version it.

powersj

@OrfeasZ huge thank you for doing this PR.

I have thrown in some suggestions that would be nice to see you take a look at. If you don't have time, let us know and I can see about taking this PR over.

Thanks!

plugins/inputs/powerdns_recursor/README.md

plugins/inputs/powerdns_recursor/protocol_commons.go

plugins/inputs/powerdns_recursor/sample.conf

plugins/inputs/powerdns_recursor/powerdns_recursor_test.go

Co-authored-by: Joshua Powers <powersj@fastmail.com>

Provide a better explanation of version compatibility and how / when to adjust it.

OrfeasZ · 2022-11-17T12:46:58Z

@powersj this is good to go from my side 👍

powersj

Thanks for the changes!

srebhan

Thanks for the update @OrfeasZ! Two small requests in the code and one suggestion, then we are good to go I think.

plugins/inputs/powerdns_recursor/protocol_commons.go

plugins/inputs/powerdns_recursor/sample.conf

Should prevent panics in the case PowerDNS updates their implementation and returns data not conforming to what we expect.

OrfeasZ · 2022-11-25T19:54:06Z

@srebhan Made the changes you requested. Let me know what you think about the endianness thing above!

srebhan

Besides the host endianness issue the code looks good. I will discuss with @powersj on how we should proceed on that. Thanks for your contribution @OrfeasZ!

srebhan

@OrfeasZ nice update. Can you please make the one if-else block a switch so we can land this today!?

plugins/inputs/powerdns_recursor/protocol_commons.go

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

OrfeasZ · 2022-12-12T16:43:37Z

@srebhan done

telegraf-tiger · 2022-12-12T17:00:33Z

Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Aug 16, 2021

OrfeasZ mentioned this pull request Aug 16, 2021

Pdns_recursor plugin fails collection with I/O timeout #9494

Closed

style: fix coding style of powerdns_recursor code

efd2dda

Merge branch 'master' into master

2a0609d

OrfeasZ added 3 commits January 6, 2022 16:45

fix: switch to using require instead of assert for pdns recursor test

76e65fb

fix: resolve lint errors in pdns recursor plugin readme

b1a728a

fix: ignore unneeded lint check in pdns recursor plugin test

dc16fd1

DasSkelett mentioned this pull request Jun 16, 2022

Use Prometheus API to collect PowerDNS Recursor stats freifunkMUC/ffmuc-salt-public#110

Merged

Merge remote-tracking branch 'upstream/master'

62e2faf

OrfeasZ added 3 commits July 11, 2022 22:50

Add support for the new PowerDNS Recursor control protocol

fd322d0

Try to silence the linter

4c6868a

Provide additional safety against misuse

7c52f21

powersj mentioned this pull request Nov 16, 2022

Telegraf getting "protocol wrong type for socket" error while trying to connect to powerdns-recursor socket #12252

Closed

powersj reopened this Nov 16, 2022

powersj reviewed Nov 16, 2022

View reviewed changes

OrfeasZ and others added 7 commits November 17, 2022 13:38

Apply suggestions from code review

3b1be25

Co-authored-by: Joshua Powers <powersj@fastmail.com>

Merge branch 'influxdata:master' into master

9f506d2

Update powerdns recursor plugin readme

6506d93

Provide a better explanation of version compatibility and how / when to adjust it.

Update powerdns recursor versions in sample config

7649148

Remove unneeded return value from writeNativeUIntToConn

f680797

Remove trailing readme spaces

eb30430

Remove trailing space

a553d66

powersj approved these changes Nov 17, 2022

View reviewed changes

powersj added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Nov 17, 2022

srebhan reviewed Nov 21, 2022

View reviewed changes

plugins/inputs/powerdns_recursor/protocol_commons.go Outdated Show resolved Hide resolved

plugins/inputs/powerdns_recursor/protocol_commons.go Outdated Show resolved Hide resolved

plugins/inputs/powerdns_recursor/sample.conf Outdated Show resolved Hide resolved

OrfeasZ added 2 commits November 25, 2022 21:52

Add length check to powerdns response parsing

974511a

Should prevent panics in the case PowerDNS updates their implementation and returns data not conforming to what we expect.

Add trailing newline to powerdns recursor sample config

9da0a14

OrfeasZ requested a review from srebhan November 25, 2022 19:54

OrfeasZ added 2 commits November 25, 2022 21:57

Update docs

6b103a5

Reformat code

5f4233c

srebhan reviewed Nov 30, 2022

View reviewed changes

srebhan mentioned this pull request Dec 8, 2022

chore: Move host-endianness definition to internal #12360

Merged

3 tasks

OrfeasZ added 2 commits December 10, 2022 16:09

Merge branch 'influxdata:master' into master

e9fe9c9

Switch to internal host endianness variable

03f6624

powersj requested a review from srebhan December 12, 2022 15:28

srebhan reviewed Dec 12, 2022

View reviewed changes

plugins/inputs/powerdns_recursor/protocol_commons.go Outdated Show resolved Hide resolved

Refactor if block into switch case

b8a4e6c

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

srebhan merged commit 459a658 into influxdata:master Dec 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(inputs.powerdns_recursor): Support for new PowerDNS recursor control protocol #9633

feat(inputs.powerdns_recursor): Support for new PowerDNS recursor control protocol #9633

OrfeasZ commented Aug 16, 2021 •

edited by powersj

telegraf-tiger bot commented Aug 16, 2021

OrfeasZ commented Aug 16, 2021

telegraf-tiger bot commented Aug 16, 2021

Artifact URLs

ajoergensen commented Jan 6, 2022

powersj commented Jan 6, 2022

OrfeasZ commented Jan 6, 2022

telegraf-tiger bot commented Jan 6, 2022

Artifact URLs

ajoergensen commented Jan 6, 2022

ajoergensen commented Jan 7, 2022

OrfeasZ commented Jan 7, 2022

OrfeasZ commented Jan 7, 2022

dmgeurts commented Apr 21, 2022

vquie commented Apr 29, 2022

powersj commented Jul 11, 2022

OrfeasZ commented Jul 11, 2022

OrfeasZ commented Jul 11, 2022

OrfeasZ commented Jul 11, 2022 •

edited

powersj left a comment

OrfeasZ commented Nov 17, 2022

powersj left a comment

srebhan left a comment

OrfeasZ commented Nov 25, 2022

srebhan left a comment

srebhan left a comment

OrfeasZ commented Dec 12, 2022

telegraf-tiger bot commented Dec 12, 2022

Artifact URLs

feat(inputs.powerdns_recursor): Support for new PowerDNS recursor control protocol #9633

feat(inputs.powerdns_recursor): Support for new PowerDNS recursor control protocol #9633

Conversation

OrfeasZ commented Aug 16, 2021 • edited by powersj

Required for all PRs:

telegraf-tiger bot commented Aug 16, 2021

OrfeasZ commented Aug 16, 2021

telegraf-tiger bot commented Aug 16, 2021

Artifact URLs

ajoergensen commented Jan 6, 2022

powersj commented Jan 6, 2022

OrfeasZ commented Jan 6, 2022

telegraf-tiger bot commented Jan 6, 2022

Artifact URLs

ajoergensen commented Jan 6, 2022

ajoergensen commented Jan 7, 2022

OrfeasZ commented Jan 7, 2022

OrfeasZ commented Jan 7, 2022

dmgeurts commented Apr 21, 2022

vquie commented Apr 29, 2022

powersj commented Jul 11, 2022

OrfeasZ commented Jul 11, 2022

OrfeasZ commented Jul 11, 2022

OrfeasZ commented Jul 11, 2022 • edited

powersj left a comment

Choose a reason for hiding this comment

OrfeasZ commented Nov 17, 2022

powersj left a comment

Choose a reason for hiding this comment

srebhan left a comment

Choose a reason for hiding this comment

OrfeasZ commented Nov 25, 2022

srebhan left a comment

Choose a reason for hiding this comment

srebhan left a comment

Choose a reason for hiding this comment

OrfeasZ commented Dec 12, 2022

telegraf-tiger bot commented Dec 12, 2022

Artifact URLs

OrfeasZ commented Aug 16, 2021 •

edited by powersj

OrfeasZ commented Jul 11, 2022 •

edited