-
Notifications
You must be signed in to change notification settings - Fork 265
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
Add the otelcol tcplog receiver #2701
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.
Thank you! It looks good. I only added a few minor comments.
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
Thanks for the review! Updated the PR based on your feedback. For the max log size stuff - I pulled all of that out of the syslog receiver, so it might be worth going and making the same changes there as well at some point. |
Hmmm I can't get the test that's failing in CI to fail locally no matter how many times I run it. I didn't copy this sleep over from the equivalent syslog test because that PR had been merged and it was passing locally. Do you think that would help? |
We attempted to remove the test after that PR was merged and it still failed in CI, so it might be relevant here as well. Haven't had the opportunity to go back and wait for the port to be ready in a better way than just the 1s wait. |
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.
Most doc suggestions are style related to bring this new topic in-line with the new/upcoming markdown styles and information flow.
A few minor content corrections were included.
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
docs/sources/reference/components/otelcol/otelcol.receiver.tcplog.md
Outdated
Show resolved
Hide resolved
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.
Apologies for the delay with the review! I added just one minor comment. It'll also need a rebase with main
and it would be ready for merging!
242ef13
to
bdbe244
Compare
I'm sorry, the |
Co-authored-by: Paulin Todev <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
…log.md Co-authored-by: Paulin Todev <[email protected]>
bdbe244
to
b340dd1
Compare
No problem - rebased and updated to 0.119 |
We should consider adding This was not done when syslogreceiver was added due to some issues I had with the implementation, but resolved when adding filelog. I'm intending to add it to syslog before next release, so I can add it to tcplog later as well if you don't have capacity to add it in with this PR. |
If you don't mind adding it in afterwards, I would appreciate that |
The build failure looks like a transient 502 from yarn. I don't think I have permission to retrigger the builds |
PR Description
This PR adds support for the otelcol tcp log receiver. I am not much of a golang programmer, so this PR was largely adapted from the one that added the syslog receiver.
Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist