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

Remove smartmon run metric #152

Merged
merged 1 commit into from
May 22, 2023
Merged

Remove smartmon run metric #152

merged 1 commit into from
May 22, 2023

Conversation

SuperQ
Copy link
Contributor

@SuperQ SuperQ commented May 22, 2023

This metric is a bit funny, it's a timestamp per device, but it's not accurate enough (seconds, not milliseconds) to derive much useful information from it. This should probably be replaced with a per-device duration metric.

@SuperQ SuperQ requested a review from dswarbrick May 22, 2023 18:07
This metric is a bit funny, it's a timestamp per device, but it's not
accurate enough (seconds, not milliseconds) to derive much useful
information from it. This should probably be replaced with a per-device
duration metric.

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ SuperQ force-pushed the superq/remove_smartmon_run branch from 9552f42 to c748fa8 Compare May 22, 2023 18:08
@dswarbrick
Copy link
Member

Is this related to my necropost on #104? ;-)

Personally I don't know what the point of this metric is, and the help text is equally vague. If the original author wanted timestamps of when the metrics were actually collected for the device, this is not the right way to go about it.

I suspect they were unaware that the Prometheus text exposition format allows optional timestamps as the last field on the line.

@SuperQ
Copy link
Contributor Author

SuperQ commented May 22, 2023

Yes, but we forbid timestamps on textfile collector metrics. And there's already a node_exporter mtime metric for when the last textfile was updated.

@dswarbrick
Copy link
Member

I was not aware that timestamps are forbidden on textfile collector metrics. I suspect I probably attempted to use them on the odd occasion in the past.

In any case, I don't see that this smartctl_run metric adds anything of significant value that cannot be derived from node_textfile_mtime_seconds, and I agree that a per-device collection duration would be more useful.

@SuperQ SuperQ merged commit 94801b8 into master May 22, 2023
@SuperQ SuperQ deleted the superq/remove_smartmon_run branch May 22, 2023 18:56
equinox0815 added a commit to spreadspace/node-exporter-textfile-collector-scripts that referenced this pull request Jul 18, 2023
The metric has mostly been removed in prometheus-community#152. This change also cleans the stale metric instance.

Signed-off-by: Christian Pointner <equinox@spreadspace.org>
This pull request was closed.
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.

2 participants