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

regexp params doesn't work when passed as string in arguments #1737

Closed
fvigotti opened this issue Jun 7, 2020 · 7 comments
Closed

regexp params doesn't work when passed as string in arguments #1737

fvigotti opened this issue Jun 7, 2020 · 7 comments

Comments

@fvigotti
Copy link

fvigotti commented Jun 7, 2020

I've lost 4 hours today because of this :
( kbuernetes pod.. )

...
image: "prom/node-exporter:v1.0.0"
args:
  - --collector.filesystem.ignored-fs-types="^(tmpfs|cgroup).*$"

doesn't work ( meaning that ignored fs types param doesn't have other effect than just disabling the default value of the param ) ,
no error , nothing logged just filesystems doesn't get ignored ( i've started playing out with escaping regex, changhing other settings, changing rootfs path ecc.. )

at the end... the light

- "--collector.filesystem.ignored-fs-types=^(tmpfs|cgroup).*$"

this works..
now there are around a lot of examples and tutorial which use the format listed in the first example .. which doesn't work.. at least in kubernetes yaml definition format

( the same is valid for : "--collector.filesystem.ignored-mount-points" ( and probably for all others regexp params? )

I hope that this post could help others that with my same issue tried, I think that this should be better documented/handled because is very nasty unwanted behaviour..

thank you

@SuperQ
Copy link
Member

SuperQ commented Jun 7, 2020

I would recommend filing issues/PRs against the guides that have the wrong documentation. There's not much we can do about it here. Unless you found the wrong syntax in official Prometheus docs. 😄

@fvigotti
Copy link
Author

fvigotti commented Jun 7, 2020

I'm not sure that this is only a documentation issue.. I mean :
if this behaviour is wanted -> then it's a documentation issue ( I hope you don't handle this issue that way )
else: args parsing should be better implemented ( my suggestion is to do that, or at least print a warning in runtime... )

edit:
for runtime warning I mean for invalid regexp/unknown args received / whatelse you find useful to handle this case ( maybe also a ENV variable version of program arguments could be a better way to provide args ( ie: see grafana ) )

@SuperQ
Copy link
Member

SuperQ commented Jun 7, 2020

We already have invalid regexp detection, the regexp.MustCompile() function we use to parse the argument will panic and crash the exporter if it's not a valid regexp. We do the same error and crash on unknown arguments.

@fvigotti
Copy link
Author

fvigotti commented Jun 7, 2020

@SuperQ understood... maybe the issue is that the parsed regepx is "regexp" instead of regexp which is still valid if compiled but wrongly received because of additional quotes ? ( I'm trying to undestand what the problem is .. I think anyway that the program should be just a bit smarter during that args parsing and don't trigger the issue I've lost 4 hours on.. ) anyway if you don't agree at least there will be this issue for others in the same situation..
which I invite to add a like to this post as "vote" for change :)
thank you again,
Francesco

@SuperQ
Copy link
Member

SuperQ commented Jun 8, 2020

Yes, that's very likely the problem. The kubernetes yaml is including the quotes when setting the flag.

I've been considering creating a more comprehensive status page for the node_exporter, that would include more runtime, build, and of course all of the flags in the output.

@discordianfish
Copy link
Member

Hrmm yeah I totally see how this could have been me wasting 4h on this..
That being said, there isn't much we can do. If you run node_exporter --collector.filesystem.ignored-fs-types="^(tmpfs|cgroup).*$" in a shell, it will remove the quotes before handing it to the command. Try this:

#!/bin/bash
echo "$1"

and invoke that with script.sh foo="bar". You'll see the script will only see foo=bar (without quotes).

But if you use something that passes the arguments directly as a list as you did, without a shell interpreting it, it will have the literal " in there..

SuperQ added a commit that referenced this issue Jun 12, 2020
Update netdev and systemd collectors to deprecate poorly chosen flag names.

Old flag names to be removed in 2.0.0.

#1742

Add log messages for parsed flag values to help discover quoting isuses in
supervisors.

#1737

Signed-off-by: Ben Kochie <superq@gmail.com>
@SuperQ
Copy link
Member

SuperQ commented Jun 12, 2020

I tacked on some logging improvements to #1743.

oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this issue Apr 9, 2024
Update netdev and systemd collectors to deprecate poorly chosen flag names.

Old flag names to be removed in 2.0.0.

prometheus#1742

Add log messages for parsed flag values to help discover quoting isuses in
supervisors.

prometheus#1737

Signed-off-by: Ben Kochie <superq@gmail.com>
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this issue Apr 9, 2024
Update netdev and systemd collectors to deprecate poorly chosen flag names.

Old flag names to be removed in 2.0.0.

prometheus#1742

Add log messages for parsed flag values to help discover quoting isuses in
supervisors.

prometheus#1737

Signed-off-by: Ben Kochie <superq@gmail.com>
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

No branches or pull requests

3 participants