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.

Sign up for GitHub

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

Jump to bottom

feat(internal/uploader): UDENG-5706 ubuntu insights uploader #9

Merged
merged 36 commits into from
Feb 3, 2025
Merged

feat(internal/uploader): UDENG-5706 ubuntu insights uploader #9

merged 36 commits into from
Feb 3, 2025
+1,983 −38

Conversation

hk21702
Copy link
Collaborator

@hk21702 hk21702 commented Jan 23, 2025 •
edited
Loading

Implements the uploader component along with relevant tests.

  • For a given source, find all the collected reports
  • Check min age against the report timestamp
  • Parse correct URL/endpoint
  • Check consent, replace payload with opt out if out out
  • Send payload to server with 10 second timeout
  • Write payload to uploaded folder, remove report from collected folder

UDENG-5706

hk21702 and others added 24 commits January 13, 2025 14:35
@hk21702
Remove flock, change consent write to be atomic, measure file count i…
e8b8d19 
…n set test
@hk21702
Merge branch 'main' into UDENG-5705-Ubuntu-Insights-Consent-Manager
ed274bb
@hk21702
Re-add golangci
bd400fc
@hk21702
fixup! Remove flock, change consent write to be atomic, measure file …
d4464e8 
…count in set test
@hk21702
Merge branch 'main' into UDENG-5705-Ubuntu-Insights-Consent-Manager
976b28c
@hk21702
Merge branch 'main' into UDENG-5705-Ubuntu-Insights-Consent-Manager
a5f6bad
@hk21702
Fix testdata whitespace
db013c3
@hk21702
Merge branch 'main' into UDENG-5705-Ubuntu-Insights-Consent-Manager
425ef87
@hk21702
Enhance consent component with improved logging and error messages
8d76e98
@hk21702
Initial report utils
4ed6a88
@hk21702
Refactor consent component
ddba13a
@hk21702
Refactor consent manager to simplify functions
928f55a
@Sploder12 @hk21702
Add mac and windows sysinfo bones
4da7645
@Sploder12 @hk21702
Apply linting
288c8ee
@hk21702
Merge branch 'main' into UDENG-5705-Ubuntu-Insights-Consent-Manager
2e873d4
@hk21702
Merge branch 'UDENG-5705-Ubuntu-Insights-Consent-Manager' into UDENG-…
75457ae 
…5706-ubuntu-insights-uploader
@hk21702
Refactor and implement tests for reportutils
bfa4099
@hk21702
Refactor consent manager to simplify functions
46e0dd6
@Sploder12 @hk21702
Add mac and windows sysinfo bones
0951650
@Sploder12 @hk21702
Apply linting
5b1c9f7
@hk21702
Merge branch 'main' into UDENG-5705-Ubuntu-Insights-Consent-Manager
d6c9d77
@hk21702
Merge branch 'UDENG-5705-Ubuntu-Insights-Consent-Manager' into UDENG-…
9d1bd7d 
…5706-ubuntu-insights-uploader
@hk21702
Merge branch 'UDENG-5705-Ubuntu-Insights-Consent-Manager' into UDENG-…
b91860c 
…5706-ubuntu-insights-uploader
@hk21702
Add file utility functions for atomic writing and existence checking;…
7427c2c 
… enhance report retrieval logic
@hk21702 hk21702 self-assigned this Jan 23, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 84.88746% with 47 lines in your changes missing coverage. Please review.

Project coverage is 72.90%. Comparing base (d127e02) to head (0378ed1).

Files with missing lines Patch % Lines
internal/uploader/upload.go 79.80% 14 Missing and 7 partials ⚠️
internal/report/report.go 89.51% 10 Missing and 5 partials ⚠️
internal/fileutils/fileutils.go 55.00% 6 Missing and 3 partials ⚠️
internal/uploader/uploader.go 93.33% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##             main       #9       +/-   ##
===========================================
+ Coverage   60.88%   72.90%   +12.02%     
===========================================
  Files          13       17        +4     
  Lines         294      598      +304     
===========================================
+ Hits          179      436      +257     
- Misses        102      134       +32     
- Partials       13       28       +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch from 68c2fc0 to b1a6d21 Compare January 23, 2025 22:09
@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch from b1a6d21 to 64e2178 Compare January 23, 2025 22:16
@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch 7 times, most recently from 25da965 to a52eafa Compare January 30, 2025 20:23
@hk21702
Copy link
Collaborator Author

hk21702 commented Jan 30, 2025 •
edited
Loading

I haven't put in the behavior for duplicate reports yet, since I figure that it should probably be something discussed as a group.

@hk21702 hk21702 marked this pull request as ready for review January 30, 2025 20:59
@hk21702 hk21702 mentioned this pull request Jan 30, 2025
Open
@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch from a52eafa to 42ce446 Compare January 30, 2025 21:17
@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch from 42ce446 to f5a0ee4 Compare January 30, 2025 21:49
@hk21702 hk21702 requested a review from Sploder12 January 31, 2025 16:05
Sploder12
Sploder12 reviewed Jan 31, 2025
View reviewed changes
Copy link
Collaborator

@Sploder12 Sploder12 left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Some small questions and readability. But the commit history is a bit strange.

internal/uploader/upload.go Outdated Show resolved Hide resolved
internal/uploader/uploader_test.go Outdated Show resolved Hide resolved
internal/uploader/testdata/TestUpload/golden/consent_manager_global_false,_source_true Outdated Show resolved Hide resolved
@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch from de2b460 to 187ac26 Compare January 31, 2025 22:13
@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch from 187ac26 to dd85be6 Compare January 31, 2025 22:34
@hk21702
Copy link
Collaborator Author

hk21702 commented Jan 31, 2025

The commit history is a bit of a mess yeah. I think it is the product of doing git merge from main or a parent branch, and then at some point doing a rebase to squash fix up commits.

@hk21702 hk21702 requested a review from Sploder12 February 3, 2025 16:07
Sploder12
Sploder12 reviewed Feb 3, 2025
View reviewed changes
Copy link
Collaborator

@Sploder12 Sploder12 left a comment

Choose a reason for hiding this comment

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

Just a few small things then this should be good to go!

internal/report/report.go Outdated Show resolved Hide resolved
internal/report/report.go Outdated Show resolved Hide resolved
internal/report/report_test.go Outdated Show resolved Hide resolved
internal/report/report_test.go Outdated Show resolved Hide resolved
Sploder12
Sploder12 approved these changes Feb 3, 2025
View reviewed changes
Copy link
Collaborator

@Sploder12 Sploder12 left a comment

Choose a reason for hiding this comment

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

LGTM! Just a couple tiny nitpicks

internal/report/report.go Outdated Show resolved Hide resolved
internal/report/report_test.go Outdated Show resolved Hide resolved
@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch from cc1cbad to f1c6c53 Compare February 3, 2025 20:02
@hk21702 hk21702 force-pushed the UDENG-5706-ubuntu-insights-uploader branch from f1c6c53 to 0378ed1 Compare February 3, 2025 20:09
@hk21702 hk21702 merged commit bb2413e into main Feb 3, 2025
14 checks passed
@hk21702 hk21702 deleted the UDENG-5706-ubuntu-insights-uploader branch February 3, 2025 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sploder12 Sploder12 Sploder12 approved these changes

Assignees

@hk21702 hk21702

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hk21702 @codecov-commenter @Sploder12