Skip to content

Commit

Permalink
Merge branch 'main' of github.com:getsentry/vroom into txiao/chore/ap…
Browse files Browse the repository at this point in the history
…pend-experimental-to-function-regression-issue
  • Loading branch information
Zylphrex committed Oct 6, 2023
2 parents 3587176 + 2ac850d commit 1e22875
Show file tree
Hide file tree
Showing 16 changed files with 151 additions and 52 deletions.
38 changes: 30 additions & 8 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,12 @@ defaults:
shell: bash

jobs:
test-vroom:
trufflehog:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- uses: actions/setup-go@v3
with:
go-version: 'stable'
- run: go install golang.org/x/tools/cmd/goimports@latest
- run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.51.1
- uses: pre-commit/[email protected]
- run: make test
- name: TruffleHog OSS
uses: trufflesecurity/[email protected]
with:
Expand All @@ -41,6 +34,35 @@ jobs:
head: HEAD
extra_args: --debug --only-verified

lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- uses: actions/setup-go@v4
with:
go-version: stable
cache: false
- run: go install golang.org/x/tools/cmd/goimports@latest
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: latest
- uses: pre-commit/[email protected]

test-vroom:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- uses: actions/setup-go@v4
with:
go-version: stable
cache: false
- run: make test

publish-to-dockerhub:
name: Publish Vroom to DockerHub
runs-on: ubuntu-20.04
Expand Down
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,23 @@
**Features**

- Return the first and deepest issue detected. ([#317](https://github.com/getsentry/vroom/pull/317))
- Release frame drop issue detection. ([#329](https://github.com/getsentry/vroom/pull/329))
- Append experimental to function regression issue. ([#334](https://github.com/getsentry/vroom/pull/334))

**Bug Fixes**

- Close remaining open events in Android profiles. ([#316](https://github.com/getsentry/vroom/pull/316))
- Enforce minimum frame duration for frame drop issue. ([#319](https://github.com/getsentry/vroom/pull/319))
- Mark sentry frames as system frames when it's dynamically linked. ([#325](https://github.com/getsentry/vroom/pull/325))
- Do not return an occurrence for unknown function or when the stack is filled with them. ([#328](https://github.com/getsentry/vroom/pull/328))

**Internal**

- Rename the frame drop issue title. ([#315](https://github.com/getsentry/vroom/pull/315))
- Add new endpoint for regressed functions. ([#318](https://github.com/getsentry/vroom/pull/318))
- Return 502 from health endpoint on shutdown. ([#323](https://github.com/getsentry/vroom/pull/323)), ([#324](https://github.com/getsentry/vroom/pull/324))

- Health endpoint returns 200 instead of 204 on success. ([#326](https://github.com/getsentry/vroom/pull/326))
- Bump max profile duration for which we generate call trees. ([#330](https://github.com/getsentry/vroom/pull/330))

## 23.9.1

Expand Down
22 changes: 15 additions & 7 deletions cmd/issuedetection/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package main

import (
"errors"
"flag"
"fmt"
"io"
"io/fs"
"log"
"log/slog"
"os"
"path/filepath"
"sync"
Expand All @@ -22,14 +24,20 @@ const (
)

func main() {
args := os.Args[1:]
if len(args) != 1 {
fmt.Println("./issuedetection <profiles directory>") // nolint
return
debug := flag.Bool("debug", false, "activate debug logs")
root := flag.String("path", ".", "path to a profile or a directory with profiles")

flag.Parse()

if *debug {
opts := slog.HandlerOptions{
Level: slog.LevelDebug,
}
handler := slog.NewTextHandler(os.Stdout, &opts)
slog.SetDefault(slog.New(handler))
}

root := args[0]
f, err := os.Open(root)
f, err := os.Open(*root)
if err != nil {
log.Fatal(err)
}
Expand All @@ -51,7 +59,7 @@ func main() {
go AnalyzeProfile(pathChannel, errChannel, &wg)
}

err = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
err = filepath.WalkDir(*root, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/vroom/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func main() {

func (e *environment) getHealth(w http.ResponseWriter, _ *http.Request) {
if _, err := os.Stat("/tmp/vroom.down"); err != nil {
w.WriteHeader(http.StatusNoContent)
w.WriteHeader(http.StatusOK)
} else {
w.WriteHeader(http.StatusBadGateway)
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/getsentry/vroom

go 1.18
go 1.21

require (
cloud.google.com/go/compute/metadata v0.2.3
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm
github.com/gin-gonic/gin v1.6.3/go.mod h1:75u5sXoLsGZoRN5Sgbi1eraJ4GU3++wFwWzhwvtwp4M=
github.com/gin-gonic/gin v1.7.7/go.mod h1:axIBovoeJpVj8S3BwE0uPMTeReE4+AfFtqpqaZ1qq1U=
github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
Expand Down Expand Up @@ -1602,6 +1603,7 @@ github.com/pierrec/lz4/v4 v4.1.12/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFu
github.com/pierrec/lz4/v4 v4.1.15 h1:MO0/ucJhngq7299dKLwIMtgTfbkoSPF6AoMYDd8Q4q0=
github.com/pierrec/lz4/v4 v4.1.15/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4=
github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8=
github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4/go.mod h1:N6UoU20jOqggOuDwUaBQpluzLNDqif3kq9z2wpdYEfQ=
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI=
Expand Down
2 changes: 1 addition & 1 deletion gocd/templates/jsonnetfile.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"subdir": "libs"
}
},
"version": "v2.2.2"
"version": "v2.3.1"
}
],
"legacyImports": true
Expand Down
4 changes: 2 additions & 2 deletions gocd/templates/jsonnetfile.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
"subdir": "libs"
}
},
"version": "54bb0adceffb690bf4aa744b23acdb7888c51fd5",
"sum": "stYmA7r5/MARC5qkFFR+94R1W5juwjmRNVQFOTxmwVA="
"version": "cfd0a0c54a580e1932e14368d0c99b2b5013c434",
"sum": "SFeCD13Z2qQftwrjVAKE19IjjgJAk8ninSJwePKhI8A="
}
],
"legacyImports": false
Expand Down
46 changes: 27 additions & 19 deletions gocd/templates/pipelines/vroom.libsonnet
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
local gocdtasks = import 'github.com/getsentry/gocd-jsonnet/libs/gocd-tasks.libsonnet';

local deploy_canary_stage(region) =
if region == 'us' then
[
{
'deploy-canary': {
fetch_materials: true,
jobs: {
deploy: {
timeout: 600,
elastic_profile_id: 'vroom',
environment_variables: {
LABEL_SELECTOR: 'service=vroom,environment=production,env=canary',
WAIT_MINUTES: '5',
},
tasks: [
gocdtasks.script(importstr '../bash/deploy.sh'),
gocdtasks.script(importstr '../bash/wait-canary.sh'),
],
},
},
},
},
]
else
[];

function(region) {
environment_variables: {
GITHUB_TOKEN: '{{SECRET:[devinfra-github][token]}}',
Expand Down Expand Up @@ -31,25 +57,7 @@ function(region) {
},
},
},
{
'deploy-canary': {
fetch_materials: true,
jobs: {
deploy: {
timeout: 600,
elastic_profile_id: 'vroom',
environment_variables: {
LABEL_SELECTOR: 'service=vroom,environment=production,env=canary',
WAIT_MINUTES: '5',
},
tasks: [
gocdtasks.script(importstr '../bash/deploy.sh'),
gocdtasks.script(importstr '../bash/wait-canary.sh'),
],
},
},
},
},
] + deploy_canary_stage(region) + [
{
'deploy-primary': {
fetch_materials: true,
Expand Down
5 changes: 3 additions & 2 deletions gocd/templates/vroom.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ local pipedream = import 'github.com/getsentry/gocd-jsonnet/libs/pipedream.libso
local pipedream_config = {
name: 'vroom',
exclude_regions: [
's4s',
'customer-1',
'customer-2',
'customer-3',
'customer-4',
'customer-5',
'customer-6',
],
materials: {
Expand All @@ -24,6 +22,9 @@ local pipedream_config = {
material_name: 'vroom_repo',
stage: 'deploy-primary',
elastic_profile_id: 'vroom',
// TODO: Remove the final_stage value once v2.3.1 a few goocd deploys are
// done with the pipeline-complete stage
final_stage: 'deploy-primary',
},
};

Expand Down
10 changes: 10 additions & 0 deletions internal/frame/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import (
var (
windowsPathRegex = regexp.MustCompile(`(?i)^([a-z]:\\|\\\\)`)
packageExtensionRegex = regexp.MustCompile(`\.(dylib|so|a|dll|exe)$`)
cocoaSystemPackage = map[string]struct{}{
"Sentry": {},
}
)

type (
Expand Down Expand Up @@ -160,6 +163,13 @@ func (f Frame) IsCocoaApplicationFrame() bool {
// as a system frame as it does not contain any user code
return false
}

// Some packages are known to be system packages.
// If we detect them, mark them as a system frame immediately.
if _, exists := cocoaSystemPackage[f.ModuleOrPackage()]; exists {
return false
}

return packageutil.IsCocoaApplicationPackage(f.Package)
}

Expand Down
8 changes: 8 additions & 0 deletions internal/frame/frame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ func TestIsCocoaApplicationFrame(t *testing.T) {
},
isApplication: true,
},
{
name: "symbolicate_internal",
frame: Frame{
Function: "symbolicate_internal",
Package: "/private/var/containers/Bundle/Application/00000000-0000-0000-0000-000000000000/App.app/Frameworks/Sentry.framework/Sentry",
},
isApplication: false,
},
}

for _, tt := range tests {
Expand Down
10 changes: 10 additions & 0 deletions internal/occurrence/detect_frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"strings"
"time"

"log/slog"

"github.com/getsentry/vroom/internal/frame"
"github.com/getsentry/vroom/internal/nodetree"
"github.com/getsentry/vroom/internal/platform"
Expand Down Expand Up @@ -120,6 +122,7 @@ func (options DetectAndroidFrameOptions) checkNode(n *nodetree.Node) *nodeInfo {
// Check if we have a list of functions associated to the package.
functions, exists := options.FunctionsByPackage[n.Package]
if !exists {
slog.Debug("package doesn't exist", slog.String("package", n.Package))
return nil
}

Expand All @@ -135,16 +138,19 @@ func (options DetectAndroidFrameOptions) checkNode(n *nodetree.Node) *nodeInfo {
// Check if we need to detect that function.
category, exists := functions[name]
if !exists {
slog.Debug("function doesn't exist", slog.String("function", name))
return nil
}

// Check if it's above the duration threshold.
if n.DurationNS < uint64(options.DurationThreshold) {
slog.Debug("duration is too small", slog.Uint64("duration_ns", n.DurationNS))
return nil
}

// Check if it's above the sample threshold.
if n.SampleCount < options.SampleThreshold {
slog.Debug("sample count is too low", slog.Int("sample_count", n.SampleCount))
return nil
}

Expand Down Expand Up @@ -470,6 +476,10 @@ func detectFrame(
if options.onlyCheckActiveThread() {
callTrees, exists := callTreesPerThreadID[p.Transaction().ActiveThreadID]
if !exists {
slog.Debug(
"call tree for active thread ID doesn't exist",
slog.Uint64("active_thread_id", p.Transaction().ActiveThreadID),
)
return
}
for _, root := range callTrees {
Expand Down
23 changes: 17 additions & 6 deletions internal/occurrence/frame_drop.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,21 @@ func newFrozenFrameStats(endNS uint64, durationNS float64) frozenFrameStats {
// nodeStackIfValid returns the nodeStack if we consider it valid as
// a frame drop cause.
func (s *frozenFrameStats) IsNodeStackValid(ns *nodeStack) bool {
return ns.n.StartNS >= s.startNS &&
return ns.n.Frame.Function != "" &&
ns.n.IsApplication &&
ns.n.StartNS >= s.startNS &&
ns.n.EndNS <= s.endNS &&
ns.n.DurationNS >= s.minDurationNS &&
ns.n.StartNS <= s.startLimitNS &&
ns.n.IsApplication
ns.n.StartNS <= s.startLimitNS
}

const (
FrameDrop Category = "frame_drop"

marginPercent float64 = 0.05
minFrameDurationPercent float64 = 0.5
startLimitPercent float64 = 0.2
marginPercent float64 = 0.05
minFrameDurationPercent float64 = 0.5
startLimitPercent float64 = 0.2
unknownFramesInTheStackThreshold float64 = 0.8
)

func findFrameDropCause(
Expand Down Expand Up @@ -85,9 +87,18 @@ func findFrameDropCause(
}
// We found a potential stacktrace responsible for this frozen frame
stackTrace := make([]frame.Frame, 0, len(cause.st))
var unknownFramesCount float64
for _, f := range cause.st {
if f.Frame.Function == "" {
unknownFramesCount++
}
stackTrace = append(stackTrace, f.ToFrame())
}
// If there are too many unknown frames in the stack,
// we do not create an occurrence.
if unknownFramesCount >= float64(len(stackTrace))*unknownFramesInTheStackThreshold {
continue
}
*occurrences = append(
*occurrences,
NewOccurrence(p, nodeInfo{
Expand Down
Loading

0 comments on commit 1e22875

Please sign in to comment.