You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "royteeuwen (via GitHub)" <gi...@apache.org> on 2026/05/24 18:54:33 UTC
[PR] SLING-13212: Add LogStoreListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
royteeuwen opened a new pull request, #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33
(no comment)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-13212: Add LogEntryListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33#issuecomment-4699511044
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log&pullRequest=33) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log&pullRequest=33)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-13212: Add LogEntryListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
Posted by "rombert (via GitHub)" <gi...@apache.org>.
rombert commented on code in PR #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33#discussion_r3381479484
##########
src/main/java/org/apache/sling/commons/log/logback/store/package-info.java:
##########
@@ -23,7 +23,7 @@
* <p>The API is intentionally decoupled from slf4j and logback to prevent binding to those APIs
* directly, which hinders evolution and is problematic when upgrading dependency versions.</p>
*/
-@Version("1.0.0")
+@Version("2.0.0")
Review Comment:
Now this we must avoid :-)
##########
src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImpl.java:
##########
@@ -47,6 +51,23 @@ public void append(LogEntry snapshot) {
entries.addLast(snapshot);
trimToSize();
}
+ // Notify listeners outside the lock so a slow or re-entrant listener does
+ // not stall other producers waiting to append.
+ for (LogEntryListener listener : listeners) {
+ listener.onEntry(snapshot);
+ }
+ }
+
+ public void addListener(LogEntryListener listener) {
+ if (listener != null) {
+ listeners.add(listener);
+ }
+ }
+
+ public void removeListener(LogEntryListener listener) {
+ if (listener != null) {
Review Comment:
Would it make sense to fail if the listener is `null` instead?
##########
src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImpl.java:
##########
@@ -47,6 +51,23 @@ public void append(LogEntry snapshot) {
entries.addLast(snapshot);
trimToSize();
}
+ // Notify listeners outside the lock so a slow or re-entrant listener does
+ // not stall other producers waiting to append.
+ for (LogEntryListener listener : listeners) {
+ listener.onEntry(snapshot);
+ }
+ }
+
+ public void addListener(LogEntryListener listener) {
+ if (listener != null) {
Review Comment:
Would it make sense to fail if the listener is `null` instead?
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-13212: Add LogEntryListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
Posted by "royteeuwen (via GitHub)" <gi...@apache.org>.
royteeuwen commented on PR #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33#issuecomment-4567541116
> Thanks for the PR @royteeuwen ! I would have two high-level comments.
>
> 1. I think LogStoreListener should not reference `LogStore` . The fact that we use the LogStore to propagate log entries is an implementation detail that might change. So it needs a new name .
> 2. There are two changes in this PR:
>
> 1. LogEntry changes (new fields) and LogStore config changes ( loggers property )
> 2. Main functionality to propagate log entries to registered listeners
>
> These should be split up in separate issues.
I split off the log store config changes to another PR: https://github.com/apache/sling-org-apache-sling-commons-log/pull/34
I kept the LogEntry changes, because the goal of this PR / ticket is to be able to easily produce telemetry data, which means we need more structured data in the LogEntry class. I did make it a separate commit though
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-13212: Add LogEntryListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
Posted by "royteeuwen (via GitHub)" <gi...@apache.org>.
royteeuwen commented on code in PR #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33#discussion_r3383581883
##########
src/main/java/org/apache/sling/commons/log/logback/store/package-info.java:
##########
@@ -23,7 +23,7 @@
* <p>The API is intentionally decoupled from slf4j and logback to prevent binding to those APIs
* directly, which hinders evolution and is problematic when upgrading dependency versions.</p>
*/
-@Version("1.0.0")
+@Version("2.0.0")
Review Comment:
@rombert Updated to 1.1.0 by adding a constructor for when it's still 7 params, can we live with that?
##########
src/main/java/org/apache/sling/commons/log/logback/store/package-info.java:
##########
@@ -23,7 +23,7 @@
* <p>The API is intentionally decoupled from slf4j and logback to prevent binding to those APIs
* directly, which hinders evolution and is problematic when upgrading dependency versions.</p>
*/
-@Version("1.0.0")
+@Version("2.0.0")
Review Comment:
@rombert Updated to 1.1.0 by adding a constructor for when it's still 7 params, can we live with that? If not, give other suggestions :D
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-13212: Add LogStoreListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33#issuecomment-4529684232
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log&pullRequest=33) **Quality Gate passed**
Issues
 [3 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [85.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log&pullRequest=33)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-13212: Add LogEntryListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
Posted by "royteeuwen (via GitHub)" <gi...@apache.org>.
royteeuwen merged PR #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-13212: Add LogEntryListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33#issuecomment-4663676199
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log&pullRequest=33) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [87.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log&pullRequest=33)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-13212: Add LogEntryListener that allows reading of the logs on a push-based API [sling-org-apache-sling-commons-log]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #33:
URL: https://github.com/apache/sling-org-apache-sling-commons-log/pull/33#issuecomment-4567570664
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log&pullRequest=33) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [88.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-commons-log&pullRequest=33&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-commons-log&pullRequest=33)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org