-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
chore(deps): Bump github.com/gwos/tcg/sdk from v8.7.2 to v8.8.0 #15947
Conversation
This package implements telegraf.Logger adapter for slog.Logger used in library. It's used to output library messages with Telegraf logger. |
5058565
to
f94db3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for you work @ymkins! I think you should tailor the implementation of the adapter to the concrete use-case here. Furthermore, I'm not convinced that logging library-internal messages with their log-level is the right approach. I would log all library-internal messages as level Trace
to allow used to silence them as usually those should not be interesting for anyone...
Update the implementation of the adapter for address review comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update @ymkins! Some more comments, with the most sever being the request to not work-around logging in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymkins thanks for clarifying my questions, just two small requests and a suggestion to silence the linter warning...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymkins the code looks good, but I really urge you to just Init()
the plugin instead of manually injecting stuff as this will not test for any errors when setting up logging in Init()
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome @ymkins! Thanks for all your effort and patience!
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
…uxdata#15947) Co-authored-by: Pavlo Sumkin <[email protected]> Co-authored-by: VladislavSenkevich <[email protected]>
Co-authored-by: Pavlo Sumkin <[email protected]> Co-authored-by: VladislavSenkevich <[email protected]> (cherry picked from commit ed6d8ae)
Summary
[outputs.groundwork] update SDK
TCG SDK that provides client used for sending data to Groundwork Monitoring has updated.
Modern version got fixes and features. Internal logger was changed to
slog
for simplify integrations.PR provides telegraf.Logger adapter for slog.Logger used in SDK. It allows to output SDK messages with Telegraf logger.
Also PR provides improvement to filter out non-UTF symbols in payloads.
Checklist
Related issues
resolves #15945