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(processors.snmp_lookup): New SNMP lookup processor #14223

Merged
merged 71 commits into from
Feb 27, 2024

Conversation

Hipska
Copy link
Contributor

@Hipska Hipska commented Oct 31, 2023

Required for all PRs

Add a new, more generic SNMP lookup processor compared to the ifname processor.

@telegraf-tiger telegraf-tiger bot added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/processor labels Oct 31, 2023
…nmp_lookup

* upstream/master:
  chore(linters): Fix findings found by testifylint: expected-actual (influxdata#14229)
  chore(deps): Bump github.com/nats-io/nkeys from 0.4.5 to 0.4.6 (influxdata#14225)
  feat(inputs.procstat)!: Remove useless zero cpu_times (influxdata#14224)
  feat(inputs.gnmi): Rework plugin (influxdata#14091)
  fix(outputs.timestream): Clip uint64 values (influxdata#14213)
  fix(inputs.cgroup): Escape backslashes (influxdata#14187)
  test(outputs.kafka): Use private network for testing (influxdata#14220)
  test(inputs.vault): Fix integration test by only testing for subset (influxdata#14222)
  fix(outputs.elasticsearch): Print error status value (influxdata#14115)
@Hipska Hipska force-pushed the feat/processors/snmp_lookup branch from f4d43f0 to 4df6fee Compare November 6, 2023 17:07
@Hipska Hipska added this to the v1.29.0 milestone Nov 7, 2023
@Hipska Hipska marked this pull request as ready for review November 7, 2023 12:14
@Hipska Hipska marked this pull request as draft November 7, 2023 12:14
* master:
  fix(inputs.win_perf_counter): Do not rely on returned buffer size (influxdata#14241)
  feat(inputs.modbus): Add support for string-fields (influxdata#14145)
  chore(deps): Bump cloud.google.com/go/storage from 1.30.1 to 1.34.1 (influxdata#14253)
  chore(deps): Bump github.com/rabbitmq/amqp091-go from 1.8.1 to 1.9.0 (influxdata#14252)
  chore(deps): Bump github.com/hashicorp/consul/api from 1.25.1 to 1.26.1 (influxdata#14251)
  chore(deps): Bump github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs from 1.23.5 to 1.26.0 (influxdata#14249)
  fix(config): Fix comment removal in TOML files (influxdata#14240)
  feat(outputs.prometheus_client): Support listen on vsock (influxdata#14108)
  fix(inputs.mqtt_consumer): Resolve could not mark message delivered (influxdata#14243)
  chore(linters): Fix findings found by testifylint for Windows and enable it. (influxdata#14238)
  feat(migrations): Add option migration for inputs.nats_consumer (influxdata#14234)
  feat(migrations): Add option migration for inputs.mqtt_consumer (influxdata#14233)
  test(inputs.jolokia2_agent): Sort metrics as order is not consistent (influxdata#14227)
@Hipska Hipska force-pushed the feat/processors/snmp_lookup branch from 5da46b7 to 2e0ed1a Compare November 9, 2023 13:09
@Hipska Hipska marked this pull request as ready for review November 9, 2023 17:08
@powersj
Copy link
Contributor

powersj commented Feb 15, 2024

My understanding is the next step was the refactoring PR you recently put up. We can take a look shortly.

@Hipska
Copy link
Contributor Author

Hipska commented Feb 23, 2024

@srebhan @powersj I think this is ready for a final review. I have it now running fine on DEV with 5 instances of the plugin.

@Hipska
Copy link
Contributor Author

Hipska commented Feb 26, 2024

@srebhan @powersj Please, there's only 2 weeks left for next feature release.

Copy link
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 good @Hipska. Please move the deprecation of ifname to another PR together with a proper migration and leave it out here.

plugins/processors/deprecations.go Outdated Show resolved Hide resolved
plugins/processors/ifname/README.md Outdated Show resolved Hide resolved
@Hipska
Copy link
Contributor Author

Hipska commented Feb 27, 2024

Done, moved deprecation to #14899.

Copy link
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.

Thanks @Hipska!

@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 Feb 27, 2024
@srebhan srebhan assigned powersj and DStrand1 and unassigned srebhan Feb 27, 2024
Copy link
Member

@DStrand1 DStrand1 left a comment

Choose a reason for hiding this comment

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

Thanks @Hipska! Just one small thing

plugins/processors/snmp_lookup/README.md Outdated Show resolved Hide resolved
@DStrand1 DStrand1 removed their assignment Feb 27, 2024
Copy link
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.

I'm +1 once Dane's suggestion lands.

Co-authored-by: Dane Strandboge <136023093+DStrand1@users.noreply.github.com>
@Hipska Hipska requested a review from powersj February 27, 2024 16:04
@powersj powersj merged commit a855165 into influxdata:master Feb 27, 2024
8 of 15 checks passed
@github-actions github-actions bot added this to the v1.30.0 milestone Feb 27, 2024
@Hipska Hipska deleted the feat/processors/snmp_lookup branch February 27, 2024 16:20
@telegraf-tiger
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/snmp feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/processor 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
Development

Successfully merging this pull request may close these issues.

4 participants