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

Fix storing of Gauge metrics #299

Merged
merged 2 commits into from
Nov 15, 2018
Merged

Fix storing of Gauge metrics #299

merged 2 commits into from
Nov 15, 2018

Conversation

ukclivecox
Copy link
Contributor

No description provided.

@ukclivecox ukclivecox requested a review from jklaise November 15, 2018 14:24
@ukclivecox ukclivecox changed the title WIP: Fix storing of Gauge metrics Fix storing of Gauge metrics Nov 15, 2018
@ukclivecox ukclivecox merged commit b406c23 into SeldonIO:master Nov 15, 2018
@ukclivecox ukclivecox deleted the custom_metric_fixes branch February 14, 2020 11:39
agrski added a commit that referenced this pull request Dec 2, 2022
* Add k8s server version & node count metrics

* Add loop & per-call limit for k8s node counting to avoid excessive memory usage

* Update go.mod & go.sum

* Add kustomization for RBAC permissions

* Rename field in collector struct for clarity

* Make k8s client a field in the collector

* Refactor logger source field creation to start of collector initialiser

* Use cleaner way of accessing kind-specific k8s clientsets

* Upgrade Go to 1.18 in Hodometer

* Use k8s client-go interface instead of concrete clientset type

* Add basic test for k8s metrics' collection

* Return k8s metrics from collector method

* Add logging to k8s metrics collection method

* Replace existing k8s metrics test with server version case

Also explain why original test case was unfeasible, without significant additional work.
This previous case was passing due to the bug of not returning k8s metrics from the collection method.

* Move node-fetching per-call limit to package-level constant from function-level

This simplifies testing by allowing reference to this constant.

* Add test case for non-zero node count

* Fix bug breaking loop before incrementing running total

* Add given/when/then explanations to k8s metrics test

* Update go.mod & go.sum

* Add test case for more nodes than per-call limit

Also add logic for mock clientset object to support this test in a meaningful way.

* Add test case for k8s node retrieval throwing errors

* Refactor mock call setup to functions for legibility in k8s metrics test

* Add test case for nil k8s client

* Fix bug causing segfault on nil k8s client

* Return interface from function not concrete type

This is a result of Go's handling of interfaces.
An interface is only nil if it is assigned a nil from a variable of the interface type.
If it is assigned from a concrete type, the interface is non-nil, even though it's wrapped value is indeed nil.

* Check nil-ness of interface using reflection

This is a bit ugly, but as this is not a performance-critical section of code,
using reflection is a tolerable overhead.

* Fix typos in RBAC manifests - rename meta -> metadata

* Elevate role & role-binding to cluster-level resources

* Add errors descriptions into error logs in collector

* Use concrete type for clientset in k8s metrics tests

* Rename namespace in k8s RBAC config

* Specify Go version (1.18) for container images

* Add error description in k8s client creation for collector

* Update error messages

* Refactor k8s metrics collection to per-metric functions

This also clarifies the semantics of a metric update, thereby simplifying error handling and making it consistent.

* Bump Hodometer version
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.

1 participant