[Innovation week] Enable compilation for macOS #1711

maciejburda · 2024-03-07T10:52:54Z

What and why?

These changes enable macOS support for Core, Logs, Trace, Crash Reporting and Web View Tracking as well as add some minor tweaks for VisionOS support.

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 for Core, RUM, Trace, Logs, CR and WVT
Run unit tests for Session Replay
Run integration tests
Run smoke tests
Run tests for tools/

datadog-datadog-prod-us1 · 2024-04-02T12:42:01Z

Datadog Report

Branch report: maciey/innovation-week-changes
Commit report: 5d3252b
Test service: dd-sdk-ios

✅ 0 Failed, 2954 Passed, 0 Skipped, 30m 37.02s Wall Time
🔻 Test Sessions change in coverage: 12 decreased, 2 increased, 1 no change

🔻 Code Coverage Decreases vs Default Branch (12)

This report shows up to 5 code coverage decreases.

test DatadogInternalTests iOS 79.08% (-0.67%) - Details
test DatadogInternalTests tvOS 79.09% (-0.63%) - Details
test DatadogTraceTests tvOS 49.23% (-0.51%) - Details
test DatadogRUMTests iOS 79.22% (-0.49%) - Details
test DatadogLogsTests iOS 45.48% (-0.46%) - Details

ncreated

With adding so many #if <os> directives we need at least smoke tests that will verify build compatibility for macOS.

DatadogCore/Private/ObjcAppLaunchHandler.m

datadog-datadog-prod-us1 · 2024-04-17T12:57:12Z

Datadog Report

Branch report: maciey/innovation-week-changes
Commit report: 8973121
Test service: dd-sdk-ios

✅ 0 Failed, 3053 Passed, 0 Skipped, 30m 53.18s Wall Time
🔻 Test Sessions change in coverage: 3 decreased, 12 increased

🔻 Code Coverage Decreases vs Default Branch (3)

test DatadogRUMTests tvOS 80.85% (-0.18%) - Details
test DatadogCoreTests tvOS 79.56% (-0.06%) - Details
test DatadogCoreTests iOS 73.48% (-0.01%) - Details

ncreated

I think we can limit the impact of this PR if targeting a higher macOS version (more might come with official support). I left a question on this.
Also, let's change the title of this PR to be more descriptive. Otherwise future selves won't find it.

💪 Other than this is it looks fine - great work on adding a smoke test 🚬 🏅 🚭.

DatadogCore/Sources/Core/Context/LowPowerModePublisher.swift

Package.swift

maxep

Looks great, very nice effort! I think we should update the core context provider enablement, especially the LowPowerModePublisher which could be enabled for macOS 12 and above.

DatadogCore/Sources/Core/Context/LowPowerModePublisher.swift

DatadogInternal/Sources/Context/DeviceInfo.swift

DatadogRUM/Sources/RUMVitals/VitalRefreshRateReader.swift

dependency-manager-tests/spm/App macOS/ContentView.swift

maciejburda · 2024-04-19T10:18:57Z

Updated the PR. Supporting macOS 12+ seems quite painless comparing to 10.15 and 11.

PTAL 🙌

ganeshnj

Compiling for macOS 11, but module 'DatadogCore' has a minimum deployment target of macOS 12.0

I tried to compile the SPM dependency manager test app with Xcode 15 and couldn't build. There seems some version mismatch.

ganeshnj · 2024-04-19T09:39:53Z

DatadogCore/Sources/Core/Context/NetworkConnectionInfoPublisher.swift

@@ -20,7 +20,7 @@ import Network
 /// We found the pulling model to not be thread-safe: accessing `currentPath` properties lead to occasional crashes.
 /// The `ThreadSafeNWPathMonitor` listens to path updates and synchonizes the values on `.current` property.
 /// This adds the necessary thread-safety and keeps the convenience of pulling.
-@available(iOS 12, tvOS 12, *)
+@available(iOS 12, tvOS 12, macOS 10.15, *)


❓
why do we have different macOS support versions here as 10.15 but 10.14 somewhere down?

Removed it. We agreed to support 12+

ganeshnj · 2024-04-19T09:41:37Z

DatadogCore/Sources/Core/Context/NetworkConnectionInfoPublisher.swift

        guard let flags = flags, flags.contains(.isWWAN) else {
            return nil
        }
        self = .cellular
+        #else
+        self = .other


There are no mac with cellular connection, this must nil for macOS.

ganeshnj · 2024-04-19T09:45:04Z

DatadogInternal/Sources/Context/DeviceInfo.swift

+        let build = (try? Sysctl.osVersion()) ?? ""
+        let model = (try? Sysctl.model()) ?? ""


can they just be nil?

Also, are there high level APIs for such information?

They're required by schema

I see, instead empty string some other string would be helpful in querying like <unknown> or <unrecognized>

ganeshnj · 2024-04-19T10:44:53Z

SUPPORTED-VERSIONS.md

@@ -5,13 +5,18 @@
 | **iOS**    |     ✅    |  `11+` |
 | **tvOS**   |     ✅    |  `11+` |
 | **iPadOS** |     ✅    |  `11+` |
-| **VisionOS** |   ⚠️    |  `1.1+` |
+| **visionOS** |   ⚠️    |  `1.0+` |
+| **macOS**  |     ⚠️    |  `10.15+` |


What kind of support we are offering here, I see

Mac (I think this is most interesting)

Mac Catalyst

Mac Designed for iPad

I made some adjustment based on previous comment. Now we only support 12+

Now target destination is a pandora box.

I'd like to discuss during the SDK meeting.
I think we can simplify and have only one target with all the destinations, instead of current iOS/tvOS split

ganeshnj · 2024-04-19T10:46:06Z

SUPPORTED-VERSIONS.md

+
+## MacOS
+
+MacOS is not officially supported by Datadog SDK. Some features may not be fully functional. Note that `RUM`, which heavily depends on `UIKit` does not build on macOS.


If we know this is not going to build, we must not add RUM support in project. This will cause unnecessary friction.

We can only have one Package definition and as far as I know, and we can't do selective destination (Session Replay is iOS only for example)

ganeshnj · 2024-04-19T10:48:10Z

bitrise.yml

@@ -360,6 +360,18 @@ workflows:
                -project "$BITRISE_SOURCE_DIR/dependency-manager-tests/spm/SPMProject.xcodeproj" \
                -destination "platform=macOS,variant=Mac Catalyst" \
                | xcpretty
+    - script:
+        title: Check macOS compatibility (build SPMProject for macOS)


❓
Only SPM?

It follows what we did for Mac Catalyst. It's minimal effort for the smoke test, as the support is not official

ganeshnj · 2024-04-22T11:56:17Z

I tried the latest version 7b094e8 but still couldn't get the sample SPM app logs and tracing working yet. I think this is minimum bar that the project not only compiles but have some basic things working.

dependency-manager-tests/spm/Shared/DatadogSetup.swift

ncreated

Looks good 👍. I changed the PR title to be more descriptive. Great initiative, thanks 🏅

Co-authored-by: Ganesh Jangir <ganesh.jangir@datadoghq.com>

maxep

Thanks!

maciejburda force-pushed the maciey/innovation-week-changes branch from 8371e78 to 9b0151a Compare March 22, 2024 13:04

maciejburda marked this pull request as ready for review April 2, 2024 11:48

maciejburda requested review from a team as code owners April 2, 2024 11:48

maciejburda force-pushed the maciey/innovation-week-changes branch 2 times, most recently from 5c55eff to 5d959ad Compare April 2, 2024 12:33

MaximBazarov mentioned this pull request Apr 3, 2024

macOS support #78

Closed

ncreated reviewed Apr 4, 2024

View reviewed changes

DatadogCore/Private/ObjcAppLaunchHandler.m Outdated Show resolved Hide resolved

maciejburda added 7 commits April 17, 2024 13:23

Correct FPS for VisionOS

c478274

Changes to support macOS logs

bd29f61

Clean up

f13329c

Fix compilation issue on ios

26385f1

Run linter

d4d5c99

PR fixes

f66af65

Fix test

7562259

maciejburda force-pushed the maciey/innovation-week-changes branch from 5d3252b to d160960 Compare April 17, 2024 12:46

maciejburda added 3 commits April 18, 2024 11:01

PR Fixes

b5a4a51

Add smoke tests for macOS

687903b

Remove failing test

090d4a8

maciejburda force-pushed the maciey/innovation-week-changes branch from 58ac3ec to 090d4a8 Compare April 18, 2024 11:43

maciejburda requested a review from ncreated April 18, 2024 11:44

ncreated requested changes Apr 19, 2024

View reviewed changes

DatadogCore/Sources/Core/Context/LowPowerModePublisher.swift Outdated Show resolved Hide resolved

Package.swift Outdated Show resolved Hide resolved

maxep requested changes Apr 19, 2024

View reviewed changes

PR fixes

6ae18fd

maciejburda requested review from maxep and ncreated April 19, 2024 10:19

Documentation update

ec76086

ganeshnj requested changes Apr 19, 2024

View reviewed changes

maciejburda added 2 commits April 19, 2024 14:51

Fix deployment target on SPM project

afa261c

PR fixes

7b094e8

ganeshnj reviewed Apr 22, 2024

View reviewed changes

dependency-manager-tests/spm/Shared/DatadogSetup.swift Outdated Show resolved Hide resolved

Revert nil interface

39d5755

ganeshnj previously approved these changes Apr 23, 2024

View reviewed changes

ncreated changed the title ~~Innovation week changes~~ [Innovation week] Enable compilation for macOS Apr 23, 2024

ncreated previously approved these changes Apr 23, 2024

View reviewed changes

Update dependency-manager-tests/spm/Shared/DatadogSetup.swift

c08e66b

Co-authored-by: Ganesh Jangir <ganesh.jangir@datadoghq.com>

maciejburda dismissed stale reviews from ncreated and ganeshnj via c08e66b April 23, 2024 18:17

maciejburda requested review from ganeshnj and ncreated April 23, 2024 18:17

ncreated approved these changes Apr 24, 2024

View reviewed changes

maxep approved these changes Apr 24, 2024

View reviewed changes

maciejburda merged commit 79943d3 into develop Apr 24, 2024
20 checks passed

maciejburda deleted the maciey/innovation-week-changes branch April 24, 2024 14:28

ganeshnj mentioned this pull request May 2, 2024

Release 2.11.0 #1808

Merged

8 tasks

maciejburda mentioned this pull request May 7, 2024

Dogfood recent changes #1814

Closed

8 tasks

This was referenced May 7, 2024

Dogfood 2.11.0 #1816

Closed

Dogfood 2.11.0 #1817

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Innovation week] Enable compilation for macOS #1711

[Innovation week] Enable compilation for macOS #1711

maciejburda commented Mar 7, 2024 •

edited

Loading

datadog-datadog-prod-us1 bot commented Apr 2, 2024 •

edited

Loading

ncreated left a comment

datadog-datadog-prod-us1 bot commented Apr 17, 2024 •

edited

Loading

ncreated left a comment •

edited

Loading

maxep left a comment

maciejburda commented Apr 19, 2024

ganeshnj left a comment

ganeshnj Apr 19, 2024

maciejburda Apr 19, 2024

ganeshnj Apr 19, 2024

ganeshnj Apr 19, 2024

maciejburda Apr 19, 2024

ganeshnj Apr 22, 2024

ganeshnj Apr 19, 2024

maciejburda Apr 19, 2024

ganeshnj Apr 19, 2024

maciejburda Apr 19, 2024

ganeshnj Apr 19, 2024

maciejburda Apr 19, 2024

ganeshnj commented Apr 22, 2024

ncreated left a comment

maxep left a comment

		let build = (try? Sysctl.osVersion()) ?? ""
		let model = (try? Sysctl.model()) ?? ""


		## MacOS

		MacOS is not officially supported by Datadog SDK. Some features may not be fully functional. Note that `RUM`, which heavily depends on `UIKit` does not build on macOS.

[Innovation week] Enable compilation for macOS #1711

[Innovation week] Enable compilation for macOS #1711

Conversation

maciejburda commented Mar 7, 2024 • edited Loading

What and why?

Review checklist

Custom CI job configuration (optional)

datadog-datadog-prod-us1 bot commented Apr 2, 2024 • edited Loading

Datadog Report

🔻 Code Coverage Decreases vs Default Branch (12)

ncreated left a comment

Choose a reason for hiding this comment

datadog-datadog-prod-us1 bot commented Apr 17, 2024 • edited Loading

Datadog Report

🔻 Code Coverage Decreases vs Default Branch (3)

ncreated left a comment • edited Loading

Choose a reason for hiding this comment

maxep left a comment

Choose a reason for hiding this comment

maciejburda commented Apr 19, 2024

ganeshnj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganeshnj commented Apr 22, 2024

ncreated left a comment

Choose a reason for hiding this comment

maxep left a comment

Choose a reason for hiding this comment

maciejburda commented Mar 7, 2024 •

edited

Loading

datadog-datadog-prod-us1 bot commented Apr 2, 2024 •

edited

Loading

datadog-datadog-prod-us1 bot commented Apr 17, 2024 •

edited

Loading

ncreated left a comment •

edited

Loading