search

newrelic / newrelic-telemetry-sdk-java

Java library for sending telemetry data to New Relic
Apache License 2.0
41 stars 37 forks source link

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

Open nmala01 opened 2 years ago

nmala01 commented 2 years ago

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 commented 2 years ago

@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?

meiao commented 2 years ago

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:

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

nmala01 commented 2 years ago

@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.

workato-integration[bot] commented 2 years ago

https://issues.newrelic.com/browse/NEWRELIC-4094

kford-newrelic commented 1 year ago

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