Feature Request: Make logging to DDNoopRUMMonitor louder

[dfed]

Background

Today, accidentally logging user actions or errors to the DDNoopRUMMonitor does not alert developers of API misuse – developers are only warned when startView events are propagated to it.

When an engineer accidentally retains an instance of Global.rum prior to running Global.rum = RUMMonitor.initialize(), they capture a DDNoopRUMMonitor instance. If an engineer then logs a user action or error to this rum instance, no warnings appear in the console, and of course no logs appear in DataDog dashboards either.

The baseline request

Update DDNoopRUMMonitor to alert a developer that they are misusing the DataDog SDK when any event (and not just startView) is sent to the DDNoopRUMMonitor. I'd be happy to put up a PR to accomplish this if the proposal is well received by maintainers.

The stretch request

Given that console logs might get lost in apps that log a lot to the console, and that errors routed to a Global.rum instance are explicitly high-priority events, ideally DDNoopRUMMonitor's addError implementation would trigger an assertionFailure(...). Again, I'd be happy to put up a PR to accomplish this if the proposal is well received by maintainers.

[ncreated]

Hello @dfed 👋 and sorry for late response. What you propose totally makes sense. We should enhance DDNoopRUMMonitor to uniformly report all misuses, not only startView.

Update DDNoopRUMMonitor to alert a developer that they are misusing the DataDog SDK when any event (and not just startView) is sent to the DDNoopRUMMonitor. I'd be happy to put up a PR to accomplish this if the proposal is well received by maintainers.

Sounds good 👍, we'll be happy to receive such PR 🙂.

Given that console logs might get lost in apps that log a lot to the console, and that errors routed to a Global.rum instance are explicitly high-priority events, ideally DDNoopRUMMonitor's addError implementation would trigger an assertionFailure(...). Again, I'd be happy to put up a PR to accomplish this if the proposal is well received by maintainers.

I get the idea, especially that (by default) assertions are disabled in Release builds. However, as convention in all similar places we only report things as error log when Datadog.verbosityLevel is set. Given that we actively work on SDK 2.x, where most things will be centralised, I'd recommend to wait with this improvement - we will consider adding it for 2.x.

[dfed]

Sounds great! I've opened #1007 to cover the baseline request. I have no qualms if you would like to close this issue after #1007 lands, though I am hoping that 2.x gets this stretch request 🙂

[maxep]

Thank you again @dfed for your contribution. This will land in 2.x when we migrate the RUM monitor 👍