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

No exception thrown when Refusing to schedule a batch in Limiting Scheduler, only warning is logged, causing data loss. #299

Open
nmala01 opened this issue Aug 11, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@nmala01
Copy link

nmala01 commented Aug 11, 2022

I am wring a code where I am sending data to new relic endpoint using TelemetryClient. But in case where batch to be scheduled cannot be allocated, the scheduler just logs the warning and proceeds. Ideal case would be to throw exception in these scenarios, so that client can handle it properly.

WARN Refusing to schedule batch of size 500 (would put us over max size 1000000, available = 96) (com.newrelic.telemetry.LimitingScheduler:37)
WARN DATA IS BEING LOST! (com.newrelic.telemetry.LimitingScheduler:45)

(Migrate to Jira)

@kford-newrelic
Copy link

@nmala01 thanks for bringing this to our attention. We're not able to commit to changing this at the moment but it's a good suggestion

Since your code is fresh in your mind, would you be willing to provide a sample app that reproduces the problem?

@kford-newrelic kford-newrelic added the enhancement New feature or request label Aug 16, 2022
@meiao
Copy link
Contributor

meiao commented Aug 16, 2022

Imagine if a logging framework threw an exception and ended up rolling back a transaction or ending a procedure early. It would not look good, and try/catch every log call is not feasible.

The point here is that both logging and New Relic are 3rd parties that should not prevent the main application from functioning.

That does not mean it is not a good idea to have a method that can allows to treatment of an exception.

So ideally there should be 2 methods:

  • one that only logs;
  • one that throws checked exceptions.

Implementation wise, the logging method should call the exception method, catch the exception, log it and continue the flow.

@nmala01
Copy link
Author

nmala01 commented Aug 17, 2022

@meiao I understand that 3rd parties should not prevent main application from functioning, but data loss is such a crucial error that just logging it seems like a bug.
Also the option for logging or throwing exception can be handled using the NotificationHandler if appropriate exception get thrown to TelemetryClient.

@kford-newrelic kford-newrelic added this to Triage in Java Engineering Board via automation Sep 8, 2022
@workato-integration
Copy link

@kford-newrelic kford-newrelic removed this from Triage in Java Engineering Board Sep 30, 2022
@kford-newrelic
Copy link

Unfortunately, we were not able to prioritize this for the next (Jan-Mar) quarter. Will consider for a future quarter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants