Skip to content

iOS append events to JSONL file and get diagnostics events#3781

Merged
vegaro merged 22 commits into
mainfrom
sdk-2898-ios-ability-to-create-a-jsonl-file-and-internal-api
Apr 8, 2024
Merged

iOS append events to JSONL file and get diagnostics events#3781
vegaro merged 22 commits into
mainfrom
sdk-2898-ios-ability-to-create-a-jsonl-file-and-internal-api

Conversation

@vegaro

@vegaro vegaro commented Apr 1, 2024

Copy link
Copy Markdown
Member

Based off #2672

Equivalent to RevenueCat/purchases-android#784

Adds DiagnosticsEntry classes and implements adding an entry to the file, getting all entries, and deleting X first entries

@vegaro vegaro added the pr:feat A new feature label Apr 3, 2024
@vegaro vegaro changed the title Sdk 2898 ios ability to create a jsonl file and internal api iOS append events to JSONL file and get diagnostics events Apr 3, 2024
@vegaro vegaro marked this pull request as ready for review April 3, 2024 15:13
Comment thread Tests/UnitTests/Diagnostics/DiagnosticsFileHandlerTests.swift Outdated
Comment thread Sources/Diagnostics/DiagnosticsEntry.swift Outdated
Comment thread Sources/Diagnostics/DiagnosticsEntry.swift Outdated
Comment thread Sources/Diagnostics/DiagnosticsFileHandler.swift
Comment thread Sources/Diagnostics/DiagnosticsFileHandler.swift Outdated
Comment thread Sources/Diagnostics/DiagnosticsFileHandler.swift
Comment thread Sources/Diagnostics/DiagnosticsFileHandler.swift
Comment thread Sources/Misc/Codable/AnyEncodable.swift Outdated
@vegaro vegaro force-pushed the sdk-2898-ios-ability-to-create-a-jsonl-file-and-internal-api branch from 7cef309 to 40cac7e Compare April 4, 2024 12:32
@vegaro vegaro requested a review from a team April 4, 2024 13:11

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks good to me! Just left some comments. Only thing blocking I think is deciding the format for the timestamp property.

Comment thread Sources/Diagnostics/DiagnosticsEntry.swift Outdated
Comment thread Sources/Diagnostics/DiagnosticsEntry.swift Outdated
await fileHandler.append(line: jsonString)
}

func getEntries() async -> [DiagnosticsEvent] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this should be fine, but note that it shouldn't be necessarily needed to parse the entries back to a list of DiagnosticsEntry. We just need a list of jsons to send in the backend request. But I don't think this should be a performance issue, so we can do this 👍

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I see! Just noticed Android is also returning a stream of jsons

Comment thread Sources/Diagnostics/DiagnosticsFileHandler.swift Outdated
Comment thread Tests/UnitTests/Diagnostics/DiagnosticsFileHandlerTests.swift Outdated
Comment thread Tests/UnitTests/Diagnostics/DiagnosticsFileHandlerTests.swift Outdated
Comment thread Tests/UnitTests/Diagnostics/DiagnosticsFileHandlerTests.swift Outdated

@aboedo aboedo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looking great so far! Consider @tonidero's suggestions, and let's make sure that we send the same data on iOS vs android, but other than that it's ready to go

Comment thread Sources/Diagnostics/DiagnosticsFileHandler.swift Outdated
Comment thread Tests/UnitTests/Diagnostics/DiagnosticsFileHandlerTests.swift Outdated
Comment thread Sources/Diagnostics/DiagnosticsEntry.swift Outdated
@vegaro vegaro merged commit 6c4d75f into main Apr 8, 2024
@vegaro vegaro deleted the sdk-2898-ios-ability-to-create-a-jsonl-file-and-internal-api branch April 8, 2024 10:47
@tonidero tonidero added refactor and removed pr:feat A new feature labels Apr 11, 2024
This was referenced Apr 11, 2024
tonidero pushed a commit that referenced this pull request Apr 12, 2024
**This is an automatic release.**

### Bugfixes
* Prevent Template 4 from wrapping Lifetime (#3789) via Josh Holtz
(@joshdholtz)
* Add enum entry for external purchases store (#3779) via Mark
Villacampa (@MarkVillacampa)
### Dependency Updates
* Bump fastlane from 2.219.0 to 2.220.0 (#3783) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* Add option to intercept touch events in `PaywallViewController`
(#3801) via Toni Rico (@tonidero)
* Create DiagnosticsPostOperation (#3795) via Cesar de la Vega (@vegaro)
* Add diagnosticsQueue to BackendConfiguration (#3794) via Cesar de la
Vega (@vegaro)
* Add origin to HTTPResponseType (#3793) via Cesar de la Vega (@vegaro)
* Add name property to HTTPRequestPath (#3790) via Cesar de la Vega
(@vegaro)
* Add name to VerificationResult (#3792) via Cesar de la Vega (@vegaro)
* Add HTTPRequest.DiagnosticsPath (#3791) via Cesar de la Vega (@vegaro)
* Add async `syncAttributesAndOfferingsIfNeeded()` (#3785) via Josh
Holtz (@joshdholtz)
* iOS append events to JSONL file and get diagnostics events (#3781) via
Cesar de la Vega (@vegaro)
* Fix offerings integration test (#3782) via Josh Holtz (@joshdholtz)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants