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

K8s Processor: logs support #1051

Merged

Conversation

pmm-sumo
Copy link
Contributor

@pmm-sumo pmm-sumo commented Sep 18, 2020

Description: Adds log support to k8sprocessor

Link to tracking Issue: open-telemetry/opentelemetry-collector#1654

Testing: unit tests cover logs use-cases, manual tests via fluentforward protocol done (and spans to confirm nothing breaks)

Documentation: updated doc.go and cleared-out (hopefully) how is the pod IP address determined

@pmm-sumo pmm-sumo requested a review from a team September 18, 2020 17:22
@codecov
Copy link

codecov bot commented Sep 18, 2020

Codecov Report

Merging #1051 into master will increase coverage by 0.01%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1051      +/-   ##
==========================================
+ Coverage   88.81%   88.83%   +0.01%     
==========================================
  Files         250      250              
  Lines       11924    11945      +21     
==========================================
+ Hits        10590    10611      +21     
  Misses        990      990              
  Partials      344      344              
Flag Coverage Δ
#integration 75.42% <ø> (ø)
#unit 87.85% <90.90%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
processor/k8sprocessor/processor.go 89.28% <66.66%> (-2.72%) ⬇️
processor/k8sprocessor/factory.go 97.33% <100.00%> (+0.66%) ⬆️
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 bff19a2...c7c8a71. Read the comment docs.

@tigrannajaryan
Copy link
Member

@keitwb @owais please review.

Copy link
Contributor

@keitwb keitwb left a comment

Choose a reason for hiding this comment

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

LGTM

@pmm-sumo
Copy link
Contributor Author

pmm-sumo commented Sep 21, 2020

While working on this PR I noticed two things:

Let me know what do you think, those should be quick to add if they make sense

@bogdandrutu
Copy link
Member

Shouldn't we move the config description from doc.go to README.md (as for other processors)? There is a closed issue on that but I am not sure if it has the originally intended outcome: #97

Using README is more consistent

Perhaps we could extract some of the hardcoded behaviour into config entries: #1077, #1078, #1079, #1080

Sounds like a good idea

@bogdandrutu bogdandrutu merged commit 7730017 into open-telemetry:master Sep 21, 2020
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