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

feat(reports): use S3 presigned URL for archived recording reports #856

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

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Mar 21, 2025

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Replaces #270
Depends on cryostatio/cryostat-reports#200
Related to #269

Description of the change:

See cryostatio/cryostat-reports#200 and #269. This adds a configuration property to control whether this new behaviour is used, and it is enabled by default.

The new behaviour uses the S3 object storage's presigned GET download URLs capability to allow the report generator to retrieve the JFR recording files on its own, rather than Cryostat needing to act as a piping intermediary.

The original implementation in #270 would also handle active recordings in the same way by first creating a transient archived recording copy for these purposes, but this does not reduce overall network traffic or latency and leads to many more potential transient recording copies polluting the storage, so I dropped that idea in this reimplementation. Report requests for active recordings will always use the intermediary pipe method, but requests for archived recordings will use presigned URLs for transfers by default if sidecar report generation is also enabled.

How to manually test:

  1. Check out and build this PR and the corresponding cryostat-reports PR
  2. ./smoketest.bash -Or
  3. podman logs -f compose_reports_1 and podman logs -f compose_cryostat_1 to watch the logs
  4. Open the UI, start some recordings as needed, then request reports for active and archived reports. Archived reports should result in log messages with the "presign" keyword, and in particular for large archived recordings the overall performance should be a bit faster (may not be noticeable in compose localhost environment).

Copy link

@andrewazores
Copy link
Member Author

/build_test

@andrewazores andrewazores marked this pull request as ready for review March 21, 2025 19:06
@andrewazores andrewazores requested a review from a team March 21, 2025 19:06
Copy link

Workflow started at 3/21/2025, 3:06:55 PM. View Actions Run.

Copy link

No OpenAPI schema changes detected.

Copy link

No GraphQL schema changes detected.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat/actions/runs/13999109309

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependent feat New feature or request safe-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant