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

Offline storage to file system on failed upload #189

Merged
merged 23 commits into from
Feb 14, 2024
Merged

Conversation

mbruin-NR
Copy link
Contributor

This draft for the offline storage has the harvest data that failed to send stored to the local file system when a request has a -1009 or -1001. All of the recorded harvests are then sent immediately after a request succeeds an upload.
Consider also adding -1003, -1004, and -1005.

@cdillard-NewRelic

This comment was marked as outdated.

@cdillard-NewRelic

This comment was marked as outdated.

@mbruin-NR mbruin-NR marked this pull request as ready for review December 20, 2023 18:37
@cdillard-NewRelic cdillard-NewRelic self-requested a review December 20, 2023 18:47
Copy link
Member

@cdillard-NewRelic cdillard-NewRelic left a comment

Choose a reason for hiding this comment

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

Latest update looking good to me I see the log when a failed harvest is persisted to disk

NewRelic(DEV,0x2804c1640):	NRMAOfflineStorage.m:47	-[NRMAOfflineStorage persistDataToDisk:]
	Successfully persisted failed upload data to disk for offline storage.

and when a harvest is successful I see the proper logs referencing offline harvests sent now that internet is back.

NewRelic(DEV,0x2804eecc0):	NRMAOfflineStorage.m:66	-[NRMAOfflineStorage getAllOfflineData:]_block_invoke
	Offline storage to be uploaded from 2023-12-28-10-58-57.txt
NewRelic(DEV,0x2804eecc0):	NRMAHarvesterConnection.m:39	-[NRMAHarvesterConnection sendOfflineStorage]
	Number of offline data posts: 1

I tested both airplane mode and 100% loss mode (via Network Link Conditioner.

Id say this is ready to be feature flagged behind a "OfflineInstrumentation" or similarly named flag and shipped 👍

@cdillard-NewRelic cdillard-NewRelic self-requested a review January 8, 2024 21:45
smalsam-newr
smalsam-newr previously approved these changes Jan 26, 2024
Copy link
Member

@cdillard-NewRelic cdillard-NewRelic left a comment

Choose a reason for hiding this comment

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

Offline Lookin' good!

@cdillard-NewRelic
Copy link
Member

Please request code review when PR is ready with additional changes.

@mbruin-NR mbruin-NR merged commit 0e3488f into develop Feb 14, 2024
5 checks passed
@mbruin-NR mbruin-NR deleted the offlineStoragePOC branch February 14, 2024 15:41
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