Emit integration invoke error metric for destination load failures#1341
Emit integration invoke error metric for destination load failures#1341MichaelGHSeg merged 3 commits intomasterfrom
Conversation
…d failures Classic destination script loading (loadIntegration) and construction (buildIntegration) failures were not emitting any metrics, causing silent failures invisible to Datadog. Similarly, action destination failures in the remote-loader only logged console.warn with no metrics. - Wrap loadIntegration/buildIntegration in LegacyDestination.load() with try/catch that calls recordIntegrationMetric (method:load) - Add recordIntegrationMetric call in remote-loader catch block - Replace silent .catch(() => []) on remoteLoader with console.error - Add tests for both classic and action destination load error metrics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 59bbf3e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1341 +/- ##
==========================================
+ Coverage 91.19% 91.20% +0.01%
==========================================
Files 163 163
Lines 4383 4388 +5
Branches 1052 1052
==========================================
+ Hits 3997 4002 +5
Misses 386 386
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves observability for destination load failures by emitting analytics_js.integration.invoke.error metrics for both classic (AJS) destinations and action (remote) destinations, and by surfacing previously swallowed remote-loader errors.
Changes:
- Emit integration invoke error metrics when classic destinations fail during
loadIntegration/buildIntegration. - Emit integration invoke error metrics when remote action plugin factories fail during remote loading.
- Replace a silent
remoteLoader()catch with explicit error logging; add unit tests covering these error-metric emissions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/browser/src/plugins/remote-loader/index.ts | Records an integration invoke error metric when a remote plugin fails to load. |
| packages/browser/src/plugins/remote-loader/tests/index.test.ts | Adds coverage ensuring the action destination load failure metric is emitted. |
| packages/browser/src/plugins/ajs-destination/index.ts | Wraps classic destination load/build to emit an integration invoke error metric on failure. |
| packages/browser/src/plugins/ajs-destination/tests/index.test.ts | Adds coverage ensuring classic destination load/build failures emit error metrics. |
| packages/browser/src/browser/index.ts | Logs remote loader failures instead of silently swallowing them. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| console.warn('Failed to load Remote Plugin', error) | ||
| recordIntegrationMetric(Context.system(), { | ||
| integrationName: remotePlugin.creationName, | ||
| methodName: 'load', | ||
| type: 'action', | ||
| didError: true, | ||
| }) |
There was a problem hiding this comment.
The new error metric uses integrationName: remotePlugin.creationName, but action destination metrics elsewhere in this module use integrationName: this.action.name (i.e., the plugin's name). This means load failures will be tagged under a different integration_name than successful load/invoke metrics for the same destination, making dashboards and error-rate calculations inconsistent. Consider using remotePlugin.name here for consistency, or update the ActionDestination metric calls to use the wrapper/destination name consistently (e.g., this.name / remotePlugin.creationName).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
chenxzhang
left a comment
There was a problem hiding this comment.
lgtm, let's just make sure the integrationName is consistent
|
@MichaelGHSeg I've opened a new pull request, #1345, to work on those changes. Once the pull request is ready, I'll request review from you. |
… metrics (#1345) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MichaelGHSeg <101233650+MichaelGHSeg@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
loadIntegration) and construction (buildIntegration) failures were not emitting anyanalytics_js.integration.invoke.errormetrics — errors went straight tohandleLoadErrorin event-queue.ts which only doesconsole.warnand removes the pluginconsole.warnwith no metricsremoteLoader()call inbrowser/index.tshad a completely silent.catch(() => [])that swallowed all errors with no loggingThese gaps were identified during investigation of a GTM incident where a new "Google Custom Domain" field with an empty value silently broke all GTM destination instances with zero visibility in Datadog.
Changes
loadIntegration/buildIntegrationinLegacyDestination.load()with try/catch that callsrecordIntegrationMetricwithmethod:load, type:classic, didError:truerecordIntegrationMetriccall in remote-loader catch block withmethod:load, type:action, didError:true.catch(() => [])onremoteLoadercall withconsole.errorloggingTest plan
recordIntegrationMetriccalled withmethod:load, didError:truewhenloadIntegrationthrows (classic)recordIntegrationMetriccalled withmethod:load, didError:truewhenbuildIntegrationthrows (classic)recordIntegrationMetriccalled withmethod:load, didError:truewhen remote plugin factory throws (action)🤖 Generated with Claude Code