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.

Sign up for GitHub

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

Jump to bottom

Adapt SNMP flow component/integration to new SNMP exporter config changes #4265

Merged
merged 10 commits into from
Jul 4, 2023
Merged

Adapt SNMP flow component/integration to new SNMP exporter config changes #4265

merged 10 commits into from
Jul 4, 2023

Conversation

marctc
Copy link
Contributor

@marctc marctc commented Jun 27, 2023 •
edited
Loading

PR Description

This PR introduces several changes that affect SNMP flow component and integration:

  • Add the last version of SNMP exporter which includes a new way to configure it . This makes breaking changes in walk_params struct of the exporter removing auth and version, but at the same time allows flexibility to uses to define auth at config level.
  • Add the ability to define SNMP config as embedded yaml in SNMP flow component and integration. This is specially useful to use in Flow with local.file component.
  • Refactor some Handler method to avoid code duplications.
  • Remove dependency of custom fork to avoid registration of metrics to the global registry.

Which issue(s) this PR fixes

Fixes #3421
Fixes #3843

Notes to the Reviewer

PR Checklist

  • CHANGELOG updated
  • Documentation added
  • Tests updated

@marctc marctc requested review from a team and clayton-cornell as code owners June 27, 2023 11:00
@marctc marctc requested a review from RichiH June 27, 2023 11:02
mattdurham
mattdurham approved these changes Jun 27, 2023
View reviewed changes
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.

LGTM, this may get you both the 2023 winner for lines removed and added =)

clayton-cornell
clayton-cornell approved these changes Jun 28, 2023
View reviewed changes
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.

Docs look OK to me

@marctc marctc merged commit 6710bda into main Jul 4, 2023
@marctc marctc deleted the snmp_exporter_cfg branch July 4, 2023 10:28
clayton-cornell pushed a commit that referenced this pull request Aug 14, 2023
@marctc @clayton-cornell
Adapt SNMP flow component/integration to new SNMP exporter config cha…
90eb30c 
…nges (#4265)
clayton-cornell pushed a commit that referenced this pull request Aug 14, 2023
@marctc @clayton-cornell
Adapt SNMP flow component/integration to new SNMP exporter config cha…
e4b3c8d 
…nges (#4265)
@rgeyer rgeyer mentioned this pull request Jan 16, 2024
Merged
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 23, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@clayton-cornell clayton-cornell clayton-cornell approved these changes

@mattdurham mattdurham mattdurham approved these changes

@RichiH RichiH Awaiting requested review from RichiH

Assignees
No one assigned
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Milestone
No milestone
3 participants
@marctc @mattdurham @clayton-cornell