Skip to content

Conversation

luizluca
Copy link
Contributor

It is heavily based on diskstats to allow metrics reuse by simply
s/disk/tape/ the query.

Signed-off-by: Luiz Angelo Daros de Luca [email protected]

@SuperQ
Copy link
Member

SuperQ commented May 21, 2021

Thanks for the new collector. We typically implement access to proc/sys files via the Prometheus procfs library rather than directly in the node_exporter.

If you wouldn't mind implementing the file parser in the library first, it would be great to add this.

(Yes, there are some legacy collectors that still use proc/sys directly)

@luizluca
Copy link
Contributor Author

Thanks for the new collector. We typically implement access to proc/sys files via the Prometheus procfs library rather than directly in the node_exporter.

If you wouldn't mind implementing the file parser in the library first, it would be great to add this.

No problem. The collector is quite simple. It might not be a big deal.

(Yes, there are some legacy collectors that still use proc/sys directly)

As a suggestion, it would be nice to have a comment on top of every "legacy" usage with a "// TODO: migrate to procfs library". As programming is mostly a bunch of ctrl+c,ctrl+v, without a warning those codes tend to replicate.

@luizluca
Copy link
Contributor Author

Done. It requires prometheus/procfs#382

@luizluca
Copy link
Contributor Author

luizluca commented Jul 3, 2021

Rebased and solved CHANGELOG.md conflicts.
procfs side is already merged.

Copy link
Member

@discordianfish discordianfish left a comment

Choose a reason for hiding this comment

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

Nice LGTM!

@SuperQ
Copy link
Member

SuperQ commented Jul 7, 2021

CC @bluecmd

@luizluca luizluca force-pushed the tapestats_master branch 2 times, most recently from 6e65aae to 74a2835 Compare July 7, 2021 18:51
@luizluca
Copy link
Contributor Author

luizluca commented Jul 7, 2021

All changes applied. Rebased on master to use procfs v0.7.0

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM.

@SuperQ
Copy link
Member

SuperQ commented Jul 8, 2021

Actually, should we name this scsi_tape instead of tapestats to match the procfs files?

@luizluca
Copy link
Contributor Author

luizluca commented Jul 8, 2021

I though collectors abstracts the underlying OS. If someone implements tapestats for windows, FreeBSD, it would not make sense to mention a Linux specific name.

Anyway, I can rename it if needed.

@SuperQ
Copy link
Member

SuperQ commented Jul 9, 2021

Most of the naming has been based on the Linux names for things, since we typically start with Linux implementations.

tapestats is fine with me.

@bluecmd Any opinions?

ch <- prometheus.MustNewConstMetric(c.readByteTotal, prometheus.CounterValue, float64(tape.Counters.ReadByteCnt), tape.Name)
ch <- prometheus.MustNewConstMetric(c.readsCompletedTotal, prometheus.CounterValue, float64(tape.Counters.ReadCnt), tape.Name)
ch <- prometheus.MustNewConstMetric(c.readTimeSeconds, prometheus.CounterValue, float64(tape.Counters.ReadNs)*0.000000001, tape.Name)
ch <- prometheus.MustNewConstMetric(c.residualTotal, prometheus.CounterValue, float64(tape.Counters.ResidCnt)*0.000000001, tape.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this really be multiplied?

Copy link
Member

Choose a reason for hiding this comment

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

Seems like a copy-n-paste bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like a copy-n-paste bug.

Yes, it is... I fixed that. Thanks!

@bluecmd
Copy link
Contributor

bluecmd commented Jul 9, 2021

I think this looks fine (save for the one comment above), I have been using the following script for a few days to test these metrics out, and it has worked fine.

#!/bin/bash

for stp in /sys/class/scsi_tape/st[0-999]
do
  drv="$(basename "${stp}")"
  echo "node_tape_io_now{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/in_flight")"
  echo "node_tape_io_time_nanoseconds_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/io_ns")"
  echo "node_tape_io_others_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/other_cnt")"
  echo "node_tape_read_bytes_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/read_byte_cnt")"
  echo "node_tape_reads_completed_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/read_cnt")"
  echo "node_tape_read_time_nanoseconds_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/read_ns")"
  echo "node_tape_residual_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/resid_cnt")"
  echo "node_tape_written_bytes_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/write_byte_cnt")"
  echo "node_tape_writes_completed_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/write_cnt")"
  echo "node_tape_write_time_nanoseconds_total{device=\""${drv}"\"} $(<"/sys/class/scsi_tape/${drv}/stats/write_ns")"
done

Looking forward to having these metrics straight in node_exporter!

It is based on diskstats to allow metrics reuse by simply
s/disk/tape/ the query.

Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
@bluecmd
Copy link
Contributor

bluecmd commented Jul 11, 2021

@SuperQ can this be merged?

@SuperQ SuperQ merged commit 59a4f3b into prometheus:master Jul 12, 2021
@SuperQ
Copy link
Member

SuperQ commented Jul 12, 2021

Done, thanks!

SuperQ added a commit that referenced this pull request Jul 12, 2021
NOTE: Ignoring invalid network speed will be the default in 2.x
NOTE: Filesystem collector flags have been renamed. `--collector.filesystem.ignored-mount-points` is now `--collector.filesystem.mount-points-exclude` and `--collector.filesystem.ignored-fs-types` is now `--collector.filesystem.fs-types-exclude`. The old flags will be removed in 2.x.

* [CHANGE] Rename filesystem collector flags to match other collectors #2012
* [CHANGE] Make node_exporter print usage to STDOUT #2039
* [FEATURE] Add conntrack statistics metrics #1155
* [FEATURE] Add ethtool stats collector #1832
* [FEATURE] Add flag to ignore network speed if it is unknown #1989
* [FEATURE] Add tapestats collector for Linux #2044
* [ENHANCEMENT] Add ErrorLog plumbing to promhttp #1887
* [ENHANCEMENT] Add time zone offset metric #2060
* [BUGFIX] Add ErrorLog plumbing to promhttp #1887
* [BUGFIX] Handle errors from disabled PSI subsystem #1983
* [BUGFIX] Fix panic when using backwards compatible flags #2000
* [BUGFIX] Only initiate collectors once #2048
* [BUGFIX] Handle small backwards jumps in CPU idle #2067

Signed-off-by: Ben Kochie <[email protected]>
@SuperQ SuperQ mentioned this pull request Jul 12, 2021
SuperQ added a commit that referenced this pull request Jul 15, 2021
NOTE: Ignoring invalid network speed will be the default in 2.x
NOTE: Filesystem collector flags have been renamed. `--collector.filesystem.ignored-mount-points` is now `--collector.filesystem.mount-points-exclude` and `--collector.filesystem.ignored-fs-types` is now `--collector.filesystem.fs-types-exclude`. The old flags will be removed in 2.x.

* [CHANGE] Rename filesystem collector flags to match other collectors #2012
* [CHANGE] Make node_exporter print usage to STDOUT #2039
* [FEATURE] Add conntrack statistics metrics #1155
* [FEATURE] Add ethtool stats collector #1832
* [FEATURE] Add flag to ignore network speed if it is unknown #1989
* [FEATURE] Add tapestats collector for Linux #2044
* [FEATURE] Add nvme collector #2062
* [ENHANCEMENT] Add ErrorLog plumbing to promhttp #1887
* [ENHANCEMENT] Add more Infiniband counters #2019
* [ENHANCEMENT] netclass: retrieve interface names and filter before parsing #2033
* [ENHANCEMENT] Add time zone offset metric #2060
* [BUGFIX] Handle errors from disabled PSI subsystem #1983
* [BUGFIX] Fix panic when using backwards compatible flags #2000
* [BUGFIX] Fix wrong value for OpenBSD memory buffer cache #2015
* [BUGFIX] Only initiate collectors once #2048
* [BUGFIX] Handle small backwards jumps in CPU idle #2067

Signed-off-by: Ben Kochie <[email protected]>
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
NOTE: Ignoring invalid network speed will be the default in 2.x
NOTE: Filesystem collector flags have been renamed. `--collector.filesystem.ignored-mount-points` is now `--collector.filesystem.mount-points-exclude` and `--collector.filesystem.ignored-fs-types` is now `--collector.filesystem.fs-types-exclude`. The old flags will be removed in 2.x.

* [CHANGE] Rename filesystem collector flags to match other collectors prometheus#2012
* [CHANGE] Make node_exporter print usage to STDOUT prometheus#2039
* [FEATURE] Add conntrack statistics metrics prometheus#1155
* [FEATURE] Add ethtool stats collector prometheus#1832
* [FEATURE] Add flag to ignore network speed if it is unknown prometheus#1989
* [FEATURE] Add tapestats collector for Linux prometheus#2044
* [FEATURE] Add nvme collector prometheus#2062
* [ENHANCEMENT] Add ErrorLog plumbing to promhttp prometheus#1887
* [ENHANCEMENT] Add more Infiniband counters prometheus#2019
* [ENHANCEMENT] netclass: retrieve interface names and filter before parsing prometheus#2033
* [ENHANCEMENT] Add time zone offset metric prometheus#2060
* [BUGFIX] Handle errors from disabled PSI subsystem prometheus#1983
* [BUGFIX] Fix panic when using backwards compatible flags prometheus#2000
* [BUGFIX] Fix wrong value for OpenBSD memory buffer cache prometheus#2015
* [BUGFIX] Only initiate collectors once prometheus#2048
* [BUGFIX] Handle small backwards jumps in CPU idle prometheus#2067

Signed-off-by: Ben Kochie <[email protected]>
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
NOTE: Ignoring invalid network speed will be the default in 2.x
NOTE: Filesystem collector flags have been renamed. `--collector.filesystem.ignored-mount-points` is now `--collector.filesystem.mount-points-exclude` and `--collector.filesystem.ignored-fs-types` is now `--collector.filesystem.fs-types-exclude`. The old flags will be removed in 2.x.

* [CHANGE] Rename filesystem collector flags to match other collectors prometheus#2012
* [CHANGE] Make node_exporter print usage to STDOUT prometheus#2039
* [FEATURE] Add conntrack statistics metrics prometheus#1155
* [FEATURE] Add ethtool stats collector prometheus#1832
* [FEATURE] Add flag to ignore network speed if it is unknown prometheus#1989
* [FEATURE] Add tapestats collector for Linux prometheus#2044
* [FEATURE] Add nvme collector prometheus#2062
* [ENHANCEMENT] Add ErrorLog plumbing to promhttp prometheus#1887
* [ENHANCEMENT] Add more Infiniband counters prometheus#2019
* [ENHANCEMENT] netclass: retrieve interface names and filter before parsing prometheus#2033
* [ENHANCEMENT] Add time zone offset metric prometheus#2060
* [BUGFIX] Handle errors from disabled PSI subsystem prometheus#1983
* [BUGFIX] Fix panic when using backwards compatible flags prometheus#2000
* [BUGFIX] Fix wrong value for OpenBSD memory buffer cache prometheus#2015
* [BUGFIX] Only initiate collectors once prometheus#2048
* [BUGFIX] Handle small backwards jumps in CPU idle prometheus#2067

Signed-off-by: Ben Kochie <[email protected]>
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.

4 participants