You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "vidakovic (via GitHub)" <gi...@apache.org> on 2023/11/06 14:13:58 UTC

[PR] FINERACT-1484: Micrometer Monitoring [fineract]

vidakovic opened a new pull request, #3568:
URL: https://github.com/apache/fineract/pull/3568

   Add Micrometer monitoring with default configuration for:
   - Prometheus
   - Loki
   - Grafana
   - Tempo
   
   Make Docker Compose configuration more modular.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1484: Micrometer Monitoring [fineract]

Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on code in PR #3568:
URL: https://github.com/apache/fineract/pull/3568#discussion_r1383754429


##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/exception/ExternalEventConfigurationNotFoundException.java:
##########
@@ -21,7 +21,7 @@
 public class ExternalEventConfigurationNotFoundException extends RuntimeException {
 
     public ExternalEventConfigurationNotFoundException() {
-        super("All external events are not configured");
+        super("No external events configured");

Review Comment:
   Renaming the exception doesn't seem to have any unintended side effect... I did it, because "All ... are not" is simply just "No...". It's just a question of brevity... I think you would say "All external events are configured" for the positive case, but "No external events configured" for the negative case (i. e. this failure/exception situation). "All external events are not configured" sounds a bit weird.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1484: Micrometer Monitoring [fineract]

Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on PR #3568:
URL: https://github.com/apache/fineract/pull/3568#issuecomment-1795619621

   > Still need to verify that the docker-compose.yml file is working... at the moment failing with this message:
   > 
   > ```
   > Caused by: org.apache.fineract.infrastructure.event.external.exception.ExternalEventConfigurationNotFoundException: All external events are not configured
   > ```
   
   Found the reason why this is not working. Gradle cache needs to be removed with `rm -rf .gradle` and then execute `./gradlew :fineract-provider:clean :fineract-provider:jibDockerBuild -x tes`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1484: Micrometer Monitoring [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #3568:
URL: https://github.com/apache/fineract/pull/3568#discussion_r1386711368


##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/exception/ExternalEventConfigurationNotFoundException.java:
##########
@@ -21,7 +21,7 @@
 public class ExternalEventConfigurationNotFoundException extends RuntimeException {
 
     public ExternalEventConfigurationNotFoundException() {
-        super("All external events are not configured");
+        super("No external events configured");

Review Comment:
   I would rather with "Not all external events are configured" because that is the closer to the truth.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1484: Micrometer Monitoring [fineract]

Posted by "reluxa (via GitHub)" <gi...@apache.org>.
reluxa commented on code in PR #3568:
URL: https://github.com/apache/fineract/pull/3568#discussion_r1383741686


##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/exception/ExternalEventConfigurationNotFoundException.java:
##########
@@ -21,7 +21,7 @@
 public class ExternalEventConfigurationNotFoundException extends RuntimeException {
 
     public ExternalEventConfigurationNotFoundException() {
-        super("All external events are not configured");
+        super("No external events configured");

Review Comment:
   https://github.com/apache/fineract/blob/2b4c932ddd62a50c686be314c442aafa1bae2eee/fineract-core/src/main/java/org/apache/fineract/infrastructure/event/external/service/ExternalEventConfigurationValidationService.java#L78-L80
   
   This exception is thrown, when number of events found on the classpath are not matching with the number of the events found in the database for the tenant. So I am not sure that the `No external events configured` is good here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1484: Micrometer Monitoring [fineract]

Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on PR #3568:
URL: https://github.com/apache/fineract/pull/3568#issuecomment-1794925336

   Still need to verify that the docker-compose.yml file is working... at the moment failing with this message:
   
   ```
   Caused by: org.apache.fineract.infrastructure.event.external.exception.ExternalEventConfigurationNotFoundException: All external events are not configured
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1484: Micrometer Monitoring [fineract]

Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic merged PR #3568:
URL: https://github.com/apache/fineract/pull/3568


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1484: Micrometer Monitoring [fineract]

Posted by "vidakovic (via GitHub)" <gi...@apache.org>.
vidakovic commented on PR #3568:
URL: https://github.com/apache/fineract/pull/3568#issuecomment-1795848436

   @reluxa ... you were right... there was a test checking for that exception message... fixed it. Thanks for the help.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org