Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PAYARA-4250 Prevent log spamming #4300

Merged
merged 2 commits into from Nov 11, 2019
Merged

PAYARA-4250 Prevent log spamming #4300

merged 2 commits into from Nov 11, 2019

Conversation

@jbee
Copy link
Contributor

jbee commented Nov 6, 2019

Summary

The goal of this change is to prevent potential log spamming caused by the collection of metrics every second in case exception is thrown by a source. This is achieved in two ways:

  • only log failing sources once (until they not fail any more)
  • only collect metrics when monitoring service is enabled

Solution

As discussed shortly during standup the solution to enable/disable via the existing setting was chosen to keep things simple both for the user and development.

The periodical collection task is submitted to the scheduler when monitoring is enabled initially or when a change event for the property changes is state to enabled. The task is cancelled should the state change to disabled.

Testing Performed

With deployed MC the configuration Monitoring => Monitoring Service: Enabled is checked. The change can be observed on the consoles Core page. Metrics start to not be for the recent 60s and eventually after 1 minute the "no data" state is reached. This was tested also for extra instances.
The change can also be observed by the info log messages when collection is started and stopped but this does not reflect if the job actually starts or stops. If the metrics disappear this means the job actually is no longer active.

To test if logging really logs failing sources only once a simple dummy class was added temporary that simulated a mostly failing source:

@Service
class FlakyDataSource implements MonitoringDataSource {
  int x = 0;
  public void collect(MonitoringDataCollector collector) {
    if (++x % 10 != 0) {
      throw new RuntimeException();
    }
  }
}

(given in case reviewer wants to redo this; can be added to any module where HK2 @Service will be picked up). The failing source can be seen as metric: Monitoring Console Internals => Failed Collection Count. To verify that the log is only written once after collection was successful enable FINE log level for the LOGGER "monitoring-console-core" or put a breakpoint at the log statement.

Note for the Reviewer

Considering the changes done by #4303 it might be more logical to connect the data collection to a new separate configuration field that reflects the deployed state of the monitoring application so that collection is enabled/disabled in union with the application being deployed/undeployed. However, we might even want to split this into two (new) flags because it can make sense to collect data without having the application deployed to have a history available in case the application is turned on.
This can be discussed and potentially combined accordingly after this PR and #4303 are merged.

…when monitoring is enabled
@jbee jbee self-assigned this Nov 6, 2019
@jbee jbee requested review from Cousjava and Pandrex247 Nov 6, 2019
@jbee jbee added this to the 5.194 milestone Nov 6, 2019
@jbee

This comment has been minimized.

Copy link
Contributor Author

jbee commented Nov 6, 2019

jenkins test please

@Cousjava

This comment has been minimized.

Copy link
Member

Cousjava commented Nov 7, 2019

How about not starting the worker thread unless monitoring is enabled?

@jbee

This comment has been minimized.

Copy link
Contributor Author

jbee commented Nov 8, 2019

jenkins test please

@jbee

This comment has been minimized.

Copy link
Contributor Author

jbee commented Nov 8, 2019

@Cousjava I now listen to config change events to start and stop the job that does the collection. (See also updated description)

@MarkWareham

This comment has been minimized.

Copy link
Contributor

MarkWareham commented Nov 8, 2019

Jenkins test please

@jbee jbee merged commit bbee202 into payara:master Nov 11, 2019
58 checks passed
58 checks passed
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.