Repository navigation
Conversation
|
✅ All CI checks and tests passed. Datadog automation helped this PR pass. 🎉 All green!🧪 All tests passed 🔄 Datadog retried 1 test - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: 00c79d9 | Docs | View more details | Give us feedback! |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
8b378d6 to
e4488d8
Compare
There was a problem hiding this comment.
Normal RUM event processing erases the new integration-local host flag, causing subsequent configuration telemetry to report first-party hosts as disabled when no global hosts are configured.
🤖 Bits Code Review · Commit 8b378d6
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b378d6c21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
46da062 to
9075614
Compare
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9075614bbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
376eb8e to
c6e4be7
Compare
c6e4be7 to
00c79d9
Compare
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00c79d9363
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return emptyList() | ||
| } | ||
| instances[key] = sdkCore | ||
| return referencesFor(key).mapNotNull { it.capture(sdkCore) } |
There was a problem hiding this comment.
Defer callbacks that require enabled features
When instrumentation has called get() before Datadog.initialize, this captures it during core registration, before the app can subsequently call Rum.enable or Trace.enable. Consequently, reportNetworkInstrumentationConfigured calls GlobalRumMonitor.get while it still returns a no-op (logging a misleading warning and permanently dropping the networking-library API-usage event), while the legacy interceptor's updateTracingFeatureContext silently returns because the Trace feature is absent. Since the reference remains captured, later requests do not rerun either callback. Split the per-core configuration capture from feature-dependent reporting and replay the latter after the corresponding feature is registered.
AGENTS.md reference: AGENTS.md:L115-L115
Useful? React with 👍 / 👎.
| return sdkCore | ||
| sdkCore to registry.registerAndGetNotifications(sdkInstanceName, sdkCore) | ||
| } | ||
| registry.notifyReferences(notifications) |
There was a problem hiding this comment.
Keep capture callbacks off the initialization thread
When an SdkReference starts observing before initialization, this invokes its public onSdkInstanceCaptured callback synchronously before Datadog.initialize returns. A customer callback that waits, performs I/O, or otherwise runs slowly now stalls SDK initialization—normally on the application's main thread—even though previously it ran only from the caller's explicit get(). Dispatch these callbacks through an existing executor, or otherwise keep arbitrary callback work out of the initialization path.
AGENTS.md reference: AGENTS.md:L113-L113
Useful? React with 👍 / 👎.
| is InternalTelemetryEvent.NetworkInstrumentationConfigured -> | ||
| event.resourceHeadersTracking ?: current.resourceHeadersTracking | ||
| is InternalTelemetryEvent.ResourceHeadersTrackingConfigured -> event.mode | ||
| else -> current.resourceHeadersTracking |
There was a problem hiding this comment.
Merge header modes independently of callback order
When multiple integrations are created before SDK initialization with different header extractors, such as one using default headers and another using custom headers, their capture callbacks are obtained from a WeakHashMap in unspecified order. Each update here unconditionally replaces the previous non-null mode, so the one-shot configuration event can nondeterministically report either default_headers or custom for the same setup. Define deterministic aggregation, for example making CUSTOM dominate, rather than allowing the final callback to win.
Useful? React with 👍 / 👎.
| _InternalProxy.updateNetworkInstrumentationConfiguration( | ||
| sdkCore, | ||
| InternalTelemetryEvent.NetworkInstrumentationConfigured(resourceHeadersExtractor?.toTrackingMode()) | ||
| ) |
There was a problem hiding this comment.
Preserve updates when the core proxy declines them
When instrumentation is bound to an InternalSdkCore implementation other than the concrete DatadogCore, this proxy call returns false by contract, but the result is ignored and the NetworkInstrumentationConfigured event is never delivered to TelemetryEventHandler. Its fallback configuration therefore remains empty, causing track_network_requests and track_resource_headers to be reported as disabled for wrapper or reliability cores. Forward the event through the monitor when the proxy declines it, or otherwise retain it in the caller as the proxy contract requires.
Useful? React with 👍 / 👎.
What does this PR do?
use_first_party_hostsfrom global hosts and per-SDK integration configuration.Motivation
Android's network-tracking and resource-header percentages were much lower than iOS in the RUM configuration telemetry dashboard. The underlying telemetry can undercount enabled features: setup signals were sampled or capped before updating state, and the legacy interceptor could report headers before its constructor fields were initialized. First-party-host usage was also not populated.
Known limitation: this PR keeps the existing one-shot configuration snapshot, scheduled five seconds after SDK initialization. OkHttp clients configured after the snapshot is built are still not reflected in that event. Reporting late network setup is intentionally left for a follow-up.
Review checklist (to be filled by reviewers)