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 kubelet stats receiver #237

Merged
merged 8 commits into from
May 26, 2020
Merged

Add kubelet stats receiver #237

merged 8 commits into from
May 26, 2020

Conversation

pmcollins
Copy link
Member

Please see README for details.

@pmcollins pmcollins requested a review from a team as a code owner May 15, 2020 17:42
receiver/kubeletstatsreceiver/kubelet/client.go Outdated Show resolved Hide resolved
receiver/kubeletstatsreceiver/README.md Outdated Show resolved Hide resolved
func (f Factory) CreateDefaultConfig() configmodels.Receiver {
return &Config{
ReceiverSettings: configmodels.ReceiverSettings{
TypeVal: typeStr, // TODO who tf uses this?
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not except for tests that expect CreateDefaultConfig() to be equal to loading an empty receiver from a config file. I'm trying to get rid of it open-telemetry/opentelemetry-collector#971

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, thanks. (this was meant be a comment for myself which I forgot to remove)

receiver/kubeletstatsreceiver/kubelet/client.go Outdated Show resolved Hide resolved
receiver/kubeletstatsreceiver/kubelet/client.go Outdated Show resolved Hide resolved
receiver/kubeletstatsreceiver/kubelet/client.go Outdated Show resolved Hide resolved
receiver/kubeletstatsreceiver/kubelet/network.go Outdated Show resolved Hide resolved
receiver/kubeletstatsreceiver/kubelet/fs.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented May 19, 2020

Codecov Report

Merging #237 into master will increase coverage by 0.14%.
The diff coverage is 80.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #237      +/-   ##
==========================================
+ Coverage   77.84%   77.99%   +0.14%     
==========================================
  Files         126      142      +16     
  Lines        6528     6862     +334     
==========================================
+ Hits         5082     5352     +270     
- Misses       1166     1218      +52     
- Partials      280      292      +12     
Impacted Files Coverage Δ
receiver/kubeletstatsreceiver/kubelet/cert.go 0.00% <0.00%> (ø)
receiver/kubeletstatsreceiver/kubelet/client.go 51.21% <51.21%> (ø)
receiver/kubeletstatsreceiver/factory.go 58.97% <58.97%> (ø)
...ver/kubeletstatsreceiver/kubelet/stats_provider.go 63.63% <63.63%> (ø)
receiver/kubeletstatsreceiver/receiver.go 81.81% <81.81%> (ø)
receiver/kubeletstatsreceiver/kubelet/cpu.go 84.61% <84.61%> (ø)
receiver/kubeletstatsreceiver/runnable.go 86.95% <86.95%> (ø)
receiver/kubeletstatsreceiver/kubelet/pb.go 91.66% <91.66%> (ø)
...ceiver/kubeletstatsreceiver/kubelet/accumulator.go 100.00% <100.00%> (ø)
receiver/kubeletstatsreceiver/kubelet/fs.go 100.00% <100.00%> (ø)
... and 23 more

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 95efa1b...ff89f35. Read the comment docs.

@tigrannajaryan tigrannajaryan added this to Review in progress in Collector May 19, 2020
@tigrannajaryan tigrannajaryan self-assigned this May 20, 2020
go.sum Outdated
@@ -922,6 +923,8 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J
github.com/onsi/gomega v1.8.1/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA=
github.com/onsi/gomega v1.9.0 h1:R1uwffexN6Pr340GtYRIdZmAiN4J+iw6WG4wog1DUXg=
github.com/onsi/gomega v1.9.0/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA=
github.com/open-telemetry/opentelemetry-collector v0.3.1-0.20200424155504-9d16f5971ef9 h1:maf2tPAv8QV17gSAePuzREqKvConQ+ozOkNM0pFgbyA=
Copy link
Member

Choose a reason for hiding this comment

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

Please rebase from master and run make gotidy

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, thanks.

Comment on lines 31 to 33
ca_cert_path: "/path/to/ca.crt"
client_key_path: "/path/to/apiserver.key"
client_cert_path: "/path/to/apiserver.crt"
Copy link
Member

Choose a reason for hiding this comment

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

There is an upcoming change that uses different key names for client configs: https://github.com/open-telemetry/opentelemetry-collector/pull/988/files/405db7ad13c8feeaeb65142081b04b48e407eaf6#diff-b99ea1d4f9c67b0e6a0dc0199de54d66
It may be useful for you to sync with @ccaraman on this. For now you can probably just use the same field names to avoid breaking changes later once @ccaraman change is merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using the same field names for now. Thanks.

cfg configmodels.Receiver,
nextConsumer consumer.TraceConsumerOld,
) (component.TraceReceiver, error) {
return nil, nil
Copy link
Member

Choose a reason for hiding this comment

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

This should return configerror.ErrDataTypeIsNotSupported.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

Pablo Collins and others added 6 commits May 20, 2020 18:06
@tigrannajaryan
Copy link
Member

@pmcollins please fix the coverage checks.

@tigrannajaryan
Copy link
Member

@pmcollins please fix the build.

Code coverage was failing the previous version because of lack of tests
of client code.

Previous client implementation was copied from smart agent with several
modifications and was not as easy to test since it did network and file
I/O. This change makes I/O code replaceable making Client easier to
test.

Additionally, ServiceAccount auth has been removed because I wasn't able
to end-to-end test it due to Minikube config issues. It will be added
in a separate PR.
Collector automation moved this from Review in progress to Reviewer approved May 26, 2020
@tigrannajaryan tigrannajaryan merged commit 9b27241 into open-telemetry:master May 26, 2020
Collector automation moved this from Reviewer approved to Done May 26, 2020
wyTrivail referenced this pull request in mxiamxia/opentelemetry-collector-contrib Jul 13, 2020
The Kubelet Stats Receiver pulls pod metrics from the API server on a kubelet
and sends it down the metric pipeline for further processing.
mxiamxia referenced this pull request in mxiamxia/opentelemetry-collector-contrib Jul 22, 2020
* Remove extraneous space in docker command
* Change filename to match sample in /examples/demotrace/
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* Jaeger exporter should be its own go module open-telemetry/opentelemetry-go#205

* fix review comments and build #205

* resolve mod conflicts #205
bogdandrutu pushed a commit that referenced this pull request May 12, 2022
Bumps [github.com/antonmedv/expr](https://github.com/antonmedv/expr) from 1.8.9 to 1.9.0.
- [Release notes](https://github.com/antonmedv/expr/releases)
- [Commits](expr-lang/expr@v1.8.9...v1.9.0)

---
updated-dependencies:
- dependency-name: github.com/antonmedv/expr
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

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

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants