-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[receiver/gelfreceiver] Added Support for GELF log receiver. #33858
base: main
Are you sure you want to change the base?
[receiver/gelfreceiver] Added Support for GELF log receiver. #33858
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.
In order to accept a new component you must formally propose it in an issue and find a sponsor.
0db0868
to
d187f9d
Compare
@djaglowski I don't have a sponsor, is it possible for you to help with that part? or is there a process to get sponsor, then please share the readme/doc for it? I know about the list of sponsors mentioned here: Link - but how do I ask someone to become sponsor/reviewer for this PR? I guess it's voluntary so I cannot really ask randomly. Also there's a Slack thread running for it, we can communicate there as well: thread link |
I don't have capacity to voluntarily sponsor any additional components. Hopefully someone else is interested. Otherwise, you can always host the component elsewhere and others can still build it into their collectors using the Collector Builder. |
@djaglowski Instead of creating a separate receiver, if I add this as a part of UDP receiver then will that be okay in terms of sponsorship as you're the contributor? GELF will be like a CODEC. |
I don't believe GELF support should be a concern of the UDP receiver. |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
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.
I don't see any tests, could you please add some to ensure the receiver works as expected?
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
@hughesjj I tried talking to folks in SIG otel-collector meeting, they said that we need sponsor for this PR and that it won't be accepted unless there's someone sponsoring this, so before I spend more effort on it - wanted to know if you would be able to sponsor this PR or get this accepted given that all comments are fixed? I don't want my efforts to go into vain. 😞 (Also I didn't realize there were more comments, sorry for the late response, missed notifications on it.) |
} | ||
|
||
if c.ListenAddress == "" { | ||
return nil, fmt.Errorf("missing required parameter 'listen_address'") |
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.
return nil, fmt.Errorf("missing required parameter 'listen_address'") | |
return nil, errors.New("missing required parameter 'listen_address'") |
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.
I am not sure but if you check ../udp/config.go, they are using fmt...I didn't wanted to diverge from it, let me know your thoughts.
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
3c69e2d
to
6578c19
Compare
771b589
to
c9942ee
Compare
Let me know if there's anything else! |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
8ec9c96
to
ef3b178
Compare
Description:
Added support for GELF based logs - please read this document to understand GELF implementation.
Tremor's implementation for GELF receiver
Otel-collector's GELF implementation document: https://docs.google.com/document/d/1P9JcrXrORqI0wsnRgrcUroluutf3rVrcUCIgLeSpFm0/edit?usp=sharing
Link to tracking Issue: #33861
Testing:
Documentation: Work-in-progress.