-
Notifications
You must be signed in to change notification settings - Fork 2.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
Receive all logs if no attributes are specified #37721
base: main
Are you sure you want to change the base?
Conversation
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.
Looks nice overall, should have a doc update as well, and a test.
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Sorry, haven't gotten around to writing the tests and doc changes. Hope to do so soon. |
@dehaansa Should be ready for full test and review. It's my first time contributing code to this repo, who's approval do I need to get CI running? (I see another reviewer as well as an assignee). |
Any approver or maintainer can approve CI when approval is required. I've approved them - looks like you've got some linter issues to resolve before this is ready to merge, otherwise looking good! |
5a9c302
to
4a281f7
Compare
Description
I'm proposing a new convention where if the attributes field is empty, then it indicates we want to ingest everything that is sent. This helps users avoid having to modify which fields they want in two places. Happy to submit a PR myself. Please check out the linked issue and let me know if you approve or want any changes. Then I will finish implementing whatever else is necessary (e.g docs, tests).
Link to tracking issue
Fixes #37720
Testing
I added a
TestEmptyAttributes()
test to verify the behavior inlogs_test.go
Documentation
I documented the new behavior in the
README.md
and added a separate config example of how it can be used