-
Notifications
You must be signed in to change notification settings - Fork 136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RUM-8558 Update DatadogTrace to OTel API 1.13.0
#2217
Conversation
which were proven flaky (#1881) with OTel API 1.6.0
Datadog ReportBranch report: ✅ 0 Failed, 321 Passed, 3499 Skipped, 53.01s Total duration (1m 42.98s time saved) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work overall in this update to OTel 1.13.0
🎖️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
// If the `OTEL_SWIFT` environment variable is set, `dd-sdk-ios` will be compiled against `OpenTelemetryApi` | ||
// from https://github.com/open-telemetry/opentelemetry-swift, which includes the full OpenTelemetry SDK. | ||
// Otherwise, it will use our lightweight mirror from https://github.com/DataDog/opentelemetry-swift-packages. | ||
// | ||
// This split is driven by feedback from https://github.com/DataDog/dd-sdk-ios/issues/1877, where | ||
// users reported that fetching the full OpenTelemetry SDK significantly increased dependency size. | ||
// | ||
// By using this environment variable, `dd-sdk-ios` consumers can choose whether to depend on the entire | ||
// OpenTelemetry SDK or just the API. This remains necessary until OpenTelemetry officially separates | ||
// the API and SDK packages (see https://github.com/open-telemetry/opentelemetry-swift/issues/486). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks!
What and why?
📦 This PR updates
DatadogTrace
to useOpenTelemetryApi
1.13.0.This addresses #2190.
How?
The new
OpenTelemetryApi
version was published in opentelemetry-swift-packages#27, and this PR integrates it intodd-sdk-ios
.Platform Versions Alignment
Since
1.6.0
(our previousOpenTelemetryApi
dependency version),open-telemetry-swift
introduced a breaking change in supported platforms, raising the minimum deployment targets from.iOS(11)
and.tvOS(11)
to.iOS(13)
and.tvOS(13)
. Because our SDK supports.iOS(12)
and.tvOS(12)
, this required adding a patch inPackage.swift
to conditionally adjustplatforms
when theOTEL_SWIFT
environment variable is set. I used this as an opportunity to add comments and better document this flag. In our repository, this is leveraged inBenchmarkTests
.New
OTelSpanBuilder
APIsThis update introduces the following
OTelSpanBuilder
APIs. Their implementation in this PR follows otel-swift, delegating active span management toOpenTelemetry.instance.contextProvider
:This PR adds test coverage for these new APIs and also re-enables
OTelSpanTests/testSetActive_givenParentSpan()
, which was disabled in #1881 due to flaky performance. The updatedOpenTelemetryApi
refines active span management, so performance should now be stable.New
OTelSpan
APIsThe update also introduces new
OTelSpan
APIs:These APIs are not implemented in this PR because their proper implementation depends on the
addEvent()
API, which is not yet supported in our OTel integration.Deprecated OTel APIs
The latest
OpenTelemetryApi
deprecates the following attribute array creation methods:These have been replaced with a more uniform
.array(.init(values: []))
API. This PR updates our test suite to use the new.array()
format and removes dependencies on deprecated APIs.Review checklist
make api-surface
)