Reporting updates #49

andrewjl-mux · 2023-07-05T19:51:30Z

Adds events for input standardization success and failure and upload failure using a revised event schema. Upload success also updated to match the revised schema.

Request headers in separate PR #67.

This PR also includes a workaround for an issue with AVAssetExportSession where the asynchronously loading the duration from the standardized file hangs / takes a very long time. As part of the workaround, the duration of the original input file rather than the standardized input file is used when reporting.

andrewjl-mux · 2023-07-07T20:05:49Z

Sources/MuxUploadSDK/InternalUtilities/Reporting/Reporter.swift

@@ -33,83 +40,248 @@ class Reporter: NSObject {
        let jsonEncoder = JSONEncoder()
        jsonEncoder.keyEncodingStrategy = JSONEncoder.KeyEncodingStrategy.convertToSnakeCase
        jsonEncoder.outputFormatting = .sortedKeys
+        jsonEncoder.dateEncodingStrategy = .iso8601


Dates are automatically formatted to be like this: 2023-07-07T04:12:48Z

andrewjl-mux · 2023-07-07T20:09:26Z

Sources/MuxUploadSDK/InternalUtilities/Reporting/Reporter.swift

-                    httpBody: httpBody
-                )
-                completionHandler(request)
+        if let pendingEvent = pendingEvents[ObjectIdentifier(task)], let redirectURL = request.url {


Reporter can now manage multiple in-flight requests and can now be shared, this is a very simplistic implementation, not thread-safe, but will work reasonably outside of weird edge cases.

andrewjl-mux · 2023-07-07T20:11:02Z

Sources/MuxUploadSDK/InternalUtilities/Reporting/Reporter.swift

+
+    func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
+        pendingEvents[
+            ObjectIdentifier(task)


We rely on a hash of the URLSessionTask pointer (ObjectIdentifier) to correlate a request with a pending event.

andrewjl-mux · 2023-07-07T21:08:58Z

Sources/MuxUploadSDK/PublicAPI/MuxUpload.swift

+                        errorDescription: "Input inspection failure",
+                        inputDuration: inspectionResult.sourceInputDuration.seconds,
+                        inputSize: inputSize,
+                        nonStandardInputReasons: [],


Send an empty array here because we cannot report any non-standard input reasons as the inspection itself failed. This is an edge case but something useful surface in a lightweight way as done here.

andrewjl-mux · 2023-07-07T21:11:32Z

Sources/MuxUploadSDK/Upload/ChunkedFileUploader.swift

                    notifyStateFromWorker(.failure(uploadError))
+
+                    // FIXME: Will only work if currentState


Workaround done in order to minimize changing ChunkedFileUploader internals, suggestions for improvement very welcome

andrewjl-mux · 2023-07-14T16:58:08Z

Sources/MuxUploadSDK/PublicAPI/MuxUpload.swift

+            let inputStandardizationStartTime = Date()
+            let reporter = Reporter.shared
+
+            // For consistency report non-std


Retrospectively, now think we should report the standardized input size if we can. I'll update this PR with that change.

daytime-em

I just had the one concern, otherwise it looks great!!

Sources/MuxUploadSDK/InternalUtilities/Reporting/Reporter.swift

* Only mark upload as started if its ready * Reporting updates Add upload failed and input standardization events Add standardization failure event handler to reporter Add standardization success event handler to reporter Rename method for upload success Rename upload event to upload succeeded Support dispatching multiple events at the same time Naive non-thread safe implementation reporter networking Change event type casing, individual files for events Fix reporter test Actually report upload failure Read off file size Use Date in events and serialize to ISO8601 string Finangle duration and start time Set correct version in correct place Keep a session UUID Route every request through one chokepoint Upload failed test Shared test encoder Input standardization failed test Input standardization succeeded test Better organize expected json strings Use a shared instance Remove dupe file * Thread asset duration through inspection and standardization * Pass export error back * Include non standard input reasons * Workaround duration load pause * Add early return in delegate callback * Call correct method * Overloaded methods * Avoid mutating while iterating * tests * Remove unnecessary check

andrewjl-mux · 2023-07-19T00:44:12Z

@daytime-em I think you meant to approve this instead of merging, right? Checking to make sure all the changes requested were actually done.

daytime-em · 2023-07-19T00:45:19Z

Aa!! Sorry, I did want to approve it. Oops!

* Only mark upload as started if its ready * Reporting updates Add upload failed and input standardization events Add standardization failure event handler to reporter Add standardization success event handler to reporter Rename method for upload success Rename upload event to upload succeeded Support dispatching multiple events at the same time Naive non-thread safe implementation reporter networking Change event type casing, individual files for events Fix reporter test Actually report upload failure Read off file size Use Date in events and serialize to ISO8601 string Finangle duration and start time Set correct version in correct place Keep a session UUID Route every request through one chokepoint Upload failed test Shared test encoder Input standardization failed test Input standardization succeeded test Better organize expected json strings Use a shared instance Remove dupe file * Thread asset duration through inspection and standardization * Pass export error back * Include non standard input reasons * Workaround duration load pause * Add early return in delegate callback * Call correct method * Overloaded methods * Avoid mutating while iterating * tests * Remove unnecessary check

Upload Input Inspection and Standardization 1 (#17) Add scaffolding Add initializers Add internal and external state mapping Remove duplicate status enum and add inline docs to external status Add inline API docs to PHAsset-based MuxUpload constructor Consolidate all `MuxUpload` options into a single struct `UploadOptions` Declare asynchronous MuxUpload constructor in PHAsset extension Place extension methods into dedicated directories Polish inline API documentation Add new API documentation and note the placeholder implementation Add option variants as static members: defaults, disabled inputStandardization Deprecate existing initializer, normally this API should be removed prior to GA, but since it was the only initializer exposed up to this point removing it would break everybody. Instead deprecate and remove at a later date. Store all MuxUpload-related options in UploadInfo Use correct starting byte parameter when restarting upload Hardcode request options when exporting AVAsset from PhotosKit chunkSize -> chunkSizeInBytes Input standardization 2 (#41) Maintain handle to canonical input asset inside UploadInput If input standardized, standardized input URL is passed to UploadInfo instead of the original input URL used for initializer Note: SDK probably needs to re-export a high quality asset anyway so possibly need a bridging status Add dedicated internal initializer for MuxUpload error with unknown error code Disable input standardization when running tests Add inspection logic Request local and remote assets Status -> TransportStatus Rely on input status in MuxUpload computed property getters Expose progress from internal upload state if available Track transport status inside of UploadInput Transport status start time optional Add TransportStatus docs Inspect and check video track frame rate Migrate tests for MuxUpload input result handler Input standardization 3 (#46) Standardize via export session Back out outputURL construction into MuxUpload Expose hook for client to cancel upload if standardization failed Call cancellation hook if inspection fails. We're not sure if the input is standard or not so better to be safe and confirm Export based on maximum resolution set by client Cleaner non standard input handler invocation (#57) Add CustomStringConvertible conformance to maximum resolution (#56) Update input state when upload is finished (#63) Update input state when the upload succeeds or fails Tweak input state definition, keep an error in the failed case and the success struct in the success case NFC: remove some if let checks for the handler closure properties in MuxUpload and replace with ? operator. Keeps the existing logic. Replace video file get with actual implementation (#64) Reporting updates (#49) Only mark upload as started if its ready Add upload failed and input standardization events Add standardization failure event handler to reporter Add standardization success event handler to reporter Rename method for upload success Rename upload event to upload succeeded Support dispatching multiple events at the same time Naive non-thread safe implementation reporter networking Change event type casing, individual files for events Fix reporter test Actually report upload failure Read off file size Use Date in events and serialize to ISO8601 string Finangle duration and start time Set correct version in correct place Keep a session UUID Route every request through one chokepoint Upload failed test Shared test encoder Input standardization failed test Input standardization succeeded test Better organize expected json strings Use a shared instance Remove dupe file Thread asset duration through inspection and standardization Pass export error back Include non standard input reasons Workaround duration load pause Avoid mutating while iterating when UploadManager makes delegate calls Set custom HTTP request header for reporting (#67) Remove Equatable and Hashable conformance from MuxUpload (#68) Safe storage for MuxUpload (#71) Intended to prevent a crash if MuxUpload is extended by the SDK client to conform to Equatable or Hashable protocols

andrewjl-mux force-pushed the ajlb/reporting-updates branch from dd2932e to 701958b Compare July 5, 2023 19:52

andrewjl-mux force-pushed the project/input-standardization branch from 4671747 to 2620b24 Compare July 6, 2023 23:07

andrewjl-mux force-pushed the ajlb/reporting-updates branch from df1d57a to 0da3734 Compare July 6, 2023 23:12

andrewjl-mux force-pushed the project/input-standardization branch from 2620b24 to ee815cf Compare July 7, 2023 04:43

andrewjl-mux force-pushed the ajlb/reporting-updates branch from 86de042 to ce7f763 Compare July 7, 2023 04:50

This was referenced Jul 7, 2023

URL request constructor extension #55

Merged

Add CustomStringConvertible conformance to maximum resolution #56

Merged

NFC: Cleaner non standard input handler invocation #57

Merged

andrewjl-mux force-pushed the project/input-standardization branch from 8a0b0d7 to 01ce954 Compare July 7, 2023 19:17

andrewjl-mux force-pushed the ajlb/reporting-updates branch from 10a42c7 to 10538f9 Compare July 7, 2023 19:22

andrewjl-mux force-pushed the project/input-standardization branch from 01ce954 to 86feb86 Compare July 7, 2023 19:27

andrewjl-mux force-pushed the ajlb/reporting-updates branch 4 times, most recently from df10243 to 28878e8 Compare July 7, 2023 19:40

andrewjl-mux mentioned this pull request Jul 7, 2023

Segment reporting related files to their own subdirectory #60

Merged

andrewjl-mux force-pushed the ajlb/reporting-updates branch from 28878e8 to 5bf2ebd Compare July 7, 2023 19:52

andrewjl-mux force-pushed the project/input-standardization branch from 86feb86 to ea5bb97 Compare July 7, 2023 19:53

andrewjl-mux force-pushed the ajlb/reporting-updates branch 2 times, most recently from 78b361c to 1fd605b Compare July 7, 2023 20:01

andrewjl-mux commented Jul 7, 2023

View reviewed changes

andrewjl-mux force-pushed the ajlb/reporting-updates branch 3 times, most recently from d168655 to 0e3a7e3 Compare July 7, 2023 20:54

andrewjl-mux commented Jul 7, 2023

View reviewed changes

andrewjl-mux force-pushed the ajlb/reporting-updates branch from 0e3a7e3 to 465f2d7 Compare July 11, 2023 19:32

andrewjl-mux force-pushed the project/input-standardization branch from ea5bb97 to 8772fd5 Compare July 12, 2023 19:23

andrewjl-mux added 5 commits July 14, 2023 08:39

Pass export error back

878342c

Include non standard input reasons

716afcb

Workaround duration load pause

0082af8

Add early return in delegate callback

42c9b09

Call correct method

f3a6c64

andrewjl-mux force-pushed the ajlb/reporting-updates branch from 61a7f1f to f3a6c64 Compare July 14, 2023 15:39

andrewjl-mux requested a review from a team July 14, 2023 15:39

andrewjl-mux marked this pull request as ready for review July 14, 2023 15:39

andrewjl-mux commented Jul 14, 2023

View reviewed changes

andrewjl-mux added 2 commits July 17, 2023 10:54

Overloaded methods

c29205a

Avoid mutating while iterating

b9607e3

andrewjl-mux force-pushed the ajlb/reporting-updates branch from 3ca1e84 to d15711a Compare July 17, 2023 17:57

tests

04525e9

andrewjl-mux force-pushed the ajlb/reporting-updates branch from d15711a to 04525e9 Compare July 17, 2023 18:03

Remove unnecessary check

a536611

daytime-em requested changes Jul 18, 2023

View reviewed changes

Sources/MuxUploadSDK/InternalUtilities/Reporting/Reporter.swift Show resolved Hide resolved

andrewjl-mux requested review from daytime-em and a team July 18, 2023 17:35

daytime-em merged commit 0c4cd7f into project/input-standardization Jul 18, 2023

daytime-em deleted the ajlb/reporting-updates branch July 18, 2023 17:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reporting updates #49

Reporting updates #49

Uh oh!

andrewjl-mux commented Jul 5, 2023 •

edited

Loading

Uh oh!

andrewjl-mux Jul 7, 2023

Uh oh!

andrewjl-mux Jul 7, 2023

Uh oh!

andrewjl-mux Jul 7, 2023

Uh oh!

andrewjl-mux Jul 7, 2023

Uh oh!

andrewjl-mux Jul 7, 2023

Uh oh!

andrewjl-mux Jul 14, 2023

Uh oh!

daytime-em left a comment

Uh oh!

Uh oh!

andrewjl-mux commented Jul 19, 2023

Uh oh!

daytime-em commented Jul 19, 2023

Uh oh!

Uh oh!

		notifyStateFromWorker(.failure(uploadError))

		// FIXME: Will only work if currentState

Reporting updates #49

Reporting updates #49

Uh oh!

Conversation

andrewjl-mux commented Jul 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewjl-mux Jul 7, 2023

Choose a reason for hiding this comment

Uh oh!

andrewjl-mux Jul 7, 2023

Choose a reason for hiding this comment

Uh oh!

andrewjl-mux Jul 7, 2023

Choose a reason for hiding this comment

Uh oh!

andrewjl-mux Jul 7, 2023

Choose a reason for hiding this comment

Uh oh!

andrewjl-mux Jul 7, 2023

Choose a reason for hiding this comment

Uh oh!

andrewjl-mux Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

daytime-em left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewjl-mux commented Jul 19, 2023

Uh oh!

daytime-em commented Jul 19, 2023

Uh oh!

Uh oh!

andrewjl-mux commented Jul 5, 2023 •

edited

Loading