Skip to content

Conversation

@jboewer
Copy link

@jboewer jboewer commented Jul 2, 2025

What type of PR is this?

/kind feature

What this PR does / why we need it:

Introduces a new --trace-profile [file] flag to generate a Go execution trace, for performance diagnostics using go tool trace.

Currently, buildah provides flags for profiling CPU (--cpu-profile) and memory (--memory-profile) usage.

However, when a build is slow but CPU usage is low, it's difficult to diagnose the bottleneck. These latency issues are often caused by time spent waiting on I/O (network or disk), blocking on system calls, or other scheduler events. The standard Go tool for investigating these specific issues is the execution tracer (go tool trace).

How to verify it

Run any buildah command with the additional --trace-profile flag.
For example

buildah info --trace-profile=/tmp/trace.out
go tool trace /tmp/trace.out

Which issue(s) this PR fixes:

None

Does this PR introduce a user-facing change?

add support for `--trace-profile [file]` to generate an execution trace.

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 2, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 2, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jboewer
Once this PR has been reviewed and has the lgtm label, please assign tomsweeneyredhat for approval. For more information see the 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:

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

Introduces a new --trace-profile [file] flag to generate a Go execution trace, enabling easier diagnosis of latency-related performance issues

Signed-off-by: Jonas Böwer <[email protected]>
Signed-off-by: Jonas Böwer <[email protected]>
@jboewer jboewer force-pushed the feat/tracing-flag branch from 7cd704c to b6f15b3 Compare July 2, 2025 17:34
@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@TomSweeneyRedHat
Copy link
Member

@jboewer thanks for the contribution. Can you add a test or three for this please?

@jboewer
Copy link
Author

jboewer commented Jul 11, 2025

@jboewer thanks for the contribution. Can you add a test or three for this please?

Hey @TomSweeneyRedHat

Just added tests for all 3 available profiling flags.

Though as far as I know, the go tool CLI flags (such as those for go tool trace and go tool pprof) are not covered by Go’s backwards compatibility promise, so the flags or behavior might change in future Go releases and potentially break these tests.

Would you prefer to keep the current assertions that use go tool trace/pprof to validate the generated files, or should we limit the tests to just checking that the files are created and non-empty?

Edit: Tests were just failing as of this very reason. I used go v1.23 locally which had a different go tool trace CLI than the CI's go v1.24. I've switched to using the more stable -pprof=net flag instead of the debug -d=2 flag, but above's question still stands.

@TomSweeneyRedHat
Copy link
Member

@jboewer unless @nalind has a differing opinion, and especially given the "fun" you ran into with v1.23 locally, I'd go with the -pprof=net flag as you suggested.

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@jboewer
Copy link
Author

jboewer commented Sep 3, 2025

Hey @TomSweeneyRedHat @nalind,

I just noticed I still have this PR open. Do you mind revisiting this subject?

Cheers

@TomSweeneyRedHat
Copy link
Member

LGTM
@nalind?

@github-actions github-actions bot removed the stale-pr label Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Categorizes issue or PR as related to a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants