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

fix(logging): CloudWatch Plugin 16KB page size support #2919

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tylerjroach
Copy link
Member

  • PR title and description conform to Pull Request guidelines.

Issue #, if available:

Description of changes:

How did you test these changes?
(Please add a line here how the changes were tested)

Documentation update required?

  • No
  • Yes (Please include a PR link for the documentation update)

General Checklist

  • Added Unit Tests
  • Added Integration Tests
  • Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@tylerjroach
Copy link
Member Author

Need to test a few scenarios given that this isn't just a version bump, but a new library for SqlCipher.

  1. Test on new install (creation of new db)
  2. Test on old install (db still readable)

@vincetran
Copy link
Member

Testing scenario:

  • Hotwired the CloudWatch plugin to NOT sync with the backend to ensure logs stay in the database

Validated:

  1. Tested a brand new install with the NEW SQLCipher dependency can insert events into the database
  2. Pulled the database locally, decrypted it, and validated the contents were correct
  3. Then tested that going BACK to using the OLDER SQLCipher works without issue
  4. Inserting NEW events into the database worked
  5. Pulled the updated database locally, decrypted it, and validated the new AND old events were there
  6. Cleared app data, validated database was deleted.
  7. Relaunched app, inserted a few events, verified a new passphrase was created
  8. Pulled the database locally, decrypted it, and validated the contents were correct
  9. Updated the SQLCipher dependency back to the NEW version
  10. Relaunched app, inserted a few new events, verified the passphrase was the same as in step 7
  11. Pulled the database locally, decrypted it, and validated the contents were correct and that the events that were inserted using the OLD dependency from step 7 were still there

@vincetran
Copy link
Member

Oh also, validated that configuring the cloudwatch plugin to start syncing logs up to CloudWatch successfully uploaded the logs and I could see it in the console :)

@vincetran vincetran marked this pull request as ready for review November 22, 2024 00:09
@vincetran vincetran requested a review from a team as a code owner November 22, 2024 00:09
Copy link

codecov bot commented Nov 22, 2024

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 44.04%. Comparing base (7f874ed) to head (d7bff5f).
Report is 21 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2919      +/-   ##
==========================================
+ Coverage   44.01%   44.04%   +0.02%     
==========================================
  Files         926      932       +6     
  Lines       30485    30601     +116     
  Branches     4351     4524     +173     
==========================================
+ Hits        13419    13478      +59     
- Misses      15601    15622      +21     
- Partials     1465     1501      +36     
---- 🚨 Try these New Features:

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.

2 participants