RUMM-2334 Tracing with RUM Through Core Context #989

maxep · 2022-09-06T11:17:12Z

What and why?

Tracing gets RUM context from the message bus.

How?

We can now add DatadogContextObserver to the DatadogContextProvider to get notified when the context changes. The DatadogCore is an observer and send a context message on the bus on any context change.

Note: The context still have passive values that are read when the context is requested. To get the most up-to-date values we notify the observers on each read operation as well. This was already the case in V1 providers, but this needs to be addressed so any value change generate a push on the message-bus.

Review checklist

Feature or bugfix MUST have appropriate tests (unit, integration)
Make sure each commit and the PR mention the Issue number or JIRA reference
Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

Run unit tests
Run integration tests
Run smoke tests

datadog-datadog-prod-us1 · 2022-09-06T11:30:45Z

Datadog Report

Branch report: maxep/RUMM-2334/tracing-with-rum-context
Commit report: f46ee48

❌ dd-sdk-ios 1 Failed, 0 New Flaky, 1952 Passed, 0 Skipped, 19m26.99s Wall Time

Failed Tests (1)

testLoggingScenario - LoggingScenarioTests - Details

Expand for error

 
 Assertion Failure at LoggingScenarioTests.swift:71: failed - Can&#39;t cast value at key path `network.client.reachability` to expected type: nil
 Assertion Failure at LoggingScenarioTests.swift:77: failed - Can&#39;t cast value at key path `network.client.available_interfaces` to expected type: nil
 Assertion Failure at LoggingScenarioTests.swift:84: XCTAssertTrue failed
 Assertion Failure at LoggingScenarioTests.swift:85: XCTAssertTrue failed
 Assertion Failure at LoggingScenarioTests.swift:86: XCTAssertTrue failed
 Assertion Failure at LoggingScenarioTests.swift:71: failed - Can&#39;t cast value at key path `network.client.reachability` to expected type: nil
 Assertion Failure at LoggingScenarioTests.swift:77: failed - Can&#39;t cast value at key path `network.client.available_interfaces` to expected type: nil
 Assertion Failure at LoggingScenarioTests.swift:84: XCTAssertTrue failed
 Assertion Failure at LoggingScenarioTests.swift:85: XCTAssertTrue failed
 ...

datadog-datadog-prod-us1 · 2022-09-13T11:31:00Z

Datadog Report

Branch report: maxep/RUMM-2334/tracing-with-rum-context
Commit report: 512ec08

❄️ dd-sdk-ios 0 Failed, 1 New Flaky, 1947 Passed, 0 Skipped, 20m49.55s Wall Time

New Flaky Tests (1)

testCrashReportingCollectOrSendWithRUMScenario - CrashReportingWithRUMScenarioTests - Last Failure

Expand for error

 Thrown Error at ServerSession.swift:50: failed: caught error: "Exceeded 30.0s timeout with pulling 0 requests and not meeting the \`condition()\`."

datadog-datadog-prod-us1 · 2022-10-06T13:15:28Z

Datadog Report

Branch report: maxep/RUMM-2334/tracing-with-rum-context
Commit report: 19d2354

✅ dd-sdk-ios 0 Failed, 0 New Flaky, 1949 Passed, 0 Skipped, 19m31.6s Wall Time

ncreated

In overall it looks good 👍. I left few feedbacks on remaining leftovers and missing tests coverage for context provider.

Sources/Datadog/DatadogCore/Context/DatadogContextProvider.swift

Sources/Datadog/DatadogInternal/MessageBus/FeatureMessage.swift

Sources/Datadog/Tracing/TracingV2Configuration.swift

Tests/DatadogTests/Datadog/TracerTests.swift

ncreated · 2022-10-14T07:29:40Z

Sources/Datadog/Tracing/Integrations/TracingWithRUMIntegration.swift

+    /// Fair and recursive lock.
+    private let lock = NSRecursiveLock()
+
+    /// Unsafe attributes.
+    private var _attribues: [String: Encodable]?
+
+    private func synchronize<T>(_ block: () -> T) -> T {
+        lock.lock()
+        defer { lock.unlock() }
+        return block()
+    }


Do we know how NSLock perf compares to DispatchQueue? Do we have plans on wider use of locks vs queues for other products in similar use case (decorating one Feature events with other Feature context)?

Here we will be running this critical section for each tracer.startSpan() (on user thread), so overhead is important. If we follow this pattern, similar thing will happen in other Features.

Yes, here a benchmark: https://www.vadimbulavin.com/benchmarking-locking-apis/

For read operation: DispatchQueue is 7-8 times slower than locks.

For write operation: DispatchQueue is 3-4 times slower than locks

DispatchQueue performs better when using concurrent queue tho. But prior to this change, the Tracer was doing a sync on the (serial) queue of the RUMMonitor from the user thread (here).

Do we have plans on wider use of locks vs queues for other products in similar use case (decorating one Feature events with other Feature context)?

No, it's on a case by case. Here I'm using a lock to avoid syncing the Tracer\queue from the user-thread. But it really depends on the Feature pipeline.

Sources/Datadog/DatadogCore/Context/DatadogContextProvider.swift

datadog-datadog-prod-us1 · 2022-10-14T10:29:59Z

Datadog Report

Branch report: maxep/RUMM-2334/tracing-with-rum-context
Commit report: e7cbfcb

❌ dd-sdk-ios 1 Failed, 0 New Flaky, 1951 Passed, 0 Skipped, 17m55.73s Wall Time

Failed Tests (1)

testStartWithPendingConsent_thenPlayScenario_thenChangeConsentToNotGranted - TrackingConsentScenarioTests - Details
Expand for error
```
 Segmentation fault. Check error.crash_log for the full crash log.
```

ncreated

🚀

maxep self-assigned this Sep 6, 2022

maxep force-pushed the maxep/RUMM-2334/logging-with-rum-context branch from af33959 to aeb3029 Compare September 6, 2022 15:13

maxep force-pushed the maxep/RUMM-2334/tracing-with-rum-context branch from 1f25087 to 75ca11b Compare September 6, 2022 15:13

maxep force-pushed the maxep/RUMM-2334/logging-with-rum-context branch 3 times, most recently from 7ad368a to b5d1217 Compare September 13, 2022 11:05

maxep force-pushed the maxep/RUMM-2334/tracing-with-rum-context branch from f46ee48 to 9a65ff1 Compare September 13, 2022 11:23

maxep force-pushed the maxep/RUMM-2334/tracing-with-rum-context branch from 9a65ff1 to 512ec08 Compare September 14, 2022 08:18

maxep added 2 commits October 6, 2022 14:51

RUMM-2334 Tracing with RUM Context

cc6e481

RUMM-2334 Add Datadog Context Observer

b682c4f

maxep force-pushed the maxep/RUMM-2334/tracing-with-rum-context branch from 512ec08 to a1f6147 Compare October 6, 2022 12:55

maxep added 2 commits October 6, 2022 15:00

RUMM-2334 Test Tracing Message Receiver

863e77b

RUMM-2334 Remove RUMIntegrations

13636b4

maxep force-pushed the maxep/RUMM-2334/tracing-with-rum-context branch from a1f6147 to 13636b4 Compare October 6, 2022 13:00

maxep changed the base branch from maxep/RUMM-2334/logging-with-rum-context to feature/v2 October 7, 2022 12:30

RUMM-2334 Use RUM attributes keys in tests

5ae5a25

maxep marked this pull request as ready for review October 7, 2022 14:21

maxep requested a review from a team as a code owner October 7, 2022 14:21

RUMM-2334 reuse existing publisher pattern

19d2354

maxep force-pushed the maxep/RUMM-2334/tracing-with-rum-context branch from fe152c2 to 19d2354 Compare October 10, 2022 13:14

ncreated requested changes Oct 14, 2022

View reviewed changes

maxep added 2 commits October 14, 2022 11:50

RUUM-2334 Fix ref doc

b4c46e8

RUMM-2334 Update tests

e7cbfcb

maxep force-pushed the maxep/RUMM-2334/tracing-with-rum-context branch from 9d174fb to e7cbfcb Compare October 14, 2022 10:07

ncreated approved these changes Oct 14, 2022

View reviewed changes

maxep merged commit 0133fbf into feature/v2 Oct 14, 2022

maxep deleted the maxep/RUMM-2334/tracing-with-rum-context branch October 14, 2022 13:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RUMM-2334 Tracing with RUM Through Core Context #989

RUMM-2334 Tracing with RUM Through Core Context #989

maxep commented Sep 6, 2022 •

edited

Loading

datadog-datadog-prod-us1 bot commented Sep 6, 2022 •

edited

Loading

datadog-datadog-prod-us1 bot commented Sep 13, 2022 •

edited

Loading

datadog-datadog-prod-us1 bot commented Oct 6, 2022 •

edited

Loading

ncreated left a comment

ncreated Oct 14, 2022

maxep Oct 14, 2022

datadog-datadog-prod-us1 bot commented Oct 14, 2022 •

edited

Loading

ncreated left a comment

RUMM-2334 Tracing with RUM Through Core Context #989

RUMM-2334 Tracing with RUM Through Core Context #989

Conversation

maxep commented Sep 6, 2022 • edited Loading

What and why?

How?

Review checklist

Custom CI job configuration (optional)

datadog-datadog-prod-us1 bot commented Sep 6, 2022 • edited Loading

Datadog Report

Failed Tests (1)

datadog-datadog-prod-us1 bot commented Sep 13, 2022 • edited Loading

Datadog Report

New Flaky Tests (1)

datadog-datadog-prod-us1 bot commented Oct 6, 2022 • edited Loading

Datadog Report

ncreated left a comment

Choose a reason for hiding this comment

ncreated Oct 14, 2022

Choose a reason for hiding this comment

maxep Oct 14, 2022

Choose a reason for hiding this comment

datadog-datadog-prod-us1 bot commented Oct 14, 2022 • edited Loading

Datadog Report

Failed Tests (1)

ncreated left a comment

Choose a reason for hiding this comment

maxep commented Sep 6, 2022 •

edited

Loading

datadog-datadog-prod-us1 bot commented Sep 6, 2022 •

edited

Loading

datadog-datadog-prod-us1 bot commented Sep 13, 2022 •

edited

Loading

datadog-datadog-prod-us1 bot commented Oct 6, 2022 •

edited

Loading

datadog-datadog-prod-us1 bot commented Oct 14, 2022 •

edited

Loading