Port JVB configs to new metaconfig lib (#1351)

* add metaconfig dep

* (temporarily) add a new core JitsiConfig and a TypesafeConfigSource

eventually these will both be done in Jicoco

* port health config over to metaconfig

* port octo config to metaconfig

* fix octo config

* port expire thread config to metaconfig

* port endpointconnectionstatusconfig to metaconfig

* use NewJitsiConfig from jicoco

* inject new legacy config service from NewJitsiConfig

* port ice config to metaconfig

* fix iceconfig enum parsing

* fix ice config

* port bandwidthprobing config to metaconfig

* port xmpp client connection config to metaconfig

* port websocketserviceconfig to metaconfig

* port videobridge config to metaconfig

* change xhow mppclientconnectionconfig builds the client configs from a legacy source

* wip: port stats config

* port stats manager bundle activator config to metaconfig

* we no longer need to use new forks of the jvm to run tests (thanks to differences in metaconfig vs old config).

* remove unneeded NewTypesafeConfigSource (moved to jicoco)

* add OctoConfig tests, fix a bug with legacy 'enabled' prop definition

* add xmppclientconnectionconfigtests, fix bug for filtering incomplete configs

* tweak method used for ConfigTest

* point to jitpack for metaconfig

* remove config debug logs

* port transportconfig

* port bitratecontrollerconfig

* port jvbapiconfig

* fixup: port bitratecontrollerconfig cont.

* update to new config class names in jicoco

* updates tests to new config class names in jicoco

* fix name of StatsManagerBundleActivatorConfig

* fix name of StatsManagerBundleActivatorConfig

* fix typos

* revert back to using string/conversion for enums (see jitsi/jitsi-metaconfig#14)

* add missing retrieve in nominationStrategy config

* remove (old) reload of config before initializing ice4j

the new legacy shim acts like the old one, so it actively checks the
system properties when retrieving (whereas the old-new config cached
them) so this is no longer necessary.

* remove (old) reload before starting jvb

the (old-new) config cached system properties on start, so a reload was
needed before reading them if we change them.  the new legacy config
shim still reads system props on demand, so the reload isn't needed.

* update XmppClientConnectionConfig to new config syntax

* update WebsocketServiceConfig to new config syntax

* update EndpointConnectionStatusConfig to new config syntax

* update VideobridgeConfig to new config syntax

* update VideobridgeExpireThreadConfig to new config syntax

* update BandwidthProbingConfig to new config syntax

* update BitrateControllerConfig to new config syntax

* update HealthConfig to new config syntax

* update IceConfig to new config syntax

* update OctoConfig to new config syntax

* update StatsManagerBundleActivatorConfig to new config syntax

* update jmc version

* update jicoco version

* update jmt dep

* remove extra spaces

* move iceconfig member to iceconfig class

* re-add license header

* wire up metaconfig logger

jvb/pom.xml

@@ -154,6 +154,10 @@
       <groupId>${project.groupId}</groupId>
       <artifactId>jitsi-utils-kotlin</artifactId>
     </dependency>
+    <dependency>
+        <groupId>${project.groupId}</groupId>
+        <artifactId>jitsi-metaconfig</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.osgi</groupId>
       <artifactId>org.osgi.core</artifactId>
@@ -284,7 +288,7 @@
       <scope>test</scope>
     </dependency>
     <dependency>
-      <groupId>org.jitsi</groupId>
+      <groupId>${project.groupId}</groupId>
       <artifactId>jicoco-test-kotlin</artifactId>
       <version>${jicoco.version}</version>
       <scope>test</scope>
@@ -488,9 +492,6 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
         <version>2.22.2</version>
-        <configuration>
-          <reuseForks>false</reuseForks>
-        </configuration>
       </plugin>
       <plugin>
         <groupId>com.github.spotbugs</groupId>

jvb/src/main/java/org/jitsi/videobridge/Endpoint.java

@@ -294,7 +294,7 @@ public void trace(@NotNull Function0<Unit> f)
             getClass().getSimpleName() + "-outgoing-packet-queue",
             TaskPools.IO_POOL,
             this::doSendSrtp,
-            TransportConfig.Config.queueSize()
+            TransportConfig.getQueueSize()
         );
         outgoingSrtpPacketQueue.setErrorHandler(queueErrorCounter);
 

jvb/src/main/java/org/jitsi/videobridge/EndpointConnectionStatus.java

@@ -26,16 +26,15 @@
 import java.util.*;
 import java.util.stream.*;
 
-import static org.jitsi.videobridge.EndpointConnectionStatusConfig.*;
-
 /**
  * This module monitors all endpoints across all conferences currently hosted
  * on the bridge for their connectivity status and sends notifications through
  * the data channel.
  *
  * An endpoint's connectivity status is considered connected as long as there
  * is any traffic activity seen on any of its endpoints. When there is no
- * activity for longer than the value of {@link Config#getMaxInactivityLimit()}, it
+ * activity for longer than the value of
+ * {@link EndpointConnectionStatusConfig#getMaxInactivityLimit()}, it
  * will be assumed that the endpoint is having some connectivity issues. Those
  * may be temporary or permanent. When that happens there will be a Colibri
  * message broadcast to all conference endpoints. The Colibri class name of
@@ -78,6 +77,8 @@ public class EndpointConnectionStatus
      */
     private Timer timer;
 
+    private final EndpointConnectionStatusConfig config = new EndpointConnectionStatusConfig();
+
     /**
      * The {@link Clock} used by this class
      */
@@ -123,12 +124,12 @@ public void run()
             logger.error("Endpoint connection monitoring is already running");
         }
 
-        if (Config.getFirstTransferTimeout().compareTo(Config.getMaxInactivityLimit()) <= 0)
+        if (config.getFirstTransferTimeout().compareTo(config.getMaxInactivityLimit()) <= 0)
         {
             throw new IllegalArgumentException(
                 String.format("first transfer timeout(%s) must be greater"
                             + " than max inactivity limit(%s)",
-                        Config.getFirstTransferTimeout(), Config.getMaxInactivityLimit()));
+                        config.getFirstTransferTimeout(), config.getMaxInactivityLimit()));
         }
 
         super.start(bundleContext);
@@ -206,7 +207,7 @@ private void monitorEndpointActivity(AbstractEndpoint abstractEndpoint)
             // We're doing that by checking how much time has elapsed since
             // the first endpoint's channel has been created.
             Duration timeSinceCreated = Duration.between(mostRecentChannelCreated, now);
-            if (timeSinceCreated.compareTo(Config.getFirstTransferTimeout()) > 0)
+            if (timeSinceCreated.compareTo(config.getFirstTransferTimeout()) > 0)
             {
                 if (logger.isDebugEnabled())
                     logger.debug(
@@ -228,7 +229,7 @@ private void monitorEndpointActivity(AbstractEndpoint abstractEndpoint)
         }
 
         Duration noActivityTime = Duration.between(lastActivity, now);
-        boolean inactive = noActivityTime.compareTo(Config.getMaxInactivityLimit()) > 0;
+        boolean inactive = noActivityTime.compareTo(config.getMaxInactivityLimit()) > 0;
         boolean changed = false;
         synchronized (inactiveEndpoints)
         {

jvb/src/main/java/org/jitsi/videobridge/Main.java

@@ -15,9 +15,12 @@
  */
 package org.jitsi.videobridge;
 
+import kotlin.jvm.functions.*;
+import org.jetbrains.annotations.*;
 import org.jitsi.cmd.*;
-import org.jitsi.config.*;
 import org.jitsi.meet.*;
+import org.jitsi.metaconfig.*;
+import org.jitsi.utils.logging2.*;
 import org.jitsi.videobridge.osgi.*;
 
 /**
@@ -59,6 +62,8 @@ public static void main(String[] args)
 
         cmdLine.parse(args);
 
+        setupMetaconfigLogger();
+
         // Parse the command-line arguments.
         String apis = cmdLine.getOptionValue(APIS_ARG_NAME);
 
@@ -77,12 +82,33 @@ public static void main(String[] args)
                 Videobridge.REST_API_PNAME,
                 Boolean.toString(apis.contains(Videobridge.REST_API)));
 
-        // Need to force a reload to see the updated system properties
-        JitsiConfig.Companion.reload();
-
         ComponentMain main = new ComponentMain();
         BundleConfig osgiBundles = new BundleConfig();
 
         main.runMainProgramLoop(osgiBundles);
     }
+
+    private static void setupMetaconfigLogger() {
+        Logger configLogger = new LoggerImpl("org.jitsi.config");
+        MetaconfigSettings.Companion.setLogger(new MetaconfigLogger()
+        {
+            @Override
+            public void warn(@NotNull Function0<String> function0)
+            {
+                configLogger.warn(function0::invoke);
+            }
+
+            @Override
+            public void error(@NotNull Function0<String> function0)
+            {
+                configLogger.error(function0::invoke);
+            }
+
+            @Override
+            public void debug(@NotNull Function0<String> function0)
+            {
+                configLogger.debug(function0::invoke);
+            }
+        });
+    }
 }

jvb/src/main/java/org/jitsi/videobridge/TransportConfig.kt

@@ -15,24 +15,13 @@
  */
 package org.jitsi.videobridge
 
-import org.jitsi.config.newConfigAttributes
-import org.jitsi.utils.config.SimpleProperty
+import org.jitsi.config.JitsiConfig
+import org.jitsi.metaconfig.config
+import org.jitsi.metaconfig.from
 
 class TransportConfig {
-    class Config {
-        companion object {
-
-            class QueueSizeProperty : SimpleProperty<Int>(
-                    newConfigAttributes {
-                        name("videobridge.transport.send.queue-size")
-                        readOnce()
-                    }
-            )
-
-            private val queueSizeProperty = QueueSizeProperty()
-
-            @JvmStatic
-            fun queueSize() = queueSizeProperty.value
-        }
+    companion object {
+        @JvmStatic
+        val queueSize: Int by config("videobridge.transport.send.queue-size".from(JitsiConfig.newConfig))
     }
 }

jvb/src/main/java/org/jitsi/videobridge/VideoConstraints.java

@@ -29,7 +29,7 @@ public class VideoConstraints
      * Static instance for the default constraints for a thumbnail.
      */
     public static final VideoConstraints thumbnailVideoConstraints =
-        new VideoConstraints(BitrateControllerConfig.Config.thumbnailMaxHeightPx());
+        new VideoConstraints(BitrateControllerConfig.thumbnailMaxHeightPx());
 
     /**
      * The ideal height of the constrained endpoint. The bridge tries to send an

jvb/src/main/java/org/jitsi/videobridge/VideoConstraintsCompatibility.java

@@ -104,7 +104,7 @@ Map<String, VideoConstraints> computeVideoConstraints()
         {
             final VideoConstraints pinnedEndpointConstraints
                 = new VideoConstraints(Math.min(
-                BitrateControllerConfig.Config.thumbnailMaxHeightPx(), maxFrameHeightCopy));
+                BitrateControllerConfig.thumbnailMaxHeightPx(), maxFrameHeightCopy));
 
             Map<String, VideoConstraints> pinnedVideoConstraintsMap
                 = pinnedEndpointsCopy
@@ -144,16 +144,16 @@ Map<String, VideoConstraints> computeVideoConstraints()
                 // nor the preferred frame-rate because we want even even
                 // distribution of bandwidth among all the tiles to avoid ninjas.
                 selectedEndpointConstraints = new VideoConstraints(
-                    Math.min(BitrateControllerConfig.Config.onstageIdealHeightPx(),
+                    Math.min(BitrateControllerConfig.onstageIdealHeightPx(),
                         maxFrameHeightCopy));
             }
             else
             {
                 selectedEndpointConstraints = new VideoConstraints(
-                    Math.min(BitrateControllerConfig.Config.onstageIdealHeightPx(),
+                    Math.min(BitrateControllerConfig.onstageIdealHeightPx(),
                         maxFrameHeightCopy),
-                    BitrateControllerConfig.Config.onstagePreferredHeightPx(),
-                    BitrateControllerConfig.Config.onstagePreferredFramerate());
+                    BitrateControllerConfig.onstagePreferredHeightPx(),
+                    BitrateControllerConfig.onstagePreferredFramerate());
             }
 
             Map<String, VideoConstraints> selectedVideoConstraintsMap

jvb/src/main/java/org/jitsi/videobridge/Videobridge.java

@@ -138,6 +138,8 @@ public class Videobridge
      */
     private VideobridgeExpireThread videobridgeExpireThread;
 
+    private final VideobridgeConfig config = new VideobridgeConfig();
+
     /**
      * The shim which handles Colibri-related logic for this
      * {@link Videobridge}.
@@ -484,13 +486,13 @@ private String getHealthStatus()
     public IQ handleShutdownIQ(ShutdownIQ shutdownIQ)
     {
         // Security not configured - service unavailable
-        if (shutdownSourcePattern == null)
+        if (config.getShutdownSourcePattern() == null)
         {
             return IQUtils.createError(shutdownIQ, XMPPError.Condition.service_unavailable);
         }
         // Check if source matches pattern
         Jid from = shutdownIQ.getFrom();
-        if (from != null && shutdownSourcePattern.matcher(from).matches())
+        if (from != null && config.getShutdownSourcePattern().matcher(from).matches())
         {
             logger.info("Accepted shutdown request from: " + from);
             if (shutdownIQ.isGracefulShutdown())
@@ -573,29 +575,8 @@ void start(final BundleContext bundleContext)
 
         UlimitCheck.printUlimits();
 
-        ConfigurationService cfg = getConfigurationService();
-
         videobridgeExpireThread.start();
 
-        String shutdownSourcesRegexp
-            = (cfg == null)
-                ? null
-                : cfg.getString(SHUTDOWN_ALLOWED_SOURCE_REGEXP_PNAME);
-
-        if (!StringUtils.isBlank(shutdownSourcesRegexp))
-        {
-            try
-            {
-                shutdownSourcePattern = Pattern.compile(shutdownSourcesRegexp);
-            }
-            catch (PatternSyntaxException exc)
-            {
-                logger.error(
-                   "Error parsing enableGracefulShutdownMode sources reg expr: "
-                        + shutdownSourcesRegexp, exc);
-            }
-        }
-
         // <conference>
         ProviderManager.addIQProvider(
                 ColibriConferenceIQ.ELEMENT_NAME,
@@ -633,6 +614,7 @@ void start(final BundleContext bundleContext)
                 HealthCheckIQ.NAMESPACE,
                 new HealthCheckIQProvider());
 
+        ConfigurationService cfg = getConfigurationService();
         startIce4j(bundleContext, cfg);
     }
 
@@ -668,9 +650,6 @@ private void startIce4j(
                     }
                 }
             }
-
-            // Reload for all the new system properties to be seen
-            JitsiConfig.Companion.reload();
         }
 
         // Initialize the the host candidate interface filters in the ice4j

jvb/src/main/java/org/jitsi/videobridge/VideobridgeExpireThread.java

@@ -19,14 +19,10 @@
 import java.util.*;
 import java.util.concurrent.*;
 
-import org.jitsi.osgi.*;
-import org.jitsi.service.configuration.*;
 import org.jitsi.utils.concurrent.*;
 import org.jitsi.utils.logging2.*;
 import org.osgi.framework.*;
 
-import static org.jitsi.videobridge.VideobridgeExpireThreadConfig.*;
-
 /**
  * Implements a <tt>Thread</tt> which expires the {@link AbstractEndpoint}s and
  * {@link Conference}s of a specific <tt>Videobridge</tt>.
@@ -70,6 +66,8 @@ public class VideobridgeExpireThread
      */
     private Videobridge videobridge;
 
+    public static final VideobridgeExpireThreadConfig config = new VideobridgeExpireThreadConfig();
+
     /**
      * Initializes a new {@link VideobridgeExpireThread} instance which is to
      * expire the {@link Conference}s of a specific {@link Videobridge}.
@@ -88,7 +86,7 @@ public VideobridgeExpireThread(Videobridge videobridge)
      */
     void start()
     {
-        Duration expireCheckSleepDuration = Config.interval();
+        Duration expireCheckSleepDuration = config.getInterval();
         logger.info(
             "Starting with " + expireCheckSleepDuration.getSeconds() + " second interval.");
 

jvb/src/main/java/org/jitsi/videobridge/cc/BandwidthProbing.java

@@ -19,12 +19,11 @@
 import org.jitsi.nlj.util.*;
 import org.jitsi.utils.concurrent.*;
 import org.jitsi.utils.logging.*;
+import org.jitsi.videobridge.cc.config.*;
 import org.json.simple.*;
 
 import java.util.*;
 
-import static org.jitsi.videobridge.cc.config.BandwidthProbingConfig.*;
-
 /**
   * @author George Politis
   */
@@ -63,13 +62,15 @@ public class BandwidthProbing
 
      private ProbingDataSender probingDataSender;
 
+     private static final BandwidthProbingConfig config = new BandwidthProbingConfig();
+
      /**
       * Ctor.
       *
       */
      public BandwidthProbing(ProbingDataSender probingDataSender)
      {
-         super(Config.paddingPeriodMs());
+         super(config.getPaddingPeriodMs());
          this.probingDataSender = probingDataSender;
      }
 
@@ -156,7 +157,7 @@ public void run()
 
          // XXX a signed int is practically sufficient, as it can represent up to
          // ~ 2GB
-         int bytes = (int) (Config.paddingPeriodMs() * paddingBps / 1000 / 8);
+         int bytes = (int) (config.getPaddingPeriodMs() * paddingBps / 1000 / 8);
 
          if (!bitrateControllerStatus.activeSsrcs.isEmpty())
          {