Merge pull request #344 from getsentry/jb/fix/browser-extension-paths

fix(frame): classify browser extension paths as system frames
getsentry · Oct 23, 2023 · 881a728 · 881a728
2 parents 7232832 + c24fab9
commit 881a728
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 **Bug Fixes**
 
 - Turn non-monotonic samples wall-clock time into monotonic. ([#337](https://github.com/getsentry/vroom/pull/337))
+- Classify browser extension paths as system frames. ([#344](https://github.com/getsentry/vroom/pull/344))
 
 **Internal**
 

diff --git a/internal/frame/frame.go b/internal/frame/frame.go
@@ -14,9 +14,10 @@ import (
 )
 
 var (
-	windowsPathRegex      = regexp.MustCompile(`(?i)^([a-z]:\\|\\\\)`)
-	packageExtensionRegex = regexp.MustCompile(`\.(dylib|so|a|dll|exe)$`)
-	cocoaSystemPackage    = map[string]struct{}{
+	windowsPathRegex                  = regexp.MustCompile(`(?i)^([a-z]:\\|\\\\)`)
+	packageExtensionRegex             = regexp.MustCompile(`\.(dylib|so|a|dll|exe)$`)
+	javascriptSystemPackagePathRegexp = regexp.MustCompile(`node_modules|^(@moz-extension|chrome-extension)`)
+	cocoaSystemPackage                = map[string]struct{}{
 		"Sentry": {},
 	}
 )
@@ -156,6 +157,14 @@ func (f Frame) IsNodeApplicationFrame() bool {
 	return !strings.HasPrefix(f.Path, "node:internal") && !strings.Contains(f.Path, "node_modules")
 }
 
+func (f Frame) IsJavaScriptApplicationFrame() bool {
+	if len(f.Path) == 0 {
+		return true
+	}
+
+	return !javascriptSystemPackagePathRegexp.MatchString(f.Path)
+}
+
 func (f Frame) IsCocoaApplicationFrame() bool {
 	isMain, _ := f.IsMain()
 	if isMain {

diff --git a/internal/frame/frame_test.go b/internal/frame/frame_test.go
@@ -196,6 +196,60 @@ func TestIsNodeApplicationFrame(t *testing.T) {
 	}
 }
 
+func TestIsJavaScriptApplicationFrame(t *testing.T) {
+	tests := []struct {
+		name          string
+		frame         Frame
+		isApplication bool
+	}{
+		{
+			name:          "empty",
+			frame:         Frame{},
+			isApplication: true,
+		},
+		{
+			name: "app",
+			frame: Frame{
+				Path: "/home/user/app/app.js",
+			},
+			isApplication: true,
+		},
+		{
+			name: "node_modules",
+			frame: Frame{
+				Path: "/home/user/app/node_modules/express/lib/express.js",
+			},
+			isApplication: false,
+		},
+		{
+			name: "app",
+			frame: Frame{
+				Path: "@moz-extension://00000000-0000-0000-0000-000000000000/app.js",
+			},
+			isApplication: false,
+		},
+		{
+			name: "app",
+			frame: Frame{
+				Path: "chrome-extension://00000000-0000-0000-0000-000000000000/app.js",
+			},
+			isApplication: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if isApplication := tt.frame.IsJavaScriptApplicationFrame(); isApplication != tt.isApplication {
+				t.Fatalf(
+					"Expected %s frame but got %s frame",
+					frameType(tt.isApplication),
+					frameType(isApplication),
+				)
+			}
+		})
+	}
+}
+
 func TestIsPHPApplicationFrame(t *testing.T) {
 	tests := []struct {
 		name          string

diff --git a/internal/platform/platform.go b/internal/platform/platform.go
@@ -3,11 +3,12 @@ package platform
 type Platform string
 
 const (
-	Android Platform = "android"
-	Cocoa   Platform = "cocoa"
-	Java    Platform = "java"
-	Node    Platform = "node"
-	PHP     Platform = "php"
-	Python  Platform = "python"
-	Rust    Platform = "rust"
+	Android    Platform = "android"
+	Cocoa      Platform = "cocoa"
+	Java       Platform = "java"
+	JavaScript Platform = "javascript"
+	Node       Platform = "node"
+	PHP        Platform = "php"
+	Python     Platform = "python"
+	Rust       Platform = "rust"
 )
diff --git a/internal/sample/sample.go b/internal/sample/sample.go
@@ -403,6 +403,8 @@ func (p *Profile) IsApplicationFrame(f frame.Frame) bool {
 	switch p.Platform {
 	case platform.Node:
 		return f.IsNodeApplicationFrame()
+	case platform.JavaScript:
+		return f.IsJavaScriptApplicationFrame()
 	case platform.Cocoa:
 		return f.IsCocoaApplicationFrame()
 	case platform.Rust: