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

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

Merged

Conversation

adrianmacarenco
Copy link
Contributor

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.

@ivnbogdan ivnbogdan merged commit aecdf1f into aptabase:main Aug 9, 2024
4 checks passed
@lucasfischer
Copy link

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
Copy link
Member

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
Copy link
Member

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

@adrianmacarenco
Copy link
Contributor Author

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
Copy link
Contributor Author

@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

@ivnbogdan
Copy link
Contributor

Changes in this PR will be reverted in #27

To be able to accommodate different environments an option can be passed in at init, forcing the events to be tracked as either Debug or Release.
Not setting this value will fallback to the previous behaviour.

More details on setting the option at
https://github.com/aptabase/aptabase-swift/blob/main/CHANGELOG.md#0311

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