Adding pyroscope and memray dependencies

[vishnuchalla]

Description

Adding pyroscope and memray dependencies

Type of change

• Refactor
• New feature
• Bug fix
• CVE fix
• Optimization
• Documentation Update
• Configuration Update
• Bump-up dependent library
• Bump-up library or tool used for development (does not change the final image)

Related Tickets & Documents

Closes

• https://issues.redhat.com/browse/OLS-964
• https://issues.redhat.com/browse/OLS-965

Checklist before requesting a review

• I have performed a self-review of my code.
• PR has passed all pre-merge test jobs.
• If it is a core feature, I have added thorough tests.

Testing

Tested and verified in local. Both pyroscope and memray get triggered as a part of OLS's startup activity.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tisnik for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 4.54545% with 21 lines in your changes missing coverage. Please review.

Project coverage is 94.32%. Comparing base (32b78e7) to head (e87e0a9).
Report is 25 commits behind head on main.

Files with missing lines	Patch %	Lines
runner.py	0.00%	21 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1550      +/-   ##
==========================================
- Coverage   94.89%   94.32%   -0.57%     
==========================================
  Files          60       60              
  Lines        2685     2698      +13     
==========================================
- Hits         2548     2545       -3     
- Misses        137      153      +16     

Files with missing lines	Coverage Δ
ols/app/models/config.py	97.65% <100.00%> (+<0.01%)	⬆️
runner.py	49.41% <0.00%> (-11.46%)	⬇️

[tisnik]

would it be possible to add these dependencies only for "devel mode"? The same way as GradIO. It will keep number of packages on lowest level possible + images will be smaller + (most important) less CVE-prone

[vishnuchalla]

would it be possible to add these dependencies only for "devel mode"? The same way as GradIO. It will keep number of packages on lowest level possible + images will be smaller + (most important) less CVE-prone

We will need these dependencies when OLS is running through the operator so that we can capture profiles while load testing. Moving them to them dev group will make it impossible.

[tisnik]

would it be possible to add these dependencies only for "devel mode"? The same way as GradIO. It will keep number of packages on lowest level possible + images will be smaller + (most important) less CVE-prone

We will need these dependencies when OLS is running through the operator so that we can capture profiles while load testing. Moving them to them dev group will make it impossible.

ok, but at least loading should be performed if really needed (and configured in config.yaml) right?

[vishnuchalla]

would it be possible to add these dependencies only for "devel mode"? The same way as GradIO. It will keep number of packages on lowest level possible + images will be smaller + (most important) less CVE-prone

We will need these dependencies when OLS is running through the operator so that we can capture profiles while load testing. Moving them to them dev group will make it impossible.

ok, but at least loading should be performed if really needed (and configured in config.yaml) right?

When we say loading, are you referring to import statement in specific?

[tisnik]

would it be possible to add these dependencies only for "devel mode"? The same way as GradIO. It will keep number of packages on lowest level possible + images will be smaller + (most important) less CVE-prone

We will need these dependencies when OLS is running through the operator so that we can capture profiles while load testing. Moving them to them dev group will make it impossible.

ok, but at least loading should be performed if really needed (and configured in config.yaml) right?

When we say loading, are you referring to import statement in specific?

Yes, in the similar way we "optimized" Gradio packages loading

[vishnuchalla]

would it be possible to add these dependencies only for "devel mode"? The same way as GradIO. It will keep number of packages on lowest level possible + images will be smaller + (most important) less CVE-prone

We will need these dependencies when OLS is running through the operator so that we can capture profiles while load testing. Moving them to them dev group will make it impossible.

ok, but at least loading should be performed if really needed (and configured in config.yaml) right?

When we say loading, are you referring to import statement in specific?

I just conducted some experiments to re-evaluate your suggestion. I think moving them to dev makes sense.
And profiling under load doesn't add any value because of huge amount of data that it spits and is impossible to interpret. Example memory profile from memray

-rw-r--r--. 1 1000700000 root       5.2G Sep 11 02:59 output.bin

And pyroscope also captured information that is impossible to interpret. So In order to get better understand on profiling, keeping them in dev group and capturing profiles against one single profile is sufficient enough to take a look at things in detail.
cc: @syedriko @xrajesh

[tisnik]

you can make it as dependency for memray-run, right

[vishnuchalla]

Changed as suggested.

[tisnik]

requests.codes.ok

[vishnuchalla]

Done

[vishnuchalla]

@tisnik can you take a look on this one as well please?

[tisnik]

/hold

[vishnuchalla]

/hold

@tisnik Is there a reason to hold this PR?

[openshift-ci]

@vishnuchalla: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-ols-cluster-4-16	e87e0a9	link	true	/test e2e-ols-cluster-4-16
ci/prow/images	e87e0a9	link	true	/test images
ci/prow/e2e-ols-cluster-4-15	e87e0a9	link	true	/test e2e-ols-cluster-4-15

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.