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 Loki as a log exporter (first PR: overall structure) #1900

Merged
merged 11 commits into from
Jan 3, 2021
Merged

Add Loki as a log exporter (first PR: overall structure) #1900

merged 11 commits into from
Jan 3, 2021

Conversation

gramidt
Copy link
Member

@gramidt gramidt commented Dec 24, 2020

Description:
Allow exporting of log data to Loki.

Link to tracking Issue:
#1894

Testing:
Unit tests are included

Documentation:
The documentation includes a short description and the configuration.

@codecov
Copy link

codecov bot commented Dec 24, 2020

Codecov Report

Merging #1900 (0ddec14) into master (fe1b43a) will increase coverage by 0.01%.
The diff coverage is 95.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1900      +/-   ##
==========================================
+ Coverage   89.90%   89.91%   +0.01%     
==========================================
  Files         380      382       +2     
  Lines       18335    18383      +48     
==========================================
+ Hits        16484    16529      +45     
- Misses       1385     1387       +2     
- Partials      466      467       +1     
Flag Coverage Δ
integration 69.88% <ø> (+0.06%) ⬆️
unit 88.62% <95.83%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
exporter/lokiexporter/loki.go 86.66% <86.66%> (ø)
exporter/lokiexporter/factory.go 100.00% <100.00%> (ø)
processor/groupbytraceprocessor/event.go 95.96% <0.00%> (-0.81%) ⬇️

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 fe1b43a...0ddec14. Read the comment docs.

@gramidt gramidt changed the title Add Loki as a log exporter Add Loki as a log exporter (first PR: overall structure) Dec 24, 2020
@gramidt gramidt marked this pull request as ready for review December 25, 2020 00:24
@gramidt gramidt requested a review from a team December 25, 2020 00:24
@gramidt
Copy link
Member Author

gramidt commented Dec 28, 2020

@open-telemetry/collector-contrib-approvers @anuraaga - Looking forward to working with you and getting Loki added as an exporter. Per the contributing docs, the second PR will include the implementation, and the third will add it to the binary.

@clhain
Copy link

clhain commented Dec 28, 2020

LGTM

exporter/lokiexporter/README.md Outdated Show resolved Hide resolved
exporter/lokiexporter/README.md Outdated Show resolved Hide resolved
exporter/lokiexporter/README.md Show resolved Hide resolved
exporter/lokiexporter/README.md Outdated Show resolved Hide resolved
exporter/lokiexporter/README.md Outdated Show resolved Hide resolved
exporter/lokiexporter/loki.go Outdated Show resolved Hide resolved
@gramidt
Copy link
Member Author

gramidt commented Dec 29, 2020

Thank you so much for reviewing, @anuraaga! I have addressed your comments and look forward to your review.

@gramidt gramidt requested a review from anuraaga December 29, 2020 02:19
Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Couple more readme nits but LGTM, thanks

exporter/lokiexporter/README.md Outdated Show resolved Hide resolved
exporter/lokiexporter/README.md Outdated Show resolved Hide resolved
@gramidt
Copy link
Member Author

gramidt commented Dec 29, 2020

Thank you so much, @anuraaga!

@gramidt
Copy link
Member Author

gramidt commented Dec 29, 2020

@anuraaga - Are you authorized to merge this?

@gramidt
Copy link
Member Author

gramidt commented Dec 31, 2020

What are the next steps to getting this merged, @anuraaga?

@anuraaga
Copy link
Contributor

anuraaga commented Jan 1, 2021

@gramidt We need a maintainer to merge this after the initial review. I guess people are on holidays so we may need to wait until next week.

@gramidt
Copy link
Member Author

gramidt commented Jan 1, 2021

Sounds good! Thank you again for all of your help, @anuraaga!

@bogdandrutu bogdandrutu merged commit 4068517 into open-telemetry:master Jan 3, 2021
@bogdandrutu
Copy link
Member

@gramidt consider to add an entry in the codeowners for this component as well as for the dependabot

@gramidt
Copy link
Member Author

gramidt commented Jan 3, 2021

Thank you so much for your help in getting this merged, @bogdandrutu! I'll be sure to commit the entrees for the codeowners.

dyladan referenced this pull request in dynatrace-oss-contrib/opentelemetry-collector-contrib Jan 29, 2021
Bumps [github.com/jaegertracing/jaeger](https://github.com/jaegertracing/jaeger) from 1.19.2 to 1.20.0.
- [Release notes](https://github.com/jaegertracing/jaeger/releases)
- [Changelog](https://github.com/jaegertracing/jaeger/blob/master/CHANGELOG.md)
- [Commits](jaegertracing/jaeger@v1.19.2...v1.20.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* Remove the Tracer method from the Span API

* Update CHANGELOG with changes

* Update CHANGELOG.md

* Fix misspell

* Address feedback
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.

4 participants