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

Add tests to verify that every config setting for each auto-configured meter registry is exposed as a configuration property #33743

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ public class DatadogProperties extends StepRegistryProperties {
private String apiKey;

/**
* Datadog application key. Not strictly required, but improves the Datadog experience
* by sending meter descriptions, types, and base units to Datadog.
* Datadog's application key. Not strictly required, but improves the Datadog
* experience by sending meter descriptions, types, and base units to Datadog.
*/
private String applicationKey;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.boot.actuate.autoconfigure.metrics.export;

import java.lang.reflect.Method;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

import org.junit.Assert;

import org.springframework.boot.actuate.autoconfigure.metrics.export.properties.PropertiesConfigAdapter;

/**
* Utility to test that all Micrometer config values are exposed via properties and
* settable using the corresponding {@link PropertiesConfigAdapter} implementation.
*
* @author Mirko Sobeck
*/
public final class TestConfigsToPropertiesExposure {

private TestConfigsToPropertiesExposure() {

}

private static final List<Class<?>> classesForPropertiesList = List.of(Boolean.class, Byte.class, Character.class,
Copy link
Member

Choose a reason for hiding this comment

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

Can we add Enum too?
We have quite a few use-cases, e.g.: HistogramFlavor for PrometheusConfig or DynatraceApiVersion for DynatraceConfig

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for taking a look, @jonatan-ivanov. I've removed the allow-list approach in my polishing. We now consider all default methods with no parameters, except those that are deprecated or that return Validated. The code that does that is here.

Short.class, Integer.class, Long.class, Double.class, Float.class, String.class, Duration.class);

/**
* Assertion to test if default methods of a given config are overridden by the
* adapter which implements it. This can be an indicator for micrometer config fields,
* that have been forgotten to expose via spring properties. Not overridden default
* methods in adapters are the most common cause of forgotten field exposure, because
* they do not for force an override.
* @param config micrometer config
* @param adapter adapter for properties {@link PropertiesConfigAdapter}
* @param excludedConfigMethods config methods that should be excluded for the
* assertion
*/
public static void assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(Class<?> config,
Class<? extends PropertiesConfigAdapter<?>> adapter, String... excludedConfigMethods) {
List<String> configDefaultMethodNames = Arrays.stream(config.getDeclaredMethods())
.filter((method) -> method.isDefault() && isSettableUsingProperties(method.getReturnType()))
.map(Method::getName).collect(Collectors.toList());

configDefaultMethodNames.removeAll(Arrays.stream(excludedConfigMethods).toList());
List<String> notOverriddenDefaultMethods = new ArrayList<>(configDefaultMethodNames);

Class<?> currentClass = adapter;
// loop through adapter class and superclasses
// to find not overridden config methods
while (!Object.class.equals(currentClass)) {
List<String> overriddenClassDefaultMethods = Arrays.stream(currentClass.getDeclaredMethods())
.map(Method::getName).filter(configDefaultMethodNames::contains).toList();

notOverriddenDefaultMethods.removeAll(overriddenClassDefaultMethods);
currentClass = currentClass.getSuperclass();
}

if (notOverriddenDefaultMethods.size() >= 1) {
Assert.fail(
"Found config default methods that are not overridden by the related PropertiesConfigAdapter: \n"
+ notOverriddenDefaultMethods + "\n"
+ "This could be an indicator for not exposed properties fields.\n"
+ "Please check if the fields are meant to be exposed and if not, "
+ "exclude them from this test by providing them to the method.");
}
}

/**
* Guess if a class can be set using properties. This will only catch the basic use
* cases regarding the micrometer configs to filter out methods that are not likely to
* be designed to be set via properties. <pre>
* isSettableUsingProperties(String.class) = true
* isSettableUsingProperties(boolean.class) = true
* isSettableUsingProperties(Object.class) = false
* </pre>
* @param clazz Class
* @return is likely to be settable using properties
*/
private static boolean isSettableUsingProperties(Class<?> clazz) {
if (Void.TYPE.equals(clazz)) {
return false;
}

if (clazz.isPrimitive()) {
return true;
}

return classesForPropertiesList.contains(clazz);
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,8 +16,10 @@

package org.springframework.boot.actuate.autoconfigure.metrics.export.appoptics;

import io.micrometer.appoptics.AppOpticsConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;
import org.springframework.boot.actuate.autoconfigure.metrics.export.properties.StepRegistryPropertiesConfigAdapterTests;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -68,4 +70,10 @@ void whenPropertiesFloorTimesIsSetAdapterFloorTimesReturnsIt() {
assertThat(createConfigAdapter(properties).floorTimes()).isTrue();
}

@Test
void allDefaultConfigMethodsAreOverriddenByAtlasPropertiesConfigAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(AppOpticsConfig.class,
AppOpticsPropertiesConfigAdapter.class, "connectTimeout");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

import java.time.Duration;

import com.netflix.spectator.atlas.AtlasConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand Down Expand Up @@ -116,4 +119,11 @@ void whenPropertiesEvalUriIsSetAdapterEvalUriReturnsIt() {
.isEqualTo("https://atlas.example.com/evaluate");
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(AtlasConfig.class,
AtlasPropertiesConfigAdapter.class, "lwcIgnorePublishStep", "initialPollingDelay", "autoStart",
"lwcStep", "validTagCharacters");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@

package org.springframework.boot.actuate.autoconfigure.metrics.export.datadog;

import io.micrometer.datadog.DatadogConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;
import org.springframework.boot.actuate.autoconfigure.metrics.export.properties.StepRegistryPropertiesConfigAdapterTests;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -76,4 +78,10 @@ void whenPropertiesUriIsSetAdapterUriReturnsIt() {
assertThat(createConfigAdapter(properties).uri()).isEqualTo("https://app.example.com/api/v1/series");
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(DatadogConfig.class,
DatadogPropertiesConfigAdapter.class);
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2022 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,8 +19,11 @@
import java.util.HashMap;

import io.micrometer.dynatrace.DynatraceApiVersion;
import io.micrometer.dynatrace.DynatraceConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand Down Expand Up @@ -125,4 +128,10 @@ void defaultValues() {
assertThat(properties.getV2().isUseDynatraceSummaryInstruments()).isTrue();
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(DynatraceConfig.class,
DynatracePropertiesConfigAdapter.class, "documentType");
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2021 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,8 +16,11 @@

package org.springframework.boot.actuate.autoconfigure.metrics.export.elastic;

import io.micrometer.elastic.ElasticConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand Down Expand Up @@ -97,4 +100,10 @@ void whenPropertiesApiKeyCredentialsIsSetAdapterPipelineReturnsIt() {
assertThat(new ElasticPropertiesConfigAdapter(properties).apiKeyCredentials()).isEqualTo("secret");
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(ElasticConfig.class,
ElasticPropertiesConfigAdapter.class, "documentType");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@
import java.util.concurrent.TimeUnit;

import info.ganglia.gmetric4j.gmetric.GMetric.UDPAddressingMode;
import io.micrometer.ganglia.GangliaConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand Down Expand Up @@ -81,4 +84,10 @@ void whenPropertiesPortIsSetAdapterPortReturnsIt() {
assertThat(new GangliaPropertiesConfigAdapter(properties).port()).isEqualTo(4242);
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(GangliaConfig.class,
GangliaPropertiesConfigAdapter.class, "protocolVersion");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
import java.time.Duration;
import java.util.concurrent.TimeUnit;

import io.micrometer.graphite.GraphiteConfig;
import io.micrometer.graphite.GraphiteProtocol;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand Down Expand Up @@ -94,4 +97,10 @@ void whenPropertiesTagsAsPrefixIsSetAdapterTagsAsPrefixReturnsIt() {
assertThat(new GraphitePropertiesConfigAdapter(properties).tagsAsPrefix()).isEqualTo(new String[] { "worker" });
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(GraphiteConfig.class,
GraphitePropertiesConfigAdapter.class);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

import java.util.Collections;

import io.micrometer.humio.HumioConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand Down Expand Up @@ -51,4 +54,10 @@ void whenPropertiesUriIsSetAdapterUriReturnsIt() {
assertThat(new HumioPropertiesConfigAdapter(properties).uri()).isEqualTo("https://humio.example.com");
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(HumioConfig.class,
HumioPropertiesConfigAdapter.class, "connectTimeout");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@
package org.springframework.boot.actuate.autoconfigure.metrics.export.influx;

import io.micrometer.influx.InfluxApiVersion;
import io.micrometer.influx.InfluxConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand Down Expand Up @@ -58,4 +61,10 @@ void adaptInfluxV2BasicConfig() {
assertThat(adapter.token()).isEqualTo("token");
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(InfluxConfig.class,
InfluxPropertiesConfigAdapter.class);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@

import java.time.Duration;

import io.micrometer.jmx.JmxConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;

import static org.assertj.core.api.Assertions.assertThat;

/**
Expand All @@ -43,4 +46,10 @@ void whenPropertiesDomainIsSetAdapterDomainReturnsIt() {
assertThat(new JmxPropertiesConfigAdapter(properties).domain()).isEqualTo("abc");
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(JmxConfig.class,
JmxPropertiesConfigAdapter.class);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@

package org.springframework.boot.actuate.autoconfigure.metrics.export.kairos;

import io.micrometer.kairos.KairosConfig;
import org.junit.jupiter.api.Test;

import org.springframework.boot.actuate.autoconfigure.metrics.export.TestConfigsToPropertiesExposure;
import org.springframework.boot.actuate.autoconfigure.metrics.export.properties.StepRegistryPropertiesConfigAdapterTests;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -62,4 +64,10 @@ void whenPropertiesPasswordIsSetAdapterPasswordReturnsIt() {
assertThat(createConfigAdapter(properties).password()).isEqualTo("secret");
}

@Test
void allConfigDefaultMethodsAreOverriddenByAdapter() {
TestConfigsToPropertiesExposure.assertThatAllConfigDefaultMethodsAreOverriddenByAdapter(KairosConfig.class,
KairosPropertiesConfigAdapter.class);
}

}
Loading