aptabase / aptabase-swift

Swift SDK for Aptabase: Open Source, Privacy-First and Simple Analytics for Mobile, Desktop and Web Apps
https://aptabase.com
MIT License
21 stars 8 forks source link

fix: isDebug environment for multiple non RELEASE build configs #24

Closed adrianmacarenco closed 1 month ago

adrianmacarenco commented 1 month ago

We are currently experiencing an issue where multiple non-DEBUG configurations, such as RELEASE, STAGING, or TEST, are being reported as RELEASE to Aptabase.

It's common for developers to set up three environments: RELEASE, STAGING/TEST, and DEBUG. Typically, only one environment—RELEASE—is production-related, while the others are used for testing (DEBUG). Therefore, it would make sense to adjust the Aptabase environment logic so that it defaults to DEBUG when it is not RELEASE.

lucasfischer commented 4 weeks ago

I am unable to get any data to show up in 'release'.

IMO, this change is not intuitive, as per default there appears to be no RELEASE environment flag set when creating a new Xcode project.

Would it be possible to revert this change or at least add some documentation on how to set this RELEASE flag?

CleanShot 2024-08-22 at 18 00 27@2x

I tried to set this flag under 'Active Compilation Conditions', which seems to correctly work for code in my project when I do sth like:

#if RELEASE
    print('release')
#endif

But the environment flag does not seem to be respected by the Aptabase library, as sessions still only show up under 'debug'.

cristipufu commented 4 weeks ago

We should also add a nullable property here https://github.com/aptabase/aptabase-swift/blob/main/Sources/Aptabase/InitOptions.swift and let the consumer of the SDK override it

cristipufu commented 3 weeks ago

@adrianmacarenco we're thinking of reverting this and introducing a configurable property in the next release. Would that work for you?

adrianmacarenco commented 3 weeks ago

I am unable to get any data to show up in 'release'.

IMO, this change is not intuitive, as per default there appears to be no RELEASE environment flag set when creating a new Xcode project.

Would it be possible to revert this change or at least add some documentation on how to set this RELEASE flag?

CleanShot 2024-08-22 at 18 00 27@2x

I tried to set this flag under 'Active Compilation Conditions', which seems to correctly work for code in my project when I do sth like:

#if RELEASE
    print('release')
#endif

But the environment flag does not seem to be respected by the Aptabase library, as sessions still only show up under 'debug'.

Here's a revised version:


Hi @lucasfischer , I believe this might be an Apple bug.

The print statement you mentioned seems to be running in the main target, not within an SPM library, which is why you're not encountering the issue.

In the Aptabase SDK, the EnvironmentInfo file is part of an SPM module called "Aptabase." Try creating an SPM module and executing the print statement there. Additionally, create a custom project scheme called STAGING, and then run your project with the STAGING scheme selected. You'll likely notice that the print statement executes, even though it should only be triggered when running in the RELEASE scheme.

adrianmacarenco commented 3 weeks ago

@adrianmacarenco we're thinking of reverting this and introducing a configurable property in the next release. Would that work for you?

Hi @cristipufu,

Regarding the motivation behind this PR, I believe the logic needs some adjustments. Currently, Aptabase only supports RELEASE and DEBUG environments, which makes sense given its purpose.

From a client’s perspective, iOS projects often have multiple environments such as RELEASE, STAGING, TEST, DEBUG, etc. When mapping these environments to Aptabase, it's typical to map RELEASE to RELEASE, while all other environments should be tracked as DEBUG in Aptabase.

For this reason, I suggest replacing isDebug with an isRelease variable and updating the logic accordingly. While isDebug might have worked, it’s not very intuitive, as @lucasfischer pointed out.

The real issue lies in the fact that preprocessor conditionals don’t behave as expected in an SPM module when the client has more than two predefined environments, which is common in the industry. Specifically, the implementation here: https://github.com/aptabase/aptabase-swift/pull/24/commits/ceea493be9969562ac1d4e9bc761f098430ecd04

Introducing an optional property may lead to bugs if clients don’t set the environment, so I don’t think that’s a good idea.

Unless we can resolve this bug with preprocessor conditionals, IMO the ideal solution would be to introduce an isProduction variable to replace isDebug, requiring the client to set it when initializing the Aptabase library:

Aptabase.shared.initialize(appKey: YOUR_KEY, isRelease: isReleaseClientMapping)

What do you think? @lucasfischer @cristipufu