Skip to content

feat: add Vault input plugin#10198

Merged
powersj merged 10 commits intoinfluxdata:masterfrom
efbar:feat-add-vault-input-plugin
Dec 10, 2021
Merged

feat: add Vault input plugin#10198
powersj merged 10 commits intoinfluxdata:masterfrom
efbar:feat-add-vault-input-plugin

Conversation

@efbar
Copy link
Copy Markdown
Contributor

@efbar efbar commented Dec 1, 2021

Required for all PRs:

This PR adds Hashicorp Vault input plugin. It is not so much different from Nomad input plugin here:
#10106

There are a couple of things that differs. An authentication/authorization through a token is mandatory and some structs have different fields.

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Dec 1, 2021
srebhan
srebhan reviewed Dec 1, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

This already looks quite nice. Thanks for this contribution @efbar! I have some small comments, the most important being the check for the tags as the assertion will panic if interface changes on the server side.

@efbar efbar requested a review from srebhan December 1, 2021 11:54
@efbar
Copy link
Copy Markdown
Contributor Author

efbar commented Dec 1, 2021

Thanks for the review @srebhan ! Let me know if you notice something else

srebhan
srebhan reviewed Dec 1, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

@efbar, I have some more comments, mostly suggestions. I recommend to rename the token things and make the error messages more specific, the rest can be ignored if you like the current style better. :-)

@srebhan srebhan self-assigned this Dec 1, 2021
@srebhan srebhan added the plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins label Dec 1, 2021
@efbar
Copy link
Copy Markdown
Contributor Author

efbar commented Dec 1, 2021

Thanks again @srebhan, I've modified that metrics creation part too!

@efbar efbar requested a review from srebhan December 1, 2021 15:15
srebhan
srebhan reviewed Dec 2, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Nice. Some more simplifications but that's really the last iteration... ;-) This plugin is a super nice example of how to do it. Thanks for that!

@efbar
Copy link
Copy Markdown
Contributor Author

efbar commented Dec 2, 2021

You're right @srebhan ! There was another error handling to fix too, so now it should be right 😄

@efbar efbar requested a review from srebhan December 2, 2021 14:06
srebhan
srebhan reviewed Dec 2, 2021
View reviewed changes
plugins/inputs/vault/vault.go Outdated
Comment on lines +114 to +119
err = buildVaultMetrics(acc, sysMetrics)
if err != nil {
return err
}

return nil
Copy link
Copy Markdown
Member

@srebhan srebhan Dec 2, 2021

Choose a reason for hiding this comment

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

Probably the linter will complain here...

Suggested change
err = buildVaultMetrics(acc, sysMetrics)
if err != nil {
return err
}
return nil
return buildVaultMetrics(acc, sysMetrics)

@efbar
Copy link
Copy Markdown
Contributor Author

efbar commented Dec 3, 2021

Good call @srebhan! Just updated it

@telegraf-tiger
Copy link
Copy Markdown
Contributor

telegraf-tiger bot commented Dec 3, 2021

🥳 This pull request decreases the Telegraf binary size by -1.37 % for linux amd64 (new size: 131.5 MB, nightly size 133.3 MB)

📦 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 freebsd_amd64.tar.gz windows_i386.zip
armel.deb armv6hl.rpm freebsd_armv7.tar.gz
armhf.deb i386.rpm freebsd_i386.tar.gz
i386.deb ppc64le.rpm linux_amd64.tar.gz
mips.deb s390x.rpm linux_arm64.tar.gz
mipsel.deb x86_64.rpm linux_armel.tar.gz
ppc64el.deb linux_armhf.tar.gz
s390x.deb linux_i386.tar.gz
linux_mips.tar.gz
linux_mipsel.tar.gz
linux_ppc64le.tar.gz
linux_s390x.tar.gz
static_linux_amd64.tar.gz

@efbar efbar requested a review from srebhan December 3, 2021 09:41
srebhan
srebhan approved these changes Dec 3, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Looks perfect to me. Thanks @efbar!

@srebhan srebhan 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 Dec 3, 2021
powersj
powersj approved these changes Dec 10, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

This is awesome, thank you!

@powersj powersj merged commit 039c968 into influxdata:master Dec 10, 2021
@efbar
Copy link
Copy Markdown
Contributor Author

efbar commented Dec 10, 2021

It’s my pleasure, @powersj !

powersj pushed a commit to powersj/telegraf that referenced this pull request Jan 21, 2022
@efbar @powersj
feat: add Vault input plugin (influxdata#10198)
8eb7e2e
phemmer added a commit to phemmer/telegraf that referenced this pull request Feb 18, 2022
@phemmer
Merge remote-tracking branch 'origin/master' into postgres
ddb5575 
* origin/master: (133 commits)
  chore: restart service if it is already running and upgraded via RPM (influxdata#9970)
  feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10237)
  fix: Handle duplicate registration of protocol-buffer files gracefully. (influxdata#10188)
  fix(http_listener_v2): fix panic on close (influxdata#10132)
  feat: add Vault input plugin (influxdata#10198)
  feat: support aws managed service for prometheus (influxdata#10202)
  fix: Make telegraf compile on Windows with golang 1.16.2 (influxdata#10246)
  Update changelog
  feat: Modbus add per-request tags (influxdata#10231)
  fix: Implement NaN and inf handling for elasticsearch output (influxdata#10196)
  feat: add nomad input plugin (influxdata#10106)
  fix: Print loaded plugins and deprecations for once and test (influxdata#10205)
  fix: eliminate MIB dependency for ifname processor (influxdata#10214)
  feat: Optimize locking for SNMP MIBs loading. (influxdata#10206)
  feat: Add SMART plugin concurrency configuration option, nvme-cli v1.14+ support and lint fixes. (influxdata#10150)
  feat: update configs (influxdata#10236)
  fix(inputs/kube_inventory): set TLS server name config properly (influxdata#9975)
  fix: Sudden close of Telegraf caused by OPC UA input plugin (influxdata#10230)
  fix: bump github.com/eclipse/paho.mqtt.golang from 1.3.0 to 1.3.5 (influxdata#9913)
  fix: json_v2 parser timestamp setting (influxdata#10221)
  fix: ensure graylog spec fields not prefixed with '_' (influxdata#10209)
  docs: remove duplicate links in CONTRIBUTING.md (influxdata#10218)
  fix: pool detection and metrics gathering for ZFS >= 2.1.x (influxdata#10099)
  fix: parallelism fix for ifname processor (influxdata#10007)
  chore: Forbids "log" package only for aggregators, inputs, outputs, parsers and processors (influxdata#10191)
  docs: address documentation gap when running telegraf in k8s (influxdata#10215)
  feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10211)
  fix: mqtt topic extracting no longer requires all three fields (influxdata#10208)
  fix: windows service - graceful shutdown of telegraf (influxdata#9616)
  feat: update etc/telegraf.conf and etc/telegraf_windows.conf (influxdata#10201)
  feat: Modbus support multiple slaves (gateway feature) (influxdata#9279)
  fix: Revert unintented corruption of the Makefile from influxdata#10200. (influxdata#10203)
  chore: remove triggering update-config bot in CI (influxdata#10195)
  Update changelog
  feat: Implement deprecation infrastructure (influxdata#10200)
  fix: extra lock on init for safety (influxdata#10199)
  fix: resolve influxdata#10027 (influxdata#10112)
  fix: register bigquery to output plugins influxdata#10177 (influxdata#10178)
  fix: sysstat use unique temp file vs hard-coded (influxdata#10165)
  refactor: snmp to use gosmi (influxdata#9518)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srebhan srebhan srebhan approved these changes

+1 more reviewer

@powersj powersj powersj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@srebhan srebhan

Labels

feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@efbar @powersj @srebhan