Skip to content

Commit

Permalink
enable analytics for release builds only (DAT-18902) (liquibase#6465)
Browse files Browse the repository at this point in the history
* Revert "turn off analytics temporarily (DAT-18902) (liquibase#6464)"

This reverts commit 4db540b.

* enable analytics for release builds only
  • Loading branch information
StevenMassaro authored Oct 28, 2024
1 parent 3db80b4 commit 789c5e6
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ import liquibase.extension.testing.testsystem.spock.LiquibaseIntegrationTest
import liquibase.util.LiquibaseUtil
import liquibase.util.SystemUtil
import org.yaml.snakeyaml.Yaml
import spock.lang.Ignore
import spock.lang.IgnoreIf
import spock.lang.Shared
import spock.lang.Specification

import java.util.concurrent.TimeUnit

@Ignore
@LiquibaseIntegrationTest
class AnalyticsIntegrationTest extends Specification {

Expand Down Expand Up @@ -132,6 +130,7 @@ class AnalyticsIntegrationTest extends Specification {
Map<String, ?> scopeVars = new HashMap<>()
scopeVars.put(AnalyticsArgs.CONFIG_ENDPOINT_URL.getKey(), "http://localhost:" + simpleWebserver.getListeningPort() + "/config-analytics.yaml")
scopeVars.put(AnalyticsArgs.CONFIG_ENDPOINT_TIMEOUT_MILLIS.getKey(), TimeUnit.SECONDS.toMillis(60)) // to allow for debugging, otherwise the thread gets killed fast
scopeVars.put(AnalyticsArgs.DEV_OVERRIDE.getKey(), true)
Scope.child(scopeVars, scopedRunner)
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
package liquibase.analytics.configuration;

import liquibase.Scope;
import liquibase.configuration.AutoloadedConfigurations;
import liquibase.configuration.ConfigurationDefinition;
import liquibase.license.LicenseServiceUtils;
import liquibase.logging.Logger;
import liquibase.util.LiquibaseUtil;
import org.apache.commons.lang3.BooleanUtils;

import java.util.logging.Level;

Expand All @@ -13,6 +18,7 @@ public class AnalyticsArgs implements AutoloadedConfigurations {
*/
public static final ConfigurationDefinition<Boolean> ENABLED;
public static final ConfigurationDefinition<String> CONFIG_ENDPOINT_URL;
private static final ConfigurationDefinition<Boolean> DEV_OVERRIDE;
public static final ConfigurationDefinition<Integer> CONFIG_ENDPOINT_TIMEOUT_MILLIS;
public static final ConfigurationDefinition<Level> LOG_LEVEL;
public static final ConfigurationDefinition<Integer> LICENSE_KEY_CHARS;
Expand All @@ -30,6 +36,12 @@ public class AnalyticsArgs implements AutoloadedConfigurations {
.setHidden(true)
.build();

DEV_OVERRIDE = builder.define("devOverride", Boolean.class)
.setDescription("By default, Liquibase will not send analytics in dev (non release) builds. To override this behavior, set this value to true and provide a value for " + CONFIG_ENDPOINT_URL.getKey())
.setHidden(true)
.setDefaultValue(false)
.build();

TIMEOUT_MILLIS = builder.define("timeoutMillis", Integer.class)
.setHidden(true)
.setDescription("By default, the timeout for sending data to the remote endpoint is configured in the config endpoint. Any value set here will override that value.")
Expand Down Expand Up @@ -69,7 +81,43 @@ public class AnalyticsArgs implements AutoloadedConfigurations {
* @throws Exception if there was a problem determining the enabled status of analytics
*/
public static boolean isAnalyticsEnabled() throws Exception {
return false;
Boolean devOverride = DEV_OVERRIDE.getCurrentValue();
Logger log = Scope.getCurrentScope().getLog(AnalyticsArgs.class);
if (LiquibaseUtil.isDevVersion() && Boolean.FALSE.equals(devOverride)) {
log.severe("Analytics is disabled because this is not a release build and the user has not provided a value for the "+DEV_OVERRIDE.getKey()+" option.");
return false;
}
String configEndpointUrl = CONFIG_ENDPOINT_URL.getCurrentValue();
if (Boolean.TRUE.equals(devOverride) && CONFIG_ENDPOINT_URL.getDefaultValue().equals(configEndpointUrl)) {
log.severe("Analytics is disabled because " + DEV_OVERRIDE.getKey() + " was set to true, but the default " +
"value was used for the " + CONFIG_ENDPOINT_URL.getKey() + " property. This is not permitted, because " +
"dev versions of Liquibase should not be pushing analytics towards the prod analytics stack. To resolve " +
"this, provide a value for " + CONFIG_ENDPOINT_URL.getKey() + " that is not the default value.");
return false;
}

// if the user set enabled to false, that overrides all
Boolean userSuppliedEnabled = ENABLED.getCurrentValue();
if (Boolean.FALSE.equals(userSuppliedEnabled)) {
log.log(LOG_LEVEL.getCurrentValue(), "User has disabled analytics.", null);
return false;
}

boolean proLicenseValid = LicenseServiceUtils.isProLicenseValid();
AnalyticsConfigurationFactory analyticsConfigurationFactory = Scope.getCurrentScope().getSingleton(AnalyticsConfigurationFactory.class);
if (proLicenseValid) {
Boolean enabled = BooleanUtils.and(new Boolean[]{analyticsConfigurationFactory.getPlugin().isProAnalyticsEnabled(), userSuppliedEnabled});
if (Boolean.FALSE.equals(enabled)) {
log.log(LOG_LEVEL.getCurrentValue(), "Analytics is disabled, because a pro license was detected and analytics was not enabled by the user or because it was turned off by Liquibase.", null);
}
return enabled;
} else {
boolean enabled = analyticsConfigurationFactory.getPlugin().isOssAnalyticsEnabled();
if (Boolean.FALSE.equals(enabled)) {
log.log(LOG_LEVEL.getCurrentValue(), "Analytics is disabled, because it was turned off by Liquibase.", null);
}
return enabled;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ private void verifySupportsMethodImplementation(Set<Change> plugins) {
try {
// if the supports method is not implemented in the plugin show the warning according to the defined level
if (plugin.getClass().getMethod("supports", Database.class).getDeclaringClass().getPackage().getName().startsWith("liquibase.change")) {
if (LiquibaseUtil.getBuildVersion().equals(LiquibaseUtil.DEV_VERSION)) {
if (LiquibaseUtil.isDevVersion()) {
throw new UnexpectedLiquibaseException(String.format(SUPPORTS_METHOD_REQUIRED_MESSAGE, plugin.getClass().getName()));
}
switch (GlobalConfiguration.SUPPORTS_METHOD_VALIDATION_LEVEL.getCurrentValue()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@
import liquibase.GlobalConfiguration;
import liquibase.Scope;
import liquibase.logging.Logger;
import liquibase.resource.ClassLoaderResourceAccessor;
import liquibase.resource.CompositeResourceAccessor;
import liquibase.resource.InputStreamList;
import liquibase.resource.Resource;
import liquibase.resource.ResourceAccessor;
import liquibase.util.LiquibaseUtil;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
Expand All @@ -16,7 +12,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.net.URLClassLoader;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -104,7 +99,7 @@ private void warnForMismatchedXsdVersion(String systemId) {
boolean found = versionMatcher.find();
if (found) {
String buildVersion = LiquibaseUtil.getBuildVersion();
if (!buildVersion.equals(LiquibaseUtil.DEV_VERSION)) {
if (!LiquibaseUtil.isDevVersion()) {
String xsdVersion = versionMatcher.group("version");
if (!buildVersion.startsWith(xsdVersion)) {
hasWarnedAboutMismatchedXsdVersion = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private void verifyPriorityMethodImplementedCorrectly(SnapshotGenerator generato
try {
int priority = generator.getPriority(null, new MockDatabase());
if (priority != PRIORITY_NONE) {
if (LiquibaseUtil.getBuildVersion().equals(LiquibaseUtil.DEV_VERSION)) {
if (LiquibaseUtil.isDevVersion()) {
throw new UnexpectedLiquibaseException(String.format(SUPPORTS_METHOD_REQUIRED_MESSAGE, generator.getClass().getName()));
}
switch (GlobalConfiguration.SUPPORTS_METHOD_VALIDATION_LEVEL.getCurrentValue()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ public static String getBuildVersion() {
return getBuildInfo("build.version");
}

public static boolean isDevVersion() {
return LiquibaseUtil.getBuildVersion().equals(LiquibaseUtil.DEV_VERSION);
}

/**
* Return the build version for release builds and a more descriptive string for snapshot builds.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@ import liquibase.Scope
import liquibase.analytics.configuration.AnalyticsArgs
import liquibase.analytics.configuration.AnalyticsConfiguration
import liquibase.analytics.configuration.AnalyticsConfigurationFactory
import spock.lang.Ignore
import spock.lang.Specification
import spock.lang.Unroll

class AnalyticsArgsTest extends Specification {

@Ignore
@Unroll
def "test all permutations of options for enabling/disabling for oss"(Boolean userCliOption, boolean remoteOssEnabled, boolean isEnabled) {
setup:
Expand Down Expand Up @@ -38,6 +36,8 @@ class AnalyticsArgsTest extends Specification {
when:
Map<String, ?> scopeKeys = new HashMap<>()
scopeKeys.put(AnalyticsArgs.ENABLED.getKey(), userCliOption)
scopeKeys.put(AnalyticsArgs.DEV_OVERRIDE.getKey(), true)
scopeKeys.put(AnalyticsArgs.CONFIG_ENDPOINT_URL.getKey(), "some other value")
Boolean actuallyEnabled = Scope.child(scopeKeys, () -> {
return AnalyticsArgs.isAnalyticsEnabled()
} as Scope.ScopedRunnerWithReturn)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ import org.apache.tools.ant.taskdefs.condition.Condition
class AnalyticsWebserverPostBodyIsAnt extends ProjectComponent implements Condition{
@Override
boolean eval() throws BuildException {
return TestAnalyticsWebserver.postBody.contains('liquibaseInterface": "ant')
return TestAnalyticsWebserver.postBody != null && TestAnalyticsWebserver.postBody.contains('liquibaseInterface": "ant')
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ class AntTestAnalyticsWebserver extends Task {
// We must use system properties, as they persist into the Ant execution.
System.setProperty(AnalyticsArgs.CONFIG_ENDPOINT_URL.getKey(), "http://localhost:" + testAnalyticsWebserver.getListeningPort() + "/config-analytics.yaml");
System.setProperty(AnalyticsArgs.CONFIG_ENDPOINT_TIMEOUT_MILLIS.getKey(), String.valueOf(TimeUnit.SECONDS.toMillis(60)));
System.setProperty(AnalyticsArgs.DEV_OVERRIDE.getKey(), true.toString())
} else {
System.clearProperty(AnalyticsArgs.CONFIG_ENDPOINT_URL.getKey())
System.clearProperty(AnalyticsArgs.CONFIG_ENDPOINT_TIMEOUT_MILLIS.getKey())
System.clearProperty(AnalyticsArgs.DEV_OVERRIDE.getKey())
testAnalyticsWebserver.stop()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
src="${liquibase.test.ant.basedir}/sql/h2-setup.sql"/>
<mkdir dir="${temp.dir}"/>
<lb:updateDatabase databaseref="test-db" changelogfile="${liquibase.test.ant.basedir}/changelog/simple-changelog.xml"/>
<db:startTestAnalyticsWebserver/>
</target>

<target name="tearDown">
<sql driver="${jdbc.driver}" url="${jdbc.url}" userid="${db.user}" password="${db.password}" encoding="UTF-8"
src="${liquibase.test.ant.basedir}/sql/h2-teardown.sql"/>
<delete dir="${temp.dir}"/>
<db:stopTestAnalyticsWebserver/>
</target>

<target name="testDropAllTask">
Expand All @@ -33,6 +35,7 @@
<lb:database driver="${jdbc.driver}" url="${jdbc.url}" user="${db.user}" password="${db.password}"/>
</lb:dropAllDatabaseObjects>
<db:assertTableDoesntExist driver="${jdbc.driver}" url="${jdbc.url}" user="${db.user}" password="${db.password}" table="TEST_USERS"/>
<db:assertAnalyticsPostBodyIsAnt/>
</target>

<target name="testDropAllTaskDatabaseRef">
Expand All @@ -41,5 +44,6 @@
<lb:dropAllDatabaseObjects databaseref="test-db"/>
<db:assertTableDoesntExist driver="${jdbc.driver}" url="${jdbc.url}" user="${db.user}" password="${db.password}"
table="TEST_USERS"/>
<db:assertAnalyticsPostBodyIsAnt/>
</target>
</project>

0 comments on commit 789c5e6

Please sign in to comment.