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

Option to track build configuration for changes between builds #34713

Merged
merged 1 commit into from
Aug 10, 2023
Merged
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 @@ -24,6 +24,7 @@
import io.quarkus.bootstrap.prebuild.CodeGenException;
import io.quarkus.deployment.codegen.CodeGenData;
import io.quarkus.deployment.configuration.BuildTimeConfigurationReader;
import io.quarkus.deployment.configuration.tracker.ConfigTrackingValueTransformer;
import io.quarkus.deployment.dev.DevModeContext;
import io.quarkus.deployment.dev.DevModeContext.ModuleInfo;
import io.quarkus.maven.dependency.ResolvedDependency;
Expand Down Expand Up @@ -185,6 +186,43 @@ public static boolean trigger(ClassLoader deploymentClassLoader,
});
}

/**
* Initializes an application build time configuration and returns current values of properties
* passed in as {@code originalProperties}.
*
* @param appModel application model
* @param launchMode launch mode
* @param buildSystemProps build system (or project) properties
* @param deploymentClassLoader build classloader
* @param originalProperties properties to read from the initialized configuration
* @return current values of the passed in original properties
*/
public static Properties readCurrentConfigValues(ApplicationModel appModel, String launchMode,
Properties buildSystemProps,
QuarkusClassLoader deploymentClassLoader, Properties originalProperties) {
Config config = null;
try {
config = getConfig(appModel, LaunchMode.valueOf(launchMode), buildSystemProps, deploymentClassLoader);
} catch (CodeGenException e) {
throw new RuntimeException("Failed to load application configuration", e);
}
var valueTransformer = ConfigTrackingValueTransformer.newInstance(config);
final Properties currentValues = new Properties(originalProperties.size());
for (var originalProp : originalProperties.entrySet()) {
var name = originalProp.getKey().toString();
var currentValue = config.getConfigValue(name);
final String current = valueTransformer.transform(name, currentValue);
if (!originalProp.getValue().equals(current)) {
log.info("Option " + name + " has changed since the last build from "
+ originalProp.getValue() + " to " + current);
}
if (current != null) {
currentValues.put(name, current);
}
}
return currentValues;
}

public static Config getConfig(ApplicationModel appModel, LaunchMode launchMode, Properties buildSystemProps,
QuarkusClassLoader deploymentClassLoader) throws CodeGenException {
final Map<String, List<String>> unavailableConfigServices = getUnavailableConfigServices(appModel.getAppArtifact(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import io.quarkus.deployment.configuration.matching.FieldContainer;
import io.quarkus.deployment.configuration.matching.MapContainer;
import io.quarkus.deployment.configuration.matching.PatternMapBuilder;
import io.quarkus.deployment.configuration.tracker.ConfigTrackingInterceptor;
import io.quarkus.deployment.configuration.type.ArrayOf;
import io.quarkus.deployment.configuration.type.CollectionOf;
import io.quarkus.deployment.configuration.type.ConverterType;
Expand Down Expand Up @@ -122,6 +123,8 @@ private static List<Class<?>> collectConfigRoots(ClassLoader classLoader) throws
final Set<String> deprecatedProperties;
final Set<String> deprecatedRuntimeProperties;

final ConfigTrackingInterceptor buildConfigTracker;

/**
* Initializes a new instance with located configuration root classes on the classpath
* of a given classloader.
Expand Down Expand Up @@ -233,6 +236,8 @@ private BuildTimeConfigurationReader(ClassLoader classLoader, final List<Class<?

deprecatedProperties = getDeprecatedProperties(allRoots);
deprecatedRuntimeProperties = getDeprecatedProperties(runTimeRoots);

buildConfigTracker = new ConfigTrackingInterceptor();
}

private static void processClass(ClassDefinition.Builder builder, Class<?> clazz,
Expand Down Expand Up @@ -399,11 +404,15 @@ public SmallRyeConfig initConfiguration(LaunchMode launchMode, Properties buildS
for (ConfigClassWithPrefix mapping : getBuildTimeVisibleMappings()) {
builder.withMapping(mapping.getKlass(), mapping.getPrefix());
}
return builder.build();

builder.withInterceptors(buildConfigTracker);
var config = builder.build();
buildConfigTracker.configure(config);
Comment on lines +408 to +410
Copy link
Member

Choose a reason for hiding this comment

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

If you need to configure the interceptor, you can use a ConfigSourceInterceptorFactory instead. This provides you access to the context with the current config.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing that out. Given that I'd need to obtain the reference to the created interceptor, not sure whether it will end up being more elegant at the end though.

return config;
}

public ReadResult readConfiguration(final SmallRyeConfig config) {
return SecretKeys.doUnlocked(() -> new ReadOperation(config).run());
return SecretKeys.doUnlocked(() -> new ReadOperation(config, buildConfigTracker).run());
}

private Set<String> getDeprecatedProperties(Iterable<RootDefinition> rootDefinitions) {
Expand Down Expand Up @@ -459,6 +468,7 @@ private void collectDeprecatedConfigItems(ClassMember classMember, Set<String> d

final class ReadOperation {
final SmallRyeConfig config;
final ConfigTrackingInterceptor buildConfigTracker;
final Set<String> processedNames = new HashSet<>();

final Map<Class<?>, Object> objectsByClass = new HashMap<>();
Expand All @@ -468,8 +478,9 @@ final class ReadOperation {

final Map<ConverterType, Converter<?>> convByType = new HashMap<>();

ReadOperation(final SmallRyeConfig config) {
ReadOperation(final SmallRyeConfig config, ConfigTrackingInterceptor buildConfigTracker) {
this.config = config;
this.buildConfigTracker = buildConfigTracker;
}

ReadResult run() {
Expand Down Expand Up @@ -662,6 +673,7 @@ ReadResult run() {
.setRunTimeMappings(runTimeMappings)
.setUnknownBuildProperties(unknownBuildProperties)
.setDeprecatedRuntimeProperties(deprecatedRuntimeProperties)
.setBuildConfigTracker(buildConfigTracker)
.createReadResult();
}

Expand Down Expand Up @@ -1129,6 +1141,7 @@ public static final class ReadResult {

final Set<String> unknownBuildProperties;
final Set<String> deprecatedRuntimeProperties;
final ConfigTrackingInterceptor.ReadOptionsProvider readOptionsProvider;

public ReadResult(final Builder builder) {
this.objectsByClass = builder.getObjectsByClass();
Expand All @@ -1151,6 +1164,8 @@ public ReadResult(final Builder builder) {

this.unknownBuildProperties = builder.getUnknownBuildProperties();
this.deprecatedRuntimeProperties = builder.deprecatedRuntimeProperties;
this.readOptionsProvider = builder.buildConfigTracker == null ? null
: builder.buildConfigTracker.getReadOptionsProvider();
}

private static Map<Class<?>, RootDefinition> rootsToMap(Builder builder) {
Expand Down Expand Up @@ -1243,6 +1258,10 @@ public Object requireObjectForClass(Class<?> clazz) {
return obj;
}

public ConfigTrackingInterceptor.ReadOptionsProvider getReadOptionsProvider() {
return readOptionsProvider;
}

static class Builder {
private Map<Class<?>, Object> objectsByClass;
private Map<String, String> allBuildTimeValues;
Expand All @@ -1257,6 +1276,7 @@ static class Builder {
private List<ConfigClassWithPrefix> runTimeMappings;
private Set<String> unknownBuildProperties;
private Set<String> deprecatedRuntimeProperties;
private ConfigTrackingInterceptor buildConfigTracker;

Map<Class<?>, Object> getObjectsByClass() {
return objectsByClass;
Expand Down Expand Up @@ -1371,6 +1391,11 @@ Builder setDeprecatedRuntimeProperties(Set<String> deprecatedRuntimeProperties)
return this;
}

Builder setBuildConfigTracker(ConfigTrackingInterceptor buildConfigTracker) {
this.buildConfigTracker = buildConfigTracker;
return this;
}

ReadResult createReadResult() {
return new ReadResult(this);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package io.quarkus.deployment.configuration.tracker;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.regex.Pattern;

import io.quarkus.runtime.annotations.ConfigPhase;
import io.quarkus.runtime.annotations.ConfigRoot;
import io.quarkus.util.GlobUtil;
import io.smallrye.config.ConfigMapping;
import io.smallrye.config.WithDefault;

/**
* Configuration options for application build time configuration usage tracking
* and dumping.
*/
@ConfigMapping(prefix = "quarkus.config-tracking")
@ConfigRoot(phase = ConfigPhase.BUILD_TIME)
public interface ConfigTrackingConfig {

/**
* Whether configuration dumping is enabled
*/
@WithDefault("false")
boolean enabled();

/**
* Directory in which the configuration dump should be stored.
* If not configured the {@code .quarkus} directory under the project directory will be used.
*/
Optional<Path> directory();

/**
* File in which the configuration dump should be stored. If not configured, the {@link #filePrefix} and
* {@link #fileSuffix} will be used to generate the final file name.
* If the configured file path is absolute, the {@link #directory} option will be ignored. Otherwise,
* the path will be considered relative to the {@link #directory}.
*/
Optional<Path> file();

/**
* File name prefix. This option will be ignored in case {@link #file} is configured.
*/
@WithDefault("quarkus")
String filePrefix();

/**
* File name suffix. This option will be ignored in case {@link #file} is configured.
*/
@WithDefault("-config-dump")
String fileSuffix();

/**
* A list of config properties that should be excluded from the report.
* GLOB patterns could be used instead of property names.
*/
Optional<List<String>> exclude();

/**
* Translates the value of {@link #exclude} to a list of {@link java.util.regex.Pattern}.
*
* @return list of patterns created from {@link #exclude}
*/
default List<Pattern> getExcludePatterns() {
return toPatterns(exclude());
}

/**
* A list of config properties whose values should be hashed in the report.
* The values will be hashed using SHA-512 algorithm.
* GLOB patterns could be used instead of property names.
*/
Optional<List<String>> hashOptions();

/**
* Translates the value of {@link #hashOptions()} to a list of {@link java.util.regex.Pattern}.
*
* @return list of patterns created from {@link #hashOptions()}
*/
default List<Pattern> getHashOptionsPatterns() {
return toPatterns(hashOptions());
}

static List<Pattern> toPatterns(Optional<List<String>> globs) {
if (globs.isEmpty()) {
return List.of();
}
var list = globs.get();
final List<Pattern> patterns = new ArrayList<>(list.size());
for (var s : list) {
patterns.add(Pattern.compile(GlobUtil.toRegexPattern(s)));
}
return patterns;
}

/**
* Whether to use a {@code ~} as an alias for user home directory in path values
*/
@WithDefault("true")
boolean useUserHomeAliasInPaths();
Copy link
Member

Choose a reason for hiding this comment

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

this seems out of place.

  1. why have this as an option in this specific corner of Quarkus ?
  2. why do we need to refer to user home path anyways for this - is it not supposed to be relative to the project ?

Copy link
Member

Choose a reason for hiding this comment

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

ah sorry I read it as this was used for reading variables.

..so I grok the relevance but is there even a value in storing this as afaik we don't interpret ~ into ${user.home} when reading the values ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't tried using ~ in config values, tbh. If we don't do this kind of transformation, every config will be different for each user. It will often still be even with ~ but user home dir path looks like an obvious thing to ignore in comparisons.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package io.quarkus.deployment.configuration.tracker;

import static io.smallrye.config.SecretKeys.doLocked;

import java.nio.file.Path;
import java.util.Collections;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import jakarta.annotation.Priority;

import org.eclipse.microprofile.config.Config;

import io.quarkus.deployment.configuration.BuildTimeConfigurationReader;
import io.quarkus.runtime.LaunchMode;
import io.smallrye.config.ConfigSourceInterceptor;
import io.smallrye.config.ConfigSourceInterceptorContext;
import io.smallrye.config.ConfigValue;
import io.smallrye.config.Priorities;

/**
* Build configuration interceptor that records all the configuration options
* and their values that are read during the build.
*/
@Priority(Priorities.APPLICATION)
public class ConfigTrackingInterceptor implements ConfigSourceInterceptor {
Copy link
Member

Choose a reason for hiding this comment

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

What if we split the writing part from the interceptor? The interceptor could just record the values in a static Map, that could be accessed from anywhere.

The writer's logic could then be extracted and applied when required.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it could be done better. I just didn't want to expose the interceptor itself with its public API to build steps.

Copy link
Member Author

Choose a reason for hiding this comment

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

I made ConfigTrackingInterceptor expose ConfigTrackingInterceptor.ReadOptionsProvider that returns a Map<String, String> of the read options. And the writing part is extracted to ConfigTrackingWriter.
The reason for using Map<String, String> instead of Map<String, ConfigValue> is to be able to record null config values in a ConcurrentHashMap


/**
* A writer that persists collected configuration options and their values to a file
*/
public interface ConfigurationWriter {
void write(ConfigTrackingConfig config, BuildTimeConfigurationReader.ReadResult configReadResult,
LaunchMode launchMode, Path buildDirectory);
}

/**
* Provides an immutable map of options that were read during the build.
*/
public interface ReadOptionsProvider {

/**
* An immutable map of options read during the build.
*
* @return immutable map of options read during the build
*/
Map<String, String> getReadOptions();
}

private boolean enabled;
// it's a String value map to be able to represent null (not configured) values
private Map<String, String> readOptions = Map.of();
private final ReadOptionsProvider readOptionsProvider = new ReadOptionsProvider() {
@Override
public Map<String, String> getReadOptions() {
return Collections.unmodifiableMap(readOptions);
}
};

/**
* Initializes the configuration tracker
*
* @param config configuration instance
*/
public void configure(Config config) {
enabled = config.getValue("quarkus.config-tracking.enabled", boolean.class);
if (enabled) {
readOptions = new ConcurrentHashMap<>();
}
}

@Override
public ConfigValue getValue(ConfigSourceInterceptorContext context, String name) {
if (!enabled) {
return context.proceed(name);
}
final ConfigValue configValue = doLocked(() -> context.proceed(name));
readOptions.put(name, ConfigTrackingValueTransformer.asString(configValue));
return configValue;
}

/**
* Read options orvipder.
*
* @return read options provider
*/
public ReadOptionsProvider getReadOptionsProvider() {
return readOptionsProvider;
}
}
Loading
Loading