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

add syslog receiver support #2482

Merged
merged 28 commits into from
Mar 16, 2021

Conversation

wph95
Copy link
Member

@wph95 wph95 commented Feb 28, 2021

Description: Add Syslog receiver
Link to tracking Issue: #2331

Testing: Unit tests are included

Documentation: Syslog receiver README.md is included

@wph95 wph95 requested a review from djaglowski as a code owner February 28, 2021 14:39
@wph95 wph95 requested a review from a team February 28, 2021 14:39
@wph95
Copy link
Member Author

wph95 commented Mar 1, 2021

opentelemetry-log-collector syslog parser has a bug
open-telemetry/opentelemetry-log-collection#31
wait for it fixed

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great. A few comments, and one more blocker on log-collection repo.

receiver/syslogreceiver/doc.go Outdated Show resolved Hide resolved
receiver/syslogreceiver/README.md Outdated Show resolved Hide resolved
receiver/syslogreceiver/README.md Outdated Show resolved Hide resolved
receiver/syslogreceiver/syslog.go Outdated Show resolved Hide resolved
receiver/syslogreceiver/syslog.go Outdated Show resolved Hide resolved
receiver/syslogreceiver/syslog_test.go Outdated Show resolved Hide resolved
receiver/syslogreceiver/syslog_test.go Show resolved Hide resolved
@tigrannajaryan
Copy link
Member

@wph95 thank you for working on this. I will assign the issue to you. Please remember to claim issues if you want to work on them in the future.

We will also need a perf test for syslog receiver. If you do not plan on implement a perf test in this PR please create a separate issue for that.

@wph95
Copy link
Member Author

wph95 commented Mar 2, 2021

@wph95 thank you for working on this. I will assign the issue to you. Please remember to claim issues if you want to work on them in the future.

We will also need a perf test for syslog receiver. If you do not plan on implement a perf test in this PR please create a separate issue for that.

thx Tigran, next time will claim issues first :)
will create a perf test issue for syslog receiver

@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #2482 (dfa5708) into main (fcbe766) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2482      +/-   ##
==========================================
+ Coverage   91.48%   91.51%   +0.02%     
==========================================
  Files         440      441       +1     
  Lines       21864    21887      +23     
==========================================
+ Hits        20002    20029      +27     
+ Misses       1393     1390       -3     
+ Partials      469      468       -1     
Flag Coverage Δ
integration 69.18% <ø> (ø)
unit 90.43% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
receiver/syslogreceiver/syslog.go 100.00% <100.00%> (ø)
receiver/carbonreceiver/transport/tcp_server.go 66.00% <0.00%> (-1.00%) ⬇️
processor/groupbytraceprocessor/event.go 96.77% <0.00%> (+0.80%) ⬆️
...eiver/awsxrayreceiver/internal/udppoller/poller.go 100.00% <0.00%> (+2.32%) ⬆️
receiver/k8sclusterreceiver/watcher.go 97.64% <0.00%> (+2.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcbe766...dfa5708. Read the comment docs.

@wph95 wph95 requested a review from djaglowski March 3, 2021 16:49
@djaglowski
Copy link
Member

@wph95 The latest release of opentelemetry-log-collection will require some minor changes to this code.

On this PR, I think we only need to change the docs from labels to attributes.

@wph95
Copy link
Member Author

wph95 commented Mar 10, 2021

@wph95 The latest release of opentelemetry-log-collection will require some minor changes to this code.

On this PR, I think we only need to change the docs from labels to attributes.

changed docs, from labels to attributes. and clean the code.
You could re-review it :)

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Thanks @wph95!

receiver/syslogreceiver/README.md Show resolved Hide resolved
receiver/syslogreceiver/syslog.go Show resolved Hide resolved
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @wph95. I believe this is ready to go.

@bogdandrutu
Copy link
Member

/cc @tigrannajaryan

@tigrannajaryan
Copy link
Member

Thank you @wph95 !

@tigrannajaryan tigrannajaryan merged commit 030675b into open-telemetry:main Mar 16, 2021
pmatyjasek-sumo referenced this pull request in pmatyjasek-sumo/opentelemetry-collector-contrib Apr 28, 2021
Add Syslog receiver
**Link to tracking Issue:** #2331 

**Testing:** Unit tests are included

**Documentation:** Syslog receiver README.md is included
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
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.

5 participants