Skip to content

Commit

Permalink
FINERACT-2149: disable liquibase phone home
Browse files Browse the repository at this point in the history
At some point, Liquibase added an analytics/telemetry "phone home" to gather user data.

I get how companies and FOSS projects greatly benefit from knowing more about their users, but I think this should only be done with proper consent. I would not assume/expect any libraries to send analytics home. The default behavior of trackers like these should always be to *not* phone home without a user first consenting / opting in.

---

I'm not sure this is the right change, but it works (feedback welcome). My naïve test/check to know this works is visually inspecting the output of the log message I added in TenantDatabaseUpgradeService to see if `liquibase.analytics.enabled` is set, and to what. I'm happy to automate this, but it seemed unnecessary (again, feedback welcome).

We could also disable the phone home with an environment variable ( LIQUIBASE_ANALYTICS_ENABLED=false , see https://docs.liquibase.com/analytics/home.html ). I'm not sure where that should be set if we did. Likely several places (for build/dev/test/ci/qa/etc.).

I originally noticed the phone home because I block requests from my computer to 3rd party tracking/analytics services, and I was getting this error in my console:

    java.net.ConnectException: Connection refused at liquibase.analytics.LiquibaseAnalyticsListener

At first I misunderstood this as a database connection error, but it was really just liquibase throwing an error because it couldn't connect to its own analytics server.

If we were to create a configurable property, I think the ideal place to set the value would be in fineract-provider/src/main/resources/application.properties, (default to false, allow override) but our version of spring boot doesn't support it yet ( see spring-projects/spring-boot#43067 ) so we'd have to add the usual scaffolding required for a Fineract setting. I'm not sure we want/need that, because I can't imagine anyone *wanting* to set the flag to `true`, and my naïve hardcoded global change is less code to maintain than providing a configurable setting. If there's demand to make it configurable later, we could of course do that (especially once it becomes easier to do so). I feel like how we let Liquibase behave is up to us, though. An under the hood / upstream decision we get to make.

See also:

* https://lists.apache.org/thread/5s7zjsxdcqk1qvjnc1wbhqqv66shpl54
* Analytics is enabled by default in OSS version · Issue #6503 · liquibase/liquibase · GitHub liquibase/liquibase#6503
* Add a method for Spring Boot to configure the new analytics feature · Issue #6501 · liquibase/liquibase · GitHub liquibase/liquibase#6501
  • Loading branch information
meonkeys authored and adamsaghy committed Nov 27, 2024
1 parent bfae135 commit 4956bdd
Showing 1 changed file with 5 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ public class TenantDatabaseUpgradeService implements InitializingBean {
// DO NOT REMOVE! Required for liquibase custom task initialization
private final List<CustomTaskChange> customTaskChangesForDependencyInjection;

static {
System.setProperty("liquibase.analytics.enabled", "false");
}

@Override
public void afterPropertiesSet() throws Exception {
if (notLiquibaseOnlyMode()) {
Expand Down Expand Up @@ -117,6 +121,7 @@ private void logTenantStoreDetails() {
log.info("- fineract.tenant.description: {}", tenant.getDescription());
log.info("- fineract.tenant.identifier: {}", tenant.getIdentifier());
log.info("- fineract.tenant.name: {}", tenant.getName());
log.info("- liquibase.analytics.enabled: {}", System.getProperty("liquibase.analytics.enabled"));

String readOnlyUsername = tenant.getReadOnlyUsername();
if (isNotBlank(readOnlyUsername)) {
Expand Down

0 comments on commit 4956bdd

Please sign in to comment.