search

DataDog / dd-sdk-ios

Datadog SDK for iOS - Swift and Objective-C.
Apache License 2.0
188 stars 121 forks source link

Request: Split out `OpenTelemetry` support into a separate Package target #1877

Open JaviSoto opened 3 weeks ago

JaviSoto commented 3 weeks ago

Feature description

v2.12.0 introduced support for OpenTelemetry. As a result, DatadogTracer now depends on opentelemetry-swift. This has a ton of transitive dependencies. Since my understanding is that the use of OpenTelemetry is optional, this introduces a lot of extra binary size and complexity to projects that depend on DatadogTracer which aren't neccesary. I'm particularly concerned about making our team's experience with Xcode and the buggy SPM support even worse by adding all these extra packages:

Proposed solution

Hopefully creating some kind of DatadogTracerOpenTelemetry target can alleviate this. Only users who need this would depend on it, and those who don't can avoid all these transitive dependencies.

Other relevant information

No response

ganeshnj commented 3 weeks ago

Thanks for contacting @JaviSoto

The introduction of OpenTelemetry support takes a dependency on OpenTelemetryApi target only which has 0 dependencies. Hence, the application only includes the OpenTelemetryApi target and not the transitive dependencies.

In my testing with a sample application, the binary size increased from 33.6MB to 37.5MB when adding the OpenTelemetry support which we believe is acceptable. (Used https://github.com/DataDog/dd-sdk-ios/tree/develop/dependency-manager-tests/spm for testing)

However, I do agree with the developer experience of having to deal with extra packages in Xcode which can totally be avoided. We share the same concerns and opened an issue in the OpenTelemetry-Swift repository to address this. Feel free to +1 it.

Regarding separating the OpenTelemetry support into a separate target, we discussed this at length and decided against it for the following reasons:

sebskuse commented 2 weeks ago

Hi,

Just found this thread after noticing that after updating to the latest version of the DataDog SDK we have sixteen new dependencies that have appeared.

In my testing with a sample application, the binary size increased from 33.6MB to 37.5MB when adding the OpenTelemetry support which we believe is acceptable. (Used https://github.com/DataDog/dd-sdk-ios/tree/develop/dependency-manager-tests/spm for testing)

Whilst I appreciate we're not compiling and shipping these extra libraries, there are other considerations than purely shipped binary size. We are still having to check out and cache those sixteen new repositories, which has significantly increased our build time locally and has doubled the size of our SPM cache (adding just under 1GB of checkout at the time of writing).

For us, the penalty of upgrading to 2.12.0 means that we will have to pin our version to 2.11.0.

ganeshnj commented 2 weeks ago

Totally @sebskuse, feel free to +1 on the linked issue to split otel APIs.

However, I wouldn't recommend pinning the version and using the old version of the SDK for obvious reasons. We keep pushing fixes and improvements in the SDK and you would be missing out on those.

sebskuse commented 2 weeks ago

Thanks @ganeshnj, I have +1'd it. My worry here is that that issue has been open since Nov 20, 2023 without much traction, so is this realistically going to get actioned?

I understand about pinning. My issue is the majority are being punished by DD supporting this pretty niche feature. If this is the route DD is going down, we may need to re-evaluate our usage of it. I'd rather not have to download and cache the whole of SwiftNIO every time any of our dependencies changes.

Kaspik commented 2 weeks ago

However, I wouldn't recommend pinning the version and using the old version of the SDK for obvious reasons. We keep pushing fixes and improvements in the SDK and you would be missing out on those.

I have to say we are gonna do the same - pin to the 2.11.0 until this is resolved / reverted. We are not willing to checkout another dozen of SPMs for a feature we are not gonna use.

clausjoergensen commented 1 week ago

Same here. I'm not willing to take in dependencies on all of these Packages, of which a lot are missing the required PrivacyInfo.xcprivacy files, and/or are old and have questionable maintainable history.

ganeshnj commented 1 week ago

I want to keep people updated here that we are working with otel folks to resolve this problem. We expected this friction but all this feedback is quite valuable for us in order to make a case to do the right thing. I really appreciate it.

Kaspik commented 1 week ago

Appreciate the info here @ganeshnj , thanks!

Right now it's a weird spot for us - 2.11.0 doesn't compile on XCode 16 (it's fixed on master) and 2.12+ has OpenTelemetry dependencies. Will look forward for a solution here!

ganeshnj commented 1 day ago

A quick update here, we have https://github.com/open-telemetry/opentelemetry-specification/issues/4084 spec change request. I can't promise any timeline but we are working actively to make it happen.

@Kaspik would it be helpful if we release a patch 2.11.1 that has Xcode 16 fix to unblock you?

Kaspik commented 1 day ago

Absolutely! 2.11.1 would be very helpful, thank you!