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

Escape double quotes in device model family #772

Merged
merged 1 commit into from
Jan 24, 2018

Conversation

vetal4444
Copy link
Contributor

Some devices has double quotes in it's model family. Something like this:
Model Family: Toshiba 3.5" DT01ACA... Desktop HDD
It makes broken metric:
smartmon_device_info{disk="/dev/sdb",type="sat",model_family="Toshiba 3.5" DT01ACA... Desktop HDD",device_model="TOSHIBA DT01ACA300",serial_number="63QY1WTGS",firmware_version="MX6OABB0"} 1
This PR is fixing this by escaping ".

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.

Thanks, LGTM!

Maybe we should rewrite this at some point in something where escapting stuff is easier :)

@@ -73,7 +73,7 @@ parse_smartctl_info() {
local disk="$1" disk_type="$2"
while read line ; do
info_type="$(echo "${line}" | cut -f1 -d: | tr ' ' '_')"
info_value="$(echo "${line}" | cut -f2- -d: | sed 's/^ \+//g')"
info_value="$(echo "${line}" | cut -f2- -d: | sed 's/^ \+//g' | sed 's/"/\\"/')"
Copy link
Member

@SuperQ SuperQ Jan 4, 2018

Choose a reason for hiding this comment

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

This could be simplified to sed 's/^ \+//g ; s/"/\\/'.

But, I also think that this might need a g on the second regexp to match all double quotes in the string.

@SuperQ SuperQ merged commit 4ed49e7 into prometheus:master Jan 24, 2018
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
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