fix: enable Sentry logging support for jvb, jicofo, and jigasi#2229
fix: enable Sentry logging support for jvb, jicofo, and jigasi#2229hluaces wants to merge 1 commit intojitsi:masterfrom
Conversation
The logging.properties templates used toBool on the SENTRY_DSN variable, which is a URL, making it impossible to configure Sentry properly. The SENTRY_DSN default was also set to "0" which is truthy in Go templates when toBool is removed. Additionally, the sentry-jul JAR (containing the SentryHandler class) was missing from all three Docker images, causing a ClassNotFoundException at startup. I believe we are not trying to move this upstream (see jitsi/jitsi-videobridge#1785) but if we wanted to make this better we should be changing upstream so that the jul jars are also included. This won't work as-is until the docker images are built and published. An override file like this can be used to force to use local images to test it: ```yaml services: jicofo: build: context: ./jicofo args: BASE_TAG: unstable jvb: build: context: ./jvb args: BASE_TAG: unstable ```
|
Managed to get it working and shipping messages: 
|
|
If we are downloading it here, can we drop it from here: https://github.com/jitsi/jitsi-videobridge/blob/4e32a93889406114ab0b5d9410f7471ee60fc1e0/jvb/pom.xml#L214? |
Not the same thing. That's I'm not entirely sure on what upstream wants to do here. Provided that Sentry wants to be supported, the long term fix is to build If we prefer open PRs in the rest, it should be straightforward. |
|
What about adding and the other dependency here and getting rid of those dependencies in jicofo and jvb? |
|
Probably doable. We'd need to download both artifacts here. Also worth noting jigasi also uses the dependency. |
2 participants
The logging.properties templates used toBool on the SENTRY_DSN variable, which is a URL, making it impossible to configure Sentry properly. The SENTRY_DSN default was also set to "0" which is truthy in Go templates when toBool is removed.
Additionally, the sentry-jul JAR (containing the SentryHandler class) was missing from all three Docker images, causing a ClassNotFoundException at startup.
I believe we are not trying to move this upstream (see https://github.com/jitsi/jitsi videobridge/pull/1785) but if we wanted to make this better we should be changing upstream so that the jul jars are also included.
This won't work as-is until the docker images are built and published. An override file like this can be used to force to use local images to test it:
For: