Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if parts: | ||
| mod = parts[0] | ||
| if mod.startswith("github.com/getsentry/sentry-go"): | ||
| sentry_modules.append(mod) |
There was a problem hiding this comment.
Go.mod parser misses single-line require directives
High Severity
The go.mod parser in _prepare_go_sdk_version takes parts[0] to get the module name, but this fails for single-line require directives like require github.com/getsentry/sentry-go v0.42.0 where parts[0] is "require", not the module path. The go/net-http app uses exactly this format, so its sentry-go dependency will never be updated — the function will log a warning and return early without calling go get.
|
|
||
| on: | ||
| pull_request: | ||
| branches: [main, feat/github-actions] |
There was a problem hiding this comment.
Development branch left in workflow trigger
Low Severity
The pull_request trigger includes feat/github-actions alongside main in the branches list. This looks like a temporary addition for testing the workflow during development and will become dead configuration once the feature branch is merged.
SDK Benchmark Results —
|
| Metric | Overhead |
|---|---|
| p50 | +23.89% |
| p90 | +3.16% |
| p95 | +3.24% |
| p99 | +2.72% |
| mean | +11.88% |
Details
- Iterations: 2
- RPS: 100
- Duration: 30s
Add Go SDK version handling to the benchmark runner so that `_prepare_sdk_version` can update sentry-go modules in go.mod via `go get` + `go mod tidy` before Docker builds. Add an e2e workflow that runs all three Go apps (net-http, gin, echo) against the latest sentry-go version with a single iteration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Temporary — allows testing the workflow on this PR before the base branch is merged to main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add conditional Go setup to benchmark.yml for Go apps - Fix find results bug (maxdepth) in benchmark.yml - Rewrite e2e-go.yml to resolve version then call benchmark.yml via matrix strategy, removing duplicated benchmark logic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lcian
force-pushed
the
feat/e2e-go-benchmarks
branch
from
546e166 to
883a2d0
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| if: startsWith(inputs.app, 'go/') | ||
| with: | ||
| go-version: "1.25" | ||
| cache: false |
There was a problem hiding this comment.
Duplicate setup-go step accidentally added to workflow
Low Severity
The actions/setup-go@v5 step (lines 53–57) is an exact duplicate of the step already present at lines 47–51. Both have the same if condition, go-version, and cache setting. This appears to be an accidental copy-paste that causes Go to be set up twice for every Go benchmark run.
The run_benchmark function was collecting raw iteration data but never aggregating it into the summary.overhead structure that format_comment expects. This resulted in empty metric tables in PR comments. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
_prepare_sdk_version) — parsesgo.modfor sentry-go modules and updates them viago get+go mod tidybefore Docker buildse2e-go.ymlworkflow that runs all three Go benchmark apps (net-http, gin, echo) against the latest sentry-go version with 1 iterationworkflow_dispatchwith optional version overrideTest plan
workflow_dispatchand verify all 3 Go apps complete successfully🤖 Generated with Claude Code