Skip to content

internal/driver: show sampling period in web profile details - #1035

Open
jabrailkhalil wants to merge 5 commits into
google:mainfrom
jabrailkhalil:feat/web-profile-sampling
Open

jabrailkhalil wants to merge 5 commits into
google:mainfrom
jabrailkhalil:feat/web-profile-sampling

Conversation

@jabrailkhalil

@jabrailkhalil jabrailkhalil commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Show valid CPU/heap sampling metadata in the existing profile-details legend. Preserve readable units and HTML escaping, and omit missing, zero or negative sampling periods.

Current-main integration

Current head: d0c6562c33f0763b2ad81c51f58f65d78c3bca34; integrated main 1facf8f91b50fbab03aa6cb249e0397d91449f9f by a normal, conflict-free merge. All PR implementation/test files are byte-identical to the previous published head. The net PR remains limited to 3 file(s).

Validation

Fresh Linux/amd64, Go 1.26.7 and Chrome 156 validation passes the complete unmodified bash test.sh: both modules, browser tests, race/coverage, vet, formatting, CI-pinned staticcheck commit 452d5bb86b45 and golangci-lint v2.14.0. Separate uncached race-test JSON passes 547 root tests/subtests (4 existing platform skips) and 6 browser tests/subtests, with no browser skips.

Both added CPU and heap Chrome cases executed. All nine sampling metadata unit cases remain covered. This is fresh Linux evidence; macOS and Windows were not rerun.

These local checks are separate from current-head upstream CI authorization and maintainer review.

@jabrailkhalil

Copy link
Copy Markdown
Contributor Author

Integrated current main 7bae8d8c in cf3909eb to clear BEHIND without adding feature scope.

The driver package tests (including sampling metadata cases) and scoped go vet pass. I adapted only the added browser regression to main’s chromedp v0.20.1 typed-selector/Do API; sampling expectations and platform guards remain unchanged, and the browser module compiles.

Fresh validation uses Windows Go 1.26.0, inherited from current main. The full Windows suite is not green: the three PE/DWARF mapping subcases also fail on clean current main in the same environment. The initial run additionally lacked the external diff executable; it is not treated as a source regression. Browser modules retain their x86-64 Linux guards and skip execution on Windows: compilation is not a new Linux/browser/race validation. No existing assertions, timing thresholds, platform guards or dependency versions were weakened. Current-head upstream CI and maintainer review are still needed.

Could a maintainer approve the new-head workflow and review the integration?

@jabrailkhalil

Copy link
Copy Markdown
Contributor Author

Validated unchanged head cf3909eb9f137982fc21952602e14322cf3b7c85 against main 7bae8d8c4c9ebc3886a79f796fbde65b7e1316f8 in an isolated author-fork Linux run. Ubuntu 24.04 / Go 1.26.8 / Chrome 154, with Graphviz and LLVM available. The complete, unmodified test.sh passed, including both modules, race/coverage, staticcheck 2026.2.1, the upstream golangci-lint v2.14.0 commands, vet and formatting. Fresh go test -race -count=1 -json ./... also passed in each module; no browser test was skipped. The four pre-existing root test/subtest platform skips are retained on both head and main.

The CPU and heap sampling Chrome cases actually execute and pass. With only regression tests applied to main, both fail because the details panel is missing the expected sampling text, not because of compilation or Chrome setup. The root web-renderer regression cases also pass on the PR head.

Main passed the same Linux commands. Module manifests, checksums and test.sh are byte-identical between the compared trees. The diagnostic workflow exists only on a separate branch in my fork; this PR head, assertions, deadlines and upstream workflow were not changed. This is author validation on one Linux/Go version, not the full upstream OS/Go matrix or a replacement for required Google CI/review; the earlier Windows failures below are not relabeled as passing.

Could a maintainer authorize the upstream workflow and review the current-head integration?

@jabrailkhalil

jabrailkhalil commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor Author

I integrated main 1facf8f9 in d0c6562c without conflicts. Fresh Linux/amd64, Go 1.26.7 and Chrome 156 validation passes the complete unmodified bash test.sh: both modules, browser tests, race/coverage, vet, formatting, CI-pinned staticcheck commit 452d5bb86b45 and golangci-lint v2.14.0. Separate uncached race-test JSON passes 547 root tests/subtests (4 existing platform skips) and 6 browser tests/subtests, with no browser skips. Upstream CI and review remain separate.

Could a maintainer approve the pending workflow runs for this head?

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