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

[Feature request] Expose metrics for configtest results #85

Closed
cryptobioz opened this issue Jun 10, 2020 · 16 comments
Closed

[Feature request] Expose metrics for configtest results #85

cryptobioz opened this issue Jun 10, 2020 · 16 comments

Comments

@cryptobioz
Copy link

cryptobioz commented Jun 10, 2020

As the Apache server configuration can be changed and reloaded on live, it could be useful to expose some metrics to monitor this.

By running an apachectl configtest command every time the exporter is scraped, we could expose the following metrics:

  • apache_configtest_last_check_successful will return 0 if the configuration is invalid and 1 if everything is okay.

  • apache_configtest_last_check_success_timestamp_seconds will return the timestamp in seconds of the last time the check was successful.

These metrics might help us to prevent an Apache server to reload a faulty configuration or alert us before it happens.

I'll start writing a Pull Request to implement this. I'm open to suggestions and comments.

@roidelapluie
Copy link
Collaborator

Thanks for creating a request.

I am not sure I want this because configtest is an expensive and slow operation.

@cryptobioz
Copy link
Author

Indeed, the other solution would be to run the configtest regularly instead running it at every request.

@mcanevet
Copy link

We tested it with a quite big configuration, and it took a few tens of milliseconds.
Maybe an opt-in parameter would be acceptable?

@roidelapluie
Copy link
Collaborator

an opt-in flag seems reasonable

@mcanevet
Copy link

I'm wondering if there's a way to ask the currently running apachectl process to launch the configtest via a signal or something else to avoid creating a new one.

@Lusitaniae
Copy link
Owner

Not sure that's gonna work

apachectl is a front end to the Apache HyperText Transfer Protocol (HTTP) server.

Second, apachectl can act as a SysV init script, taking simple one-word arguments like start, restart, and stop, and translating them into appropriate signals to httpd.

and when doing configtest, httpd is launched, reads the config file and exits:

Run syntax tests for configuration files only. The program immediately exits after these syntax parsing tests with either a return code of 0 (Syntax OK) or return code not equal to 0 (Syntax Error).

But please look more into this, only had a brief look at this

@cryptobioz
Copy link
Author

Well, we're facing a design problem. The exporter can, and it's a good thing, be used remotely. It doesn't depend on anything and only rely on the HTTP endpoint provided by Apache. It can be deployed next to Apache or on an other container or even on an other machine.

If we add the feature to expose the configtest result, we add an important dependency in this exporter. It will have to be deployed next to Apache in order to work properly because it will need access to the configuration files and the apachectl binary.

Therefore, I'm not sure it's a good thing to add this feature because it will begin to change the design of this exporter...

@Lusitaniae
Copy link
Owner

Lusitaniae commented Jun 11, 2020 via email

@roidelapluie
Copy link
Collaborator

I do think that it is an issue that would be better covered by node_exporter file collector

@cryptobioz
Copy link
Author

When you talk about "node_exporter file collector", do you mean the textfile collector?

@roidelapluie
Copy link
Collaborator

Yes, calling external commands from the exporter exposes it to a new range of security threats.

@roidelapluie
Copy link
Collaborator

Also it has serious limitations like you should run the exporter with the same user as apache and use apache default path for configuration

@cryptobioz
Copy link
Author

Indeed, I'm not a big fan of this solution too but I'd prefer to have a kind of real exporter instead a cronjob. However, you're totally right regarding the security issues and limitations that this kind of workflow involves so I think your solution is the best.

@roidelapluie
Copy link
Collaborator

In general, configtest is run by e.g. config management before putting the new file in place, so it looks like the process to modify the file should run the configtest too and rollback wrong configs..

@cryptobioz
Copy link
Author

Yeah, of course. But in our case, developers have the ability to modify the configuration files so they might break something and we would like to be informed as soon as possible. We will both agree to say that this is not a good practice but we don't have choice here.

@roidelapluie
Copy link
Collaborator

Please note that they can break the configuration in other ways that would not be caugh by this (e.g. removing vhosts). So that would anyway not be a complete solution for you.

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

4 participants