Skip to content

Create DiagnosticsPostOperation#3795

Merged
vegaro merged 8 commits into
mainfrom
create-diagnostics-api
Apr 11, 2024
Merged

Create DiagnosticsPostOperation#3795
vegaro merged 8 commits into
mainfrom
create-diagnostics-api

Conversation

@vegaro

@vegaro vegaro commented Apr 10, 2024

Copy link
Copy Markdown
Member

Add DiagnosticsPostOperation and DiagnosticsAPI

@vegaro vegaro force-pushed the create-diagnostics-api branch from 1e2bf4c to c088873 Compare April 10, 2024 12:33
@vegaro vegaro force-pushed the create-diagnostics-api branch from c088873 to 30482f3 Compare April 10, 2024 13:44
@vegaro vegaro requested a review from a team April 10, 2024 14:10

@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.

Just a couple of questions but looking good!


import Foundation

typealias DiagnosticsEntries = [String]

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.

Oh so the whole body of each event is sent as a string then? We should be careful with encoding characters then, but maybe Body is already taking care of that? (not sure)

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.

So Body is Encodable and Body has the entries, so I think this should work. But I am going to change it to the way PostAttributionDataOperation works. It stores the data as AnyEncodable instead of [String:Any]

Comment thread Sources/Networking/DiagnosticsAPI.swift Outdated
@vegaro vegaro requested a review from tonidero April 10, 2024 18:27
"request" : {
"body" : {
"data" : [
"{\n \"properties\": {\"key\": \"value\"},\n \"timestamp\": \"2024-04-04T12:55:59Z\",\n \"name\": \"HTTP_REQUEST_PERFORMED\",\n \"version\": 1\n}",

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.

Hmm we should send these as json objects, instead of strings. Maybe we need to process it back to an object after all?

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.

hmm good point, I hadn't noticed that! Good catch

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.

Ended up doing a refactor to make the code similar to Paywall Events, and now they are sent as json with the correct date format. lmkwyt

@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.

LGTM!

@vegaro vegaro merged commit f813e32 into main Apr 11, 2024
@vegaro vegaro deleted the create-diagnostics-api branch April 11, 2024 15:12
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.

2 participants