jenkinsci / prometheus-plugin

Jenkins Prometheus Plugin
https://plugins.jenkins.io/prometheus/
Apache License 2.0
184 stars 152 forks source link

Don't log warning when environment variable ``COLLECT_DISK_USAGE`` is not set #563

Closed AB-xdev closed 1 year ago

AB-xdev commented 1 year ago

Steps to reproduce

Install the plugin and start Jenkins

Actual behaviour

The following warning occurs:

2023-09-25 08:53:00.318+0000 [id=31]    WARNING o.j.p.p.c.PrometheusConfiguration#setCollectDiskUsageBasedOnEnvironmentVariableIfDefined: Unable to parse environment variable 'COLLECT_DISK_USAGE'. Must either be 'true' or 'false'. Ignoring...

Expected behaviour

No warning occurs because COLLECT_DISK_USAGE is not set.

We e.g. configured Jenkins via CasC and therefore don't use environment variables.

The warning occurs due to https://github.com/jenkinsci/prometheus-plugin/blob/e274fc47e78ef9499b3696564df4850b32e88b2d/src/main/java/org/jenkinsci/plugins/prometheus/config/PrometheusConfiguration.java#L125-L146

I think the warning should only occur if the environment variable is set but it's invalid. This could be achieved with the following code:

    public void setCollectDiskUsageBasedOnEnvironmentVariableIfDefined() {
        try {
            final String envValue = System.getenv(COLLECT_DISK_USAGE);
            if (envValue != null) {
                this.collectDiskUsage = getValidBooleanValueOrThrowException(envValue);
            }
        } catch (IllegalArgumentException e) {
            logger.warn("Unable to parse environment variable '{}'. Must either be 'true' or 'false'. Ignoring...", COLLECT_DISK_USAGE);
        }
    }

    private boolean getValidBooleanValueOrThrowException(String value) throws IllegalArgumentException {
        if (i"true".equalsIgnoreCase(value) || "false".equalsIgnoreCase(value)) {
            return Boolean.parseBoolean(value);
        }
        throw new IllegalArgumentException();
    }

Server configuration

Operating system: N/A Jenkins Version: N/A Plugin Version: 2.3.3 (currently latest)

Notify

@Waschndolos