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

Add a tool to measure the performance characteristics of providers #76

Merged
merged 3 commits into from
Apr 19, 2023

Conversation

sergenyalcin
Copy link
Member

@sergenyalcin sergenyalcin commented Mar 8, 2023

Description of your changes

With this PR, a tool is added to measure the performance characteristics of providers.

I have:

  • Run make reviewable test to ensure this PR is ready for review.

@sergenyalcin sergenyalcin marked this pull request as draft March 8, 2023 12:29
Copy link

@Piotr1215 Piotr1215 left a comment

Choose a reason for hiding this comment

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

Thank you for putting this together @sergenyalcin , it has been invaluable help in the provider Azure performance tests.

cmd/perf/internal/managed/managed.go Outdated Show resolved Hide resolved
cmd/perf/internal/managed/managed.go Outdated Show resolved Hide resolved
@Piotr1215
Copy link

@sergenyalcin as we briefly discussed, it would be great to add two new functionalities to the tool:

Gather processes from the provider pod

On top of measuring resources utilization, we also want to record processes running in the provider pod to make sure there are no zombie processes 🧟 . A potential implementation idea would be to kick off a separate routine to run the command ps -o pid,ppid,etime,comm,args and capture output to a file.

Capture state of managed resources

In the middle of the experiment, before resources are deleted, we could capture the state of all managed resources with kubectl get managed -o yaml. From there we could check for any additional errors with grep or with the tool itself and save the file for further interrogation.

@sergenyalcin sergenyalcin marked this pull request as ready for review April 17, 2023 12:39
Copy link
Contributor

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

Thanks @sergenyalcin, left some comments for you to consider. I think the only blocking comment is the one from @Piotr1215, regarding the metadata naming of the test resources. Thank you very much.

cmd/perf/internal/common/common.go Outdated Show resolved Hide resolved
cmd/perf/internal/common/common.go Outdated Show resolved Hide resolved
cmd/perf/internal/common/common.go Outdated Show resolved Hide resolved
// Data represents a collected data
type Data struct {
Timestamp time.Time
Value float64
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We may consider using a string here to support metrics of other types, especially state metrics. But my understanding is that we currently support model.Matrix as of now, so this should be fine.

cmd/perf/internal/common/common.go Outdated Show resolved Hide resolved
cmd/perf/internal/managed/managed.go Outdated Show resolved Hide resolved
cmd/perf/internal/managed/managed.go Outdated Show resolved Hide resolved
cmd/perf/internal/quantify.go Show resolved Hide resolved
cmd/perf/internal/quantify.go Show resolved Hide resolved
cmd/perf/internal/quantify.go Show resolved Hide resolved
cmd/perf/main.go Outdated Show resolved Hide resolved
@sergenyalcin sergenyalcin force-pushed the performance-tool branch 4 times, most recently from 84d338b to ccc844c Compare April 19, 2023 09:08
Signed-off-by: Sergen Yalçın <[email protected]>
@sergenyalcin sergenyalcin merged commit 7a0f063 into upbound:main Apr 19, 2023
@sergenyalcin sergenyalcin deleted the performance-tool branch April 19, 2023 15:48
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.

3 participants