Skip to content

ExporterExporter improvements#426

Open
inqrphl wants to merge 13 commits into
mainfrom
exporter-improvements
Open

ExporterExporter improvements#426
inqrphl wants to merge 13 commits into
mainfrom
exporter-improvements

Conversation

@inqrphl

@inqrphl inqrphl commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Fix 'url prefix' not working. ExporterExporter was registering the prefixes correctly so that listener routed them to the ExporterExporter. But it was not removing the prefix in ServeHTTP before checking if the remaining portion is "/list" or "/proxy"


Read modules: The ExporterExporter can specify a module configuration path, in which multiple module config files can be put inside. If opening/parsing/validating one of the configs failed, it would return an error that would propage upwards to the snclient module initialization system. This would make snclient module agent exit, due to ExporterExporter module not being able to start.

In this PR, it just skips over the modules that cause errors, this can be either due to bad YAML syntax, file being unparsable as a module config file, file not being readable etc.

Even if all of the module config files are wrong, it does not return an error and cause snclient to stop.


File watcher: The module directory is now watched for changes using fsnotify. This uses a new library: fsnotify , which uses inodes on Linux and 'ReadDirectoryChangesW' on windows. Seems to be more robust than manually checking files.

If a file is written into, deleted or renamed an event is fired. The events that are related to non .yml / .yaml files are ignored. If so, the modules are regenerated from the module config folder. Until the regeneration is complete, the exporter_exporter uses the old parsed config, but once the new one is ready it is switched to new version. The change is protected with a mutex. After a change new events that come for 5 seconds are ignored in a debounce period.

I checked this both by hand in Linux and Windows, seems to be notificing and firing events, and reloading the modules index without problems.

curl http://127.0.0.1:[port]/[url prefix]/list -u :[password]

# prefix is set to 'hello' , password is set to 'nicepassword'
curl 127.0.0.1:8443/hello/list -u :nicepassword
<h2>Exporters:</h2>
<ul>
<li><a href="/hello/proxy?module=node-zart">node-zart</a></li>
<li><a href="/hello/proxy?module=node">node</a></li>
</ul>

the file watcher is toggleable using the 'modules dir watcher' option. By default it is false, so only users needing this may turn it on knowing its possible side effects.


Add 'allowed methods' configuration point.

Exporter_exporter has multiple ways of getting results from a prometheus conforming exporter. Https does a query and gets the output, file reads from a file (which is presumably refreshed from time to time) and exec calls an exporter directly.

Add a whitelist in the exporter_exporter configuration section called 'allowed methods', if its populated it will reject module definitions that dont use its methods. Is meant to protect against 'exec' types of modules.


Ahmet Oeztuerk added 2 commits July 21, 2026 16:34
the prefix is set using the "url prefix" line in the configuration section /settings/ExporterExporter/server

HandlerExporterExporter.GetMappings were returning prefixed paths already,

But HandlerExporterExporter.ServeHTTP was not checking for them
…ctory instead of failing

returning an error would propagate up and stop snclient from starting

decision: ignore exporterexporter config files that fail to be parsed. even if all config files are failing, do not return an error
@inqrphl
inqrphl marked this pull request as draft July 21, 2026 15:29
@inqrphl
inqrphl marked this pull request as ready for review July 21, 2026 15:29
@inqrphl
inqrphl marked this pull request as draft July 21, 2026 15:29
inqrphl and others added 4 commits July 21, 2026 17:29
… restarts and remove the stop(), init(), start() loop

when a request to exporter_exporter comes, it dynamically picks which module to use from Handler.ExporterExporter.modules

the mappings it passes to listener are always the same, l.urlPrefix + "/list" and l.urlPrefix + "/proxy" the listener does not need to be restarted on a file watcher cahnge.

only the loaded modules need to be changed, which can be done using reloadModules()

reload the modules, and protect modules using a mutex during its reload
…estricts expoter modules based on the method they want to use

mainly to restrict usage of the 'exec' method so that someone can not just write a file in the modules directory and then execute something malicious. that would not even need a restart of snclient due to newly added file watcher
@inqrphl
inqrphl force-pushed the exporter-improvements branch from b92c5b2 to 2d0b441 Compare July 22, 2026 11:35
Ahmet Oeztuerk added 3 commits July 22, 2026 14:09
added both in HTML and in JSON outputs
…unctionality, including newly added functions

- checks if bad module configuration files of different reasons are skipped, and valid module configuration files are added
- check if 'allowed methods' are working properly, including comma separation of allowed modules
- check if snclient agent and exporter_exporter starts, even if no valid module files exist
- check if url prefix is used, and is correctly listed under /list endpoints
- check if adding a new file in the module config directory triggers a reload, and the new file is listed in /list
- check if /list works without a module dir

also on the /list endpoint, always append the urlPrefix to links of the modules and change the HTTP status code to OK.
@inqrphl

inqrphl commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@lgmu

I did implement the file watcher now, and it has a debounce period of 5 seconds.

The file watcher sends events through a channel. To not reload modules for every single event, I implemented a debounce mechanism. Meaning it ignores events for 5 seconds after a reload of modules was triggered. Is that ok?

Secondly, the watcher does not care if the event added/modified/deleted a file that ends with .yml or .yaml extension. I did this so that even writing to a "timestamp" file the current time can trigger a reload of the modules. Thought this would help out due script. I can either leave it out like that, or throw away events that dont work on files that arent YAML

Ahmet Oeztuerk and others added 3 commits July 22, 2026 15:49
… the modules dir watcher and by default it is off

it is better to set it default off, so only users wanting this toggle it on.
…ConfigDirectoryNoReloadWhenWatcherDisabled - very nice name, checks if the file watcher functionality is toggled off
@inqrphl
inqrphl marked this pull request as ready for review July 22, 2026 13:57
@inqrphl
inqrphl marked this pull request as draft July 22, 2026 14:15
….yml and .yaml extension files. it ignores other file changes

ai change - add another test TestExporterExporterFileWatcherIgnoresNonYaml that checks for this behavior - .yml file is added while .txt file is ignored after refresh
@inqrphl
inqrphl marked this pull request as ready for review July 22, 2026 14:26
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.

1 participant