Skip to content

Commit 676fb3d

Browse files
authored
Config phase 1 (#939)
* add jvb new config helper classes * migrate health interval to new property type * add config readme * dynamically retrieve the confiig inside the suppliiers this is important, because we replace the config instances on config reload, so we need to make sure we get the most up-to-date instance * initialize the jvb config in main * fix accidental not-found-strategy for health interval property * add unit test for healthintervalproperty * config readme tweak * change where we log a found legacy config with this change we no longer log an empty legacy config if one wasn't actually found * reorganize some logs around config loading, throw if videobridge config isn't found * config readme tweaks * more config readme tweaks * add traiiling newline to reference.conf * add jvbconfig.init to focuscontroltest this is needed because the tests don't run Main, where the configuration is normally initialized. i looked at moving this code to the start method of Videobridge, but that won't work either as this isn't run before main and main needs the config as well. i could have it in both places, but worried about double-initializing config during normal run as that effectively re-loads the config. * always add at least an empty new config in prop tests this is needed since we now throw if the 'videobridge' config block isn't found * init config in bridgeshutdowntest * no longer require an explicit loading of jvb config i had previously made this explicit mainly to fix some issues with unit tests, but also because i thought it would be better to make it clearer when the config files were being loaded. unfortunately, this caused issues with some flows (tests, mainly, but also loading videobridge as an osgi service). the config load can't be put in the videbridge osgi loader, as Main doesn't use that before loading some config it wants. this also causes issues with jicofo which loads jvb as a library in its tests. anyway, the test config loading has changed a bit and works fine with this method (implicit initialization of config) so we'll try this and adapt in the future if need be. * add config validation * update jitsi-utils * minor version bump
1 parent 0ed9a9f commit 676fb3d

17 files changed

+742
-27
lines changed

CONFIG.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# Configuration
2+
3+
##### 10/14/19
4+
We've begun the migration to a new configuration framework. The 'old' method (the `sip-communicator.properties` file and program arguments) will be supported throughout the transition and the bridge will be fully backwards-compatible with the old config. No changes are necessary until we pull the plug on the old config completely, and we'll give ample warning before that time. It's also worth noting that any new properties will only be supported in the new config files.
5+
6+
Moving forward, we'll be using lightbend's [config library]([https://github.com/lightbend/config](https://github.com/lightbend/config)). Their GitHub page gives lots of details, but below are the changes most relevant to configuring the JVB.
7+
8+
### Configuration sources
9+
During the transition, we'll continue to read from the original locations (`sip-communicator.properties` and program arguments) for all configuration properties, falling back to the new `reference.conf` file for defaults. The `reference.conf` file holds defaults for all configuration properties, and shouldn't be modified in a deployment--though it may be interesting for developers who want to see the defaults.
10+
11+
To override values, users can either put a file named `application.conf` in the classpath or pass a path to a file as a system property via `-Dconfig.file=/path/to/config.conf`. Values in the file given will override ones in `reference.conf`. The lightbend docs do a good job of describing [how configuration is loaded]([https://github.com/lightbend/config#standard-behavior](https://github.com/lightbend/config#standard-behavior)), but here's an example of overriding a configuration value for JVB:
12+
13+
### Overriding default values
14+
Say we want to override some of the default values in `reference.conf`, namely: the XMPP API server values, the health check interval, and whether or not onstage video suspension is enabled. We'll create an `application.conf` files with the values we want to change:
15+
16+
```
17+
videobridge {
18+
health {
19+
// Override the health check interval
20+
interval=60 seconds
21+
}
22+
cc {
23+
// Override the onstage video suspension setting
24+
onstage-video-suspension-enabled=true
25+
}
26+
}
27+
```
28+
You can override just the values you want to change, all others will fallback to the defaults set in `reference.conf`
29+
30+
### Migrating from old config
31+
Below are the mappings from old property values to new ones. Note: for new property names, the 'flat paths' are given, i.e. a value of `videobridge.health.interval` corresponds to the setting:
32+
```
33+
videobridge {
34+
health {
35+
interval
36+
}
37+
}
38+
```
39+
This list will be updated as properties are migrated:
40+
41+
| Old property name | New property name | Notes |
42+
| -------- | ------- | ------- |
43+
| org.jitsi.videobridge.health.INTERVAL | videobridge.health.interval | The new config models this as a duration, rather than an amount of milliseconds |

pom.xml

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
<groupId>org.jitsi</groupId>
88
<artifactId>jitsi-videobridge</artifactId>
9-
<version>2.0-SNAPSHOT</version>
9+
<version>2.1-SNAPSHOT</version>
1010
<packaging>jar</packaging>
1111

1212
<name>jitsi-videobridge</name>
@@ -80,11 +80,16 @@
8080
<artifactId>annotations</artifactId>
8181
<version>15.0</version>
8282
</dependency>
83+
<dependency>
84+
<groupId>com.typesafe</groupId>
85+
<artifactId>config</artifactId>
86+
<version>1.3.4</version>
87+
</dependency>
8388
<!-- org.jitsi -->
8489
<dependency>
8590
<groupId>${project.groupId}</groupId>
8691
<artifactId>jitsi-media-transform</artifactId>
87-
<version>1.0-57-g690bd5d</version>
92+
<version>1.0-58-g13fe894</version>
8893
</dependency>
8994
<dependency>
9095
<groupId>${project.groupId}</groupId>
@@ -125,7 +130,7 @@
125130
<dependency>
126131
<groupId>${project.groupId}</groupId>
127132
<artifactId>jitsi-utils</artifactId>
128-
<version>1.0-19-ge5cd0ce</version>
133+
<version>1.0-20-g37eacac</version>
129134
</dependency>
130135
<dependency>
131136
<groupId>${project.groupId}</groupId>
@@ -186,6 +191,11 @@
186191
<artifactId>javassist</artifactId>
187192
<version>3.22.0-CR2</version>
188193
</dependency>
194+
<dependency>
195+
<groupId>org.reflections</groupId>
196+
<artifactId>reflections</artifactId>
197+
<version>0.9.11</version>
198+
</dependency>
189199

190200
<!-- runtime -->
191201
<dependency>
@@ -245,12 +255,6 @@
245255
<version>4.11</version>
246256
<scope>test</scope>
247257
</dependency>
248-
<dependency>
249-
<groupId>org.reflections</groupId>
250-
<artifactId>reflections</artifactId>
251-
<version>0.9.11</version>
252-
<scope>test</scope>
253-
</dependency>
254258
</dependencies>
255259

256260
<build>

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@
1717

1818
import org.jitsi.cmd.*;
1919
import org.jitsi.meet.*;
20+
import org.jitsi.utils.config.validation.*;
2021
import org.jitsi.videobridge.osgi.*;
22+
import org.jitsi.videobridge.util.config.*;
2123
import org.jitsi.videobridge.xmpp.*;
2224

25+
import java.util.*;
26+
import java.util.stream.*;
27+
2328
/**
2429
* Provides the <tt>main</tt> entry point of the Jitsi Videobridge application
2530
* which implements an external Jabber component.
@@ -98,6 +103,7 @@ public class Main
98103
public static void main(String[] args)
99104
throws Exception
100105
{
106+
validateConfig();
101107
CmdLine cmdLine = new CmdLine();
102108

103109
cmdLine.parse(args);
@@ -156,4 +162,14 @@ public static void main(String[] args)
156162
main.runMainProgramLoop(osgiBundles);
157163
}
158164
}
165+
166+
protected static void validateConfig()
167+
{
168+
ConfigValidator configValidator = new ConfigValidator("org.jitsi");
169+
//TODO: pass command-line args as well
170+
Set<String> configPropNames = JvbConfig.getConfig().withOnlyPath("videobridge").entrySet().stream()
171+
.map(Map.Entry::getKey)
172+
.collect(Collectors.toSet());
173+
configValidator.validate(configPropNames);
174+
}
159175
}

src/main/java/org/jitsi/videobridge/health/Health.java

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,18 +52,6 @@ public class Health
5252
private static final RecurringRunnableExecutor executor
5353
= new RecurringRunnableExecutor(Health.class.getName());
5454

55-
/**
56-
* The default interval between health checks.
57-
*/
58-
private static final int PERIOD_DEFAULT = 10000;
59-
60-
/**
61-
* The name of the property which configures the interval between health
62-
* checks.
63-
*/
64-
public static final String PERIOD_PNAME
65-
= "org.jitsi.videobridge.health.INTERVAL";
66-
6755
/**
6856
* The default timeout for health checks.
6957
*/
@@ -286,18 +274,13 @@ private static String generateEndpointID()
286274
*/
287275
public Health(Videobridge videobridge, ConfigurationService cfg)
288276
{
289-
super(videobridge, PERIOD_DEFAULT, true);
277+
super(videobridge, HealthIntervalProperty.getValue(), true);
290278

291279
if (cfg == null)
292280
{
293281
logger.warn("Configuration service is null, using only defaults.");
294282
}
295283

296-
int period =
297-
cfg == null ? PERIOD_DEFAULT
298-
: cfg.getInt(PERIOD_PNAME, PERIOD_DEFAULT);
299-
setPeriod(period);
300-
301284
timeout =
302285
cfg == null ? TIMEOUT_DEFAULT
303286
: cfg.getInt(TIMEOUT_PNAME, TIMEOUT_DEFAULT);
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright @ 2018 - present 8x8, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.jitsi.videobridge.health;
18+
19+
import org.jitsi.utils.config.*;
20+
import org.jitsi.videobridge.util.config.*;
21+
22+
import java.util.concurrent.*;
23+
24+
25+
/**
26+
* The property which configures the interval between health
27+
* checks.
28+
*/
29+
public class HealthIntervalProperty extends AbstractConfigProperty<Integer>
30+
{
31+
protected static final String legacyPropName = "org.jitsi.videobridge.health.INTERVAL";
32+
protected static final String propName = "videobridge.health.interval";
33+
34+
private static HealthIntervalProperty singleton = new HealthIntervalProperty();
35+
36+
protected HealthIntervalProperty()
37+
{
38+
super(new JvbPropertyConfig<Integer>()
39+
.fromLegacyConfig(config -> config.getInt(legacyPropName))
40+
.fromNewConfig(config -> (int)config.getDuration(propName, TimeUnit.MILLISECONDS))
41+
.readOnce()
42+
.throwIfNotFound()
43+
);
44+
}
45+
46+
public static Integer getValue()
47+
{
48+
return singleton.get();
49+
}
50+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Copyright @ 2018 - present 8x8, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.jitsi.videobridge.util.config;
18+
19+
import org.jitsi.cmd.*;
20+
import org.jitsi.utils.config.*;
21+
22+
import java.util.function.*;
23+
24+
/**
25+
* A supplier which reads from the parsed command-line arguments.
26+
*
27+
* @param <T> the type of the configuration property's value
28+
*/
29+
public class CommandLineConfigValueSupplier<T> implements Supplier<T>
30+
{
31+
protected final String argName;
32+
protected final Function<CmdLine, T> getter;
33+
34+
/**
35+
* This method is only capable returning strings, so we'll rely on the fact
36+
* that T has been properly set to String and perform an unchecked cast
37+
*
38+
* @param argName the name of the command-line argument
39+
*/
40+
@SuppressWarnings("unchecked")
41+
public CommandLineConfigValueSupplier(String argName)
42+
{
43+
this(argName, cmdLine -> (T)cmdLine.getOptionValue(argName, null));
44+
}
45+
46+
public CommandLineConfigValueSupplier(String argName, Function<CmdLine, T> getter)
47+
{
48+
this.argName = argName;
49+
this.getter = getter;
50+
}
51+
52+
@Override
53+
public T get()
54+
{
55+
String[] commandLineArgs = JvbConfig.getCommandLineArgs();
56+
CmdLine cmdLine = new CmdLine();
57+
try
58+
{
59+
cmdLine.parse(commandLineArgs);
60+
T value = getter.apply(cmdLine);
61+
if (value == null)
62+
{
63+
throw new ConfigPropertyNotFoundException(argName);
64+
}
65+
return value;
66+
}
67+
catch (ParseException ignored) { }
68+
69+
throw new ConfigPropertyNotFoundException(argName);
70+
}
71+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Copyright @ 2018 - present 8x8, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.jitsi.videobridge.util.config;
18+
19+
import com.typesafe.config.*;
20+
21+
import java.util.function.*;
22+
23+
/**
24+
* A helper class which contains specifically where to pull 'new' config
25+
* values from.
26+
*
27+
* @param <T> the type of the configuration property's value
28+
*/
29+
public class ConfigValueSupplier<T> implements Supplier<T>
30+
{
31+
protected final Supplier<T> typesafeConfigSupplier;
32+
33+
/**
34+
* Create a {@link ConfigValueSupplier}
35+
*
36+
* @param getter a function which will be passed a {@link Config}
37+
* instance and should return the value for the
38+
* configuration property for which this supplier
39+
* was created.
40+
*/
41+
public ConfigValueSupplier(Function<Config, T> getter)
42+
{
43+
this.typesafeConfigSupplier =
44+
new TypesafeConfigValueSupplier<>(() -> getter.apply(JvbConfig.getConfig()));
45+
}
46+
47+
/**
48+
* Get the value of the configuration property for which
49+
* this supplier was created
50+
*
51+
* @return the configuration property's value, or
52+
* {@link org.jitsi.utils.config.ConfigPropertyNotFoundException} if
53+
* the property was not found
54+
*/
55+
@Override
56+
public T get()
57+
{
58+
return typesafeConfigSupplier.get();
59+
}
60+
}

0 commit comments

Comments
 (0)