Skip to content

Commit

Permalink
fix(chunk): Normalize in_app value for profile chunks (#488)
Browse files Browse the repository at this point in the history
  • Loading branch information
phacops authored Jul 22, 2024
1 parent 13c6deb commit f0ea408
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 53 deletions.
2 changes: 2 additions & 0 deletions cmd/vroom/chunk.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ func (env *environment) postChunk(w http.ResponseWriter, r *http.Request) {
return
}

c.Normalize()

if hub != nil {
hub.Scope().SetContext("Profile metadata", map[string]interface{}{
"chunk_id": c.ID,
Expand Down
8 changes: 7 additions & 1 deletion cmd/vroom/chunk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/getsentry/vroom/internal/chunk"
"github.com/getsentry/vroom/internal/frame"
"github.com/getsentry/vroom/internal/platform"
"github.com/getsentry/vroom/internal/storageutil"
"github.com/getsentry/vroom/internal/testutil"
"github.com/google/uuid"
Expand Down Expand Up @@ -60,7 +61,11 @@ func TestPostAndReadChunk(t *testing.T) {
ProjectID: 1,
Profile: chunk.Data{
Frames: []frame.Frame{
{Function: "test"},
{
Function: "test",
InApp: &testutil.True,
Platform: platform.Python,
},
},
Stacks: [][]int{
{0},
Expand Down Expand Up @@ -141,6 +146,7 @@ type KafkaWriterMock struct{}
func (k KafkaWriterMock) WriteMessages(_ context.Context, _ ...kafka.Message) error {
return nil
}

func (k KafkaWriterMock) Close() error {
return nil
}
8 changes: 8 additions & 0 deletions internal/chunk/chunk.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ func (c *Chunk) StartEndTimestamps() (float64, float64) {
return c.Profile.Samples[0].Timestamp, c.Profile.Samples[count-1].Timestamp
}

func (c *Chunk) Normalize() {
for i := range c.Profile.Frames {
f := c.Profile.Frames[i]
f.Normalize(c.Platform)
c.Profile.Frames[i] = f
}
}

func StoragePath(OrganizationID uint64, ProjectID uint64, ProfilerID string, ID string) string {
return fmt.Sprintf(
"%d/%d/%s/%s",
Expand Down
59 changes: 58 additions & 1 deletion internal/frame/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ func (f Frame) IsPythonApplicationFrame() bool {
if strings.Contains(f.Path, "/site-packages/") ||
strings.Contains(f.Path, "/dist-packages/") ||
strings.Contains(f.Path, "\\site-packages\\") ||
strings.Contains(f.Path, "\\dist-packages\\") {
strings.Contains(f.Path, "\\dist-packages\\") ||
strings.HasPrefix(f.Path, "/usr/local/") {
return false
}

Expand Down Expand Up @@ -278,3 +279,59 @@ func (f Frame) FullyQualifiedName(p platform.Platform) string {
}
return formatter(f)
}

func (f *Frame) SetInApp(p platform.Platform) {
// for react-native the in_app field seems to be messed up most of the times,
// with system libraries and other frames that are clearly system frames
// labelled as `in_app`.
// This is likely because RN uses static libraries which are bundled into the app binary.
// When symbolicated they are marked in_app.
//
// For this reason, for react-native app (p.Platform != f.Platform), we skip the f.InApp!=nil
// check as this field would be highly unreliable, and rely on our rules instead
if f.InApp != nil && (p == f.Platform) {
return
}
var isApplication bool
switch f.Platform {
case platform.Node:
isApplication = f.IsNodeApplicationFrame()
case platform.JavaScript:
isApplication = f.IsJavaScriptApplicationFrame()
case platform.Cocoa:
isApplication = f.IsCocoaApplicationFrame()
case platform.Rust:
isApplication = f.IsRustApplicationFrame()
case platform.Python:
isApplication = f.IsPythonApplicationFrame()
case platform.PHP:
isApplication = f.IsPHPApplicationFrame()
}
f.InApp = &isApplication
}

func (f *Frame) IsInApp() bool {
if f.InApp == nil {
return false
}
return *f.InApp
}

func (f *Frame) SetPlatform(p platform.Platform) {
if f.Platform == "" {
f.Platform = p
}
}

func (f *Frame) SetStatus() {
if f.Data.SymbolicatorStatus != "" {
f.Status = f.Data.SymbolicatorStatus
}
}

func (f *Frame) Normalize(p platform.Platform) {
// Call order is important since SetInApp uses Status and Platform
f.SetStatus()
f.SetPlatform(p)
f.SetInApp(p)
}
54 changes: 6 additions & 48 deletions internal/sample/sample.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ func (p *Profile) Speedscope() (speedscope.Output, error) {
// it alone for now
Image: fr.ModuleOrPackage(),
Inline: fr.IsInline(),
IsApplication: p.IsApplicationFrame(fr),
IsApplication: fr.IsInApp(),
Line: fr.Line,
Name: symbolName,
Path: fr.Path,
Expand Down Expand Up @@ -402,35 +402,6 @@ func (p *Profile) Speedscope() (speedscope.Output, error) {
}, nil
}

func (p *Profile) IsApplicationFrame(f frame.Frame) bool {
// for react-native the in_app field seems to be messed up most of the times,
// with system libraries and other frames that are clearly system frames
// labelled as `in_app`.
// This is likely because RN uses static libraries which are bundled into the app binary.
// When symbolicated they are marked in_app.
//
// For this reason, for react-native app (p.Platform != f.Platform), we skip the f.InApp!=nil
// check as this field would be highly unreliable, and rely on our rules instead
if f.InApp != nil && (p.Platform == f.Platform) {
return *f.InApp
}
switch f.Platform {
case platform.Node:
return f.IsNodeApplicationFrame()
case platform.JavaScript:
return f.IsJavaScriptApplicationFrame()
case platform.Cocoa:
return f.IsCocoaApplicationFrame()
case platform.Rust:
return f.IsRustApplicationFrame()
case platform.Python:
return f.IsPythonApplicationFrame()
case platform.PHP:
return f.IsPHPApplicationFrame()
}
return true
}

func (p *Profile) Metadata() metadata.Metadata {
return metadata.Metadata{
Architecture: p.Device.Architecture,
Expand All @@ -452,7 +423,11 @@ func (p *Profile) Metadata() metadata.Metadata {
}

func (p *Profile) Normalize() {
p.normalizeFrames()
for i := range p.Trace.Frames {
f := p.Trace.Frames[i]
f.Normalize(p.Platform)
p.Trace.Frames[i] = f
}

if p.Platform == platform.Cocoa {
p.Trace.trimCocoaStacks()
Expand Down Expand Up @@ -594,23 +569,6 @@ func (t Trace) CollectFrames(stackID int) []frame.Frame {
return frames
}

func (p *Profile) normalizeFrames() {
for i := range p.Trace.Frames {
f := p.Trace.Frames[i]

// Set if frame is in application
inApp := p.IsApplicationFrame(f)
f.InApp = &inApp

// Set Symbolicator status
if f.Status != "" {
f.Data.SymbolicatorStatus = f.Status
}

p.Trace.Frames[i] = f
}
}

func (p *RawProfile) moveTransaction() {
if len(p.Transactions) > 0 {
p.Transaction = p.Transactions[0]
Expand Down
Loading

0 comments on commit f0ea408

Please sign in to comment.