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 sample directory size exporter #789

Merged
merged 2 commits into from
Feb 21, 2018
Merged

Conversation

anarcat
Copy link
Contributor

@anarcat anarcat commented Jan 16, 2018

This is a possible workaround for the lack of metrics in the new
storage backend, as documented in:

prometheus/prometheus#3684

Partly inspired by this post as well:

https://www.robustperception.io/monitoring-directory-sizes-with-the-textfile-collector/

@SuperQ
Copy link
Member

SuperQ commented Jan 16, 2018

Thanks. One note, textfile scripts follow the standard of sending the data to STDOUT.

@anarcat
Copy link
Contributor Author

anarcat commented Jan 16, 2018

i followed the example in https://www.robustperception.io/monitoring-directory-sizes-with-the-textfile-collector/, which ensures atomicity... should i change this? i don't mind :)

@SuperQ
Copy link
Member

SuperQ commented Jan 16, 2018

Yes, for the examples here, we decided to go with simple STDOUT as our interface method. This way the script operators can deal with the atomicity themselves.

It also makes it easier to test out scripts by running them manually, so they don't automatically update metrics.

This is a possible workaround for the lack of metrics in the new
storage backend, as documented in:

prometheus/prometheus#3684

Partly inspired by this post as well:

https://www.robustperception.io/monitoring-directory-sizes-with-the-textfile-collector/
@anarcat
Copy link
Contributor Author

anarcat commented Jan 16, 2018

alright, how does 541c05e look instead then?

@anarcat
Copy link
Contributor Author

anarcat commented Jan 16, 2018

hmm... i can't review the build failure on buildkite as i don't have an account. can anyone enlighten me?

@SuperQ
Copy link
Member

SuperQ commented Jan 17, 2018

The buildkite tests are broken, don't worry about them.

@SuperQ
Copy link
Member

SuperQ commented Jan 17, 2018

I would suggest adding s/"/\\"/g ; to the beginning of the sed expression in order to escape any " in the path.

@anarcat
Copy link
Contributor Author

anarcat commented Jan 17, 2018

hmm... is that the only character we would need to escape? this path is provided by the admin... they would be shooting themselves in the foot by submitting such a nasty path...

@SuperQ
Copy link
Member

SuperQ commented Jan 17, 2018

Yes, double quote is the only thing you need to escape.

@brian-brazil
Copy link
Contributor

Backslash also needs escaping in label values.

@anarcat
Copy link
Contributor Author

anarcat commented Jan 17, 2018

so how about adding s/"/\\"/g;s/\\/\\\\/g? does that cover the escaping correctly?

what a mess shell is :p

@SuperQ
Copy link
Member

SuperQ commented Feb 13, 2018

@anarcat Yes, except possibly \\ before \"

@anarcat
Copy link
Contributor Author

anarcat commented Feb 21, 2018

@SuperQ not sure that's necessary:

$ du --block-size=1 --summarize foo\"bar\"baz\\back/| sed -ne 's/\\/\\\\/;s/"/\\"/g;s/^\([0-9]\+\)\t\(.*\)$/node_directory_size_bytes{directory="\2"} \1/p'
node_directory_size_bytes{directory="foo\"bar\"baz\\back/"} 4096

my original sed was backwards, so it was escaping the backslashes from the previous ones...

i've added a commit to properly escape stuff, see if it fits for you... too bad there's no unit tests for this... another reason why this would be a good addition in the core...

@SuperQ SuperQ self-requested a review February 21, 2018 15:23
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 SuperQ merged commit 79ae03c into prometheus:master Feb 21, 2018
@anarcat anarcat deleted the disk-space branch February 21, 2018 15:57
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
* add sample directory size exporter

This is a possible workaround for the lack of metrics in the new
storage backend, as documented in:

prometheus/prometheus#3684

Partly inspired by this post as well:

https://www.robustperception.io/monitoring-directory-sizes-with-the-textfile-collector/

* properly escape backslashes and double-quotes
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.

3 participants