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

smartagent: refactor logrus shim and support monitorID field #1885

Merged
merged 1 commit into from
Aug 17, 2022

Conversation

rmfitzpatrick
Copy link
Contributor

@rmfitzpatrick rmfitzpatrick commented Aug 16, 2022

These changes adopt the monitorID field inclusion provided by signalfx/signalfx-agent#2334 as well as refactor the existing logrusToZap logic:

  1. Relocate all unredirect logic from the logrus shim to the tests since we should never stop listening to a monitor's logger just because the parent receiver has been stopped. Afaict this was originally added for the tests to pass.
  2. Remove redundant default zap logger creation since we should always be using the actual service logger. Rewriting the name field with default is my proposed change.
  3. General var naming and comments for clarity since the current functionality is a bit cryptic imo.*
  4. Improve some test practices, for var reference bungs, and handle missing fatal/panic level cases.

@rmfitzpatrick rmfitzpatrick requested review from a team as code owners August 16, 2022 16:18
@rmfitzpatrick rmfitzpatrick force-pushed the addmonitoridtologrusshim branch from 67171df to a1d1207 Compare August 16, 2022 16:30
@rmfitzpatrick rmfitzpatrick force-pushed the addmonitoridtologrusshim branch from a1d1207 to 182d28a Compare August 16, 2022 16:32
@rmfitzpatrick rmfitzpatrick merged commit 1b21f59 into main Aug 17, 2022
@delete-merged-branch delete-merged-branch bot deleted the addmonitoridtologrusshim branch August 17, 2022 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants