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

feat(exporter): add collector for fetching metrics using seachest library #349

Merged
merged 11 commits into from
Dec 27, 2019

Conversation

akhilerm
Copy link
Contributor

  • added node level exporter for fetching metrics using seachest library.
  • In this PR, only the current temperature info of the disk is exposed as metrics. More metrics like disk rotation rate can be fetched from seachest library.
  • More collectors can be added by following steps in docs(exporter): steps to add a new collector #347 .

add seachest collector for getting temperature info of blockdevices

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
add yaml spec to run ndm-node-exporter as a daemonset into
operator.yaml

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
remove exposing seachest metrics for sparse devices

Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Found some fixes!

P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).

pkg/metrics/seachest/metrics.go Outdated Show resolved Hide resolved
pkg/metrics/seachest/metrics.go Outdated Show resolved Hide resolved
pkg/metrics/seachest/metrics.go Outdated Show resolved Hide resolved
pkg/metrics/seachest/metrics.go Outdated Show resolved Hide resolved
pkg/metrics/seachest/metrics.go Outdated Show resolved Hide resolved
pkg/metrics/seachest/metrics.go Outdated Show resolved Hide resolved
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@codecov-io
Copy link

codecov-io commented Nov 25, 2019

Codecov Report

Merging #349 into master will increase coverage by 0.55%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #349      +/-   ##
==========================================
+ Coverage   51.67%   52.22%   +0.55%     
==========================================
  Files          55       55              
  Lines        2601     2874     +273     
==========================================
+ Hits         1344     1501     +157     
- Misses       1159     1262     +103     
- Partials       98      111      +13
Impacted Files Coverage Δ
db/kubernetes/convert.go 57.14% <100%> (+2.14%) ⬆️
cmd/ndm_daemonset/probe/eventhandler.go 46.05% <0%> (-7.9%) ⬇️
cmd/ndm_daemonset/controller/diskstore.go 90.64% <0%> (-0.19%) ⬇️
pkg/hierarchy/device.go 0% <0%> (ø) ⬆️
cmd/ndm_daemonset/controller/blockdevicestore.go 86.18% <0%> (+0.03%) ⬆️
pkg/udev/common.go 92.1% <0%> (+0.04%) ⬆️
cmd/ndm_daemonset/probe/udevprobe.go 68.26% <0%> (+2.4%) ⬆️
pkg/controller/disk/disk_controller.go 24.32% <0%> (+4.32%) ⬆️
cmd/ndm_daemonset/controller/controller.go 22.58% <0%> (+5.1%) ⬆️
...er/blockdeviceclaim/blockdeviceclaim_controller.go 44.09% <0%> (+5.93%) ⬆️
... and 2 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 9ed7454...b4ac874. Read the comment docs.

@akhilerm akhilerm requested a review from kmova November 25, 2019 10:23
// TemperatureInformation stores the temperature information of the blockdevice
type TemperatureInformation struct {
// TemperatureDataValid specifies whether the current temperature
// data reported is valid or not
Copy link
Contributor

Choose a reason for hiding this comment

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

how do you determine if the temperature value is valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seachest gives that information. For every temperature information that seachest provides there is an associated boolean value, which specifies whether the data is valid or not


// SeachestMetricCollector contains the metrics, concurrency handler and client to get the
// metrics from seachest
type SeachestMetricCollector struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this generic. For example this is SMART metrics that we are getting and in the current implementation we are using SeaChest to get the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its more towards the library we use. We can use seachest library for some metrics. SMART for some other metrics. There will be some metrics that overlaps. But I think a library specific metric will be good, even if they overlaps. This will give the option to the user on which metric he need to choose.

If we think in angle of node exporter, there will be some disk related metrics that will be exposed by both node-exporter and NDM. But the user can choose the metric using his query, rather than we limiting the user to metrics from a certain lib only.

@kmova kmova changed the title feat(exporter): add collector for fetching metrics using seachest library WIP: feat(exporter): add collector for fetching metrics using seachest library Dec 24, 2019
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Found some fixes!

P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).

ndm-exporter/collector/seachest.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
pkg/metrics/smart/metrics.go Show resolved Hide resolved
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm akhilerm changed the title WIP: feat(exporter): add collector for fetching metrics using seachest library feat(exporter): add collector for fetching metrics using seachest library Dec 27, 2019
@akhilerm akhilerm requested a review from kmova December 27, 2019 13:03
@kmova kmova added this to the 0.5 milestone Dec 27, 2019
@kmova kmova merged commit 622d968 into openebs-archive:master Dec 27, 2019
@akhilerm akhilerm deleted the seachest-collector branch December 30, 2019 05:22
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.

None yet

3 participants