Skip to content

Fix kubeletstats logging and self reported metrics #357

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

Merged
merged 3 commits into from
Jul 8, 2020
Merged

Fix kubeletstats logging and self reported metrics #357

merged 3 commits into from
Jul 8, 2020

Conversation

pmcollins
Copy link
Member

Kubeletstats receiver was missing log statements on important errors and self-reported metrics was not working in lab.

Tested self-reported metrics by looking at prometheus endpoint at 8888.

Kubeletstats receiver was missing log statements on
important errors.
@pmcollins pmcollins requested a review from a team June 24, 2020 17:37
@codecov
Copy link

codecov bot commented Jun 24, 2020

Codecov Report

Merging #357 into master will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #357      +/-   ##
==========================================
+ Coverage   82.78%   82.83%   +0.04%     
==========================================
  Files         170      170              
  Lines        9070     9075       +5     
==========================================
+ Hits         7509     7517       +8     
+ Misses       1236     1234       -2     
+ Partials      325      324       -1     
Flag Coverage Δ
#integration 0.00% <ø> (ø)
#unit 82.83% <100.00%> (+0.04%) ⬆️
Impacted Files Coverage Δ
receiver/kubeletstatsreceiver/receiver.go 81.81% <100.00%> (ø)
receiver/kubeletstatsreceiver/runnable.go 100.00% <100.00%> (+13.04%) ⬆️

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 a00790d...f2f9b50. Read the comment docs.

const transport = "http"
ctx := obsreport.StartMetricsReceiveOp(r.ctx, dataformat, transport)
ctx := obsreport.ReceiverContext(r.ctx, typeStr, transport, r.receiverName)
ctx = obsreport.StartMetricsReceiveOp(ctx, typeStr, transport)
Copy link
Member

Choose a reason for hiding this comment

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

Ifaik StartMetricsReceiveOp calls must always match with EndMetricsReceiveOp. So we shouldn't put EndMetricsReceiveOp under a condition. I don't think we need StartMetricsReceiveOp call here. We probably only need only one StartMetricsReceiveOp call and one EndMetricsReceiveOp call in this function, both of them should wrap err = r.consumer.ConsumeMetricsData(ctx, *md) in every iteration of the loop over MetricsData.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, will fix.

@bogdandrutu bogdandrutu merged commit 97e5200 into open-telemetry:master Jul 8, 2020
@bogdandrutu bogdandrutu deleted the ks-obsreport branch July 8, 2020 16:06
wyTrivail referenced this pull request in mxiamxia/opentelemetry-collector-contrib Jul 13, 2020
* Add logging on errors in Runnable

Kubeletstats receiver was missing log statements on
important errors.

* Fix self reported metrics

* Fix obsreport calls
mxiamxia referenced this pull request in mxiamxia/opentelemetry-collector-contrib Jul 22, 2020
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
bogdandrutu pushed a commit that referenced this pull request May 12, 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.

3 participants