Skip to content
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

Created OpenTelemetryApi and OpenTelemetrySdk podspecs #591

Conversation

ArielDemarco
Copy link
Contributor

@ArielDemarco ArielDemarco commented Aug 16, 2024

Description

The idea of this PR is to create the podspecs to distribute via CocoaPods:

  • OpenTelemetryApi
  • OpenTelemetrySdk

Many of the decisions made in the podspecs were extracted from Package@swift-5.9.swift, for example:

  • Swift Version declared in podspecs is 5.9
  • Minimum iOS Deployment Target is v13.0
  • Minimum tvOS Deployment Target is v13.0
  • Minimum watchOS Deployment Target is v6.0

Important

Versioning

Both podspecs should have the same version to work properly. Since OpenTelemetrySdk depends on OpenTelemetryApi and they are closely linked, the version of the dependency in both podspecs must be identical.

How to test this

Unfortunately, we cannot use the existing examples in the OTel repository. Ideally, we should test that integration with both Pods is achievable and that there are no compiling issues.

To achieve this, I've pushed those two podspecs to a private repo, created two private pods, and tested them in an app. Below are the steps to replicate this process:

Create the private Pods

  1. Create a private git repository.

  2. Clone the private repository locally

  3. Copy both OpenTelemetryApi.podspec and OpenTelemetrySdk.podspec in the cloned repository.

  4. Commit and push both podspecs into your main branch in your private repository.

  5. Publish the Podspecs to your private CocoaPods repo:

    # Replace the github with your git private repository
    pod repo add MyPrivateOtel https://github.com/your-username/your-private-repo.git
    
    # Order matters! Push OpenTelemetryApi First
    pod repo push MyPrivateOtel OpenTelemetryApi.podspec --allow-warnings
    pod repo push MyPrivateOtel OpenTelemetrySdk.podspec --allow-warnings

    We use --allow-warnings because the process might fail due to some "harmless" warnings, like immutable value 'xyz' was never used; consider replacing with '_' or removing it.

  6. Now you can test the two new Pods in your project. However, keep two things in mind:

    • Add the private source at the beginning of your Podfile.
    • Include both dependencies.

    Here’s how your Podfile should look:

    platform :ios, '14.0'
    source 'https://github.com/your-username/your-private-repo'
    
    target 'YourMainTarget' do
       pod 'OpenTelemetryApi'
       pod 'OpenTelemetrySdk'
       # Other pods
       # ...
    end

Extra

To make step 6 easier I created a script that sets up a sample project, adds the two pods as dependencies, imports them, and compiles. I've attached a zip file that contains the script. Run ./test-opentelemetry-pods.sh -h or ./test-opentelemetry-pods.sh --help to see how to use it. The script will install some dependencies to properly create an .xcodeproj and use cocoapods (like bundler and xcodegen). It needs both Xcode and Homebrew to work correctly.
test-opentelemetry-pods.sh.zip

Copy link

linux-foundation-easycla bot commented Aug 16, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@atreat
Copy link
Contributor

atreat commented Sep 10, 2024

I pulled this into a project using this branch and pointing a local path to point to these podspecs

Here's the Podfile

target 'OtelTestApp' do
  use_frameworks!

  pod "OpenTelemetryApi", path: "../otel/opentelemetry-swift"          # Relative path on local machine
  pod "OpenTelemetrySdk", path: "../otel/opentelemetry-swift"          # Relative path on local machine

end

Works great for me, I think this is a good approach for those vendors who want to distribute via source on Cocoapods.

@@ -0,0 +1,24 @@
Pod::Spec.new do |spec|
spec.name = "OpenTelemetryApi"
spec.version = "1.10.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to make sure these versions get bumped when performing releases. Would hope that could be automated in the existing release process.

@atreat
Copy link
Contributor

atreat commented Sep 10, 2024

Also realized another easy way to test is to point the Podfile directly to this branch:

target 'OtelTestApp' do
  use_frameworks!

  pod "OpenTelemetryApi", :git => 'git@github.com:embrace-io/opentelemetry-swift.git', :branch => 'aridemarco/add-specs-for-cocoapods'
  pod "OpenTelemetrySdk", :git => 'git@github.com:embrace-io/opentelemetry-swift.git', :branch => 'aridemarco/add-specs-for-cocoapods'
end

@bryce-b
Copy link
Member

bryce-b commented Sep 18, 2024

I'm attempting to test these podspecs following your testing procedures, but I'm getting errors when cocoapods lints the spec when calling pod repo push MyPrivateOtel OpenTelemetryApi.podspec --allow-warnings

with error:

-> OpenTelemetryApi (1.10.1)
- ERROR | xcodebuild: Returned an unsuccessful exit code. You can use --verbose for more information.
- NOTE | xcodebuild: xcodebuild: error: Unable to find a destination matching the provided destination specifier:
- NOTE | [watchOS] xcodebuild: xcodebuild: error: Found no destinations for the scheme 'App' and action clean.

@ArielDemarco
Copy link
Contributor Author

ArielDemarco commented Sep 19, 2024

It seems to be an issue with the environment (failing to find a destination/simulator to run on for the test app that the pod repo push generates under the hood) and not with the podspecs.
Are you testing with Xcode 16 (whether RC or Beta)? Do you have the different destinations and simulators downloaded for each one? Additionally, to specifically see which one fails, you could add a --verbose flag like this:

pod repo push MyPrivateOtel OpenTelemetryApi.podspec --allow-warnings --verbose

Edit: here's a link of an issue on Cocoapods explaining why this tends to happen

@nachoBonafonte
Copy link
Member

@bryce-b can you confirm your issue is related to what @ArielDemarco mentions?

Copy link
Member

@nachoBonafonte nachoBonafonte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just waiting for @bryce-b confirmation for merging

@bryce-b
Copy link
Member

bryce-b commented Sep 20, 2024

Turns out it I was having issues with the simulators. I'm running the GA version of Xcode 16 right now and I'm having issues confirming it works due unsupported changes in the xcodeproj structure.

@bryce-b
Copy link
Member

bryce-b commented Sep 20, 2024

Looks good to me! After merging, the next step is to push to cocoapods trunk?

@ArielDemarco
Copy link
Contributor Author

ArielDemarco commented Sep 20, 2024

Looks good to me! After merging, the next step is to push to cocoapods trunk?

Exactly! Or automate the process with a worfklow. e.g. any time we tag we generate a cocoapods version associated to that tag.

Edit: One thing to take into account is that the trunk must first be registered. This ensures that only authorized maintainers can push updates to the pod. If this hasn't been set up yet, it involves creating an account and getting authentication tokens from CocoaPods. Here's the setup process. I can help with the process if needed

@bryce-b
Copy link
Member

bryce-b commented Sep 20, 2024

I'm still trying to sort out why the linux test job isn't passing.

@nachoBonafonte
Copy link
Member

I merged yesterday a pr which had issues in Linux. I have fixed with a new merge this early morning, I have tried updating this pr with the latest changes from main, I don't know if it will work

@bryce-b
Copy link
Member

bryce-b commented Sep 20, 2024

@ArielDemarco Please update with the lastest changes from main and the tests should pass.

@ArielDemarco
Copy link
Contributor Author

@ArielDemarco Please update with the lastest changes from main and the tests should pass.

Seems that tests keep failing 🤔

@nachoBonafonte
Copy link
Member

This time it was iOS, retrying as we have some flaky tests

@nachoBonafonte nachoBonafonte merged commit c494e0c into open-telemetry:main Sep 20, 2024
6 checks passed
@nachoBonafonte
Copy link
Member

Thanks a lot for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants