From 8f30fd954684e8a2d906090328f910e2985a0c8c Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 6 Nov 2024 13:40:13 -0500 Subject: [PATCH 1/4] feat(statistical-detectors): Handle profile chunks in regressed endpoint In order to support function regressions on profile chunks, we need to change from passing a simple profile id to an example metadata so we can load the chunk containing the function. --- cmd/vroom/regressed.go | 2 +- internal/chunk/android.go | 11 ++ internal/chunk/chunk.go | 2 + internal/chunk/sample.go | 9 ++ internal/frame/frame.go | 3 + internal/occurrence/regressed_frame.go | 180 +++++++++++-------------- internal/profile/android.go | 11 ++ internal/profile/legacy.go | 4 + internal/profile/profile.go | 6 + internal/profile/trace.go | 2 + internal/sample/sample.go | 9 ++ 11 files changed, 139 insertions(+), 100 deletions(-) diff --git a/cmd/vroom/regressed.go b/cmd/vroom/regressed.go index 2c8ecbf..8f648fb 100644 --- a/cmd/vroom/regressed.go +++ b/cmd/vroom/regressed.go @@ -25,7 +25,7 @@ func (env *environment) postRegressed(w http.ResponseWriter, r *http.Request) { for _, regressedFunction := range regressedFunctions { s := sentry.StartSpan(ctx, "processing") s.Description = "Generating occurrence for payload" - occurrence, err := occurrence.ProcessRegressedFunction(ctx, env.storage, regressedFunction) + occurrence, err := occurrence.ProcessRegressedFunction(ctx, env.storage, regressedFunction, readJobs) s.Finish() if err != nil { hub.CaptureException(err) diff --git a/internal/chunk/android.go b/internal/chunk/android.go index 3761d99..a2da10b 100644 --- a/internal/chunk/android.go +++ b/internal/chunk/android.go @@ -6,6 +6,7 @@ import ( "github.com/getsentry/vroom/internal/clientsdk" "github.com/getsentry/vroom/internal/debugmeta" + "github.com/getsentry/vroom/internal/frame" "github.com/getsentry/vroom/internal/nodetree" "github.com/getsentry/vroom/internal/platform" "github.com/getsentry/vroom/internal/profile" @@ -112,5 +113,15 @@ func (c AndroidChunk) GetOptions() utils.Options { return c.Options } +func (c AndroidChunk) GetFrameWithFingerprint(target uint32) (frame.Frame, error) { + for _, m := range c.Profile.Methods { + f := m.Frame() + if f.Fingerprint() == target { + return f, nil + } + } + return frame.Frame{}, frame.ErrFrameNotFound +} + func (c *AndroidChunk) Normalize() { } diff --git a/internal/chunk/chunk.go b/internal/chunk/chunk.go index c82323c..cc715c8 100644 --- a/internal/chunk/chunk.go +++ b/internal/chunk/chunk.go @@ -3,6 +3,7 @@ package chunk import ( "fmt" + "github.com/getsentry/vroom/internal/frame" "github.com/getsentry/vroom/internal/platform" "github.com/getsentry/vroom/internal/utils" ) @@ -19,6 +20,7 @@ type ( GetRelease() string GetRetentionDays() int GetOptions() utils.Options + GetFrameWithFingerprint(uint32) (frame.Frame, error) DurationMS() uint64 EndTimestamp() float64 diff --git a/internal/chunk/sample.go b/internal/chunk/sample.go index 37062c4..d629672 100644 --- a/internal/chunk/sample.go +++ b/internal/chunk/sample.go @@ -252,3 +252,12 @@ func (c SampleChunk) GetOrganizationID() uint64 { func (c SampleChunk) GetOptions() utils.Options { return c.Options } + +func (c SampleChunk) GetFrameWithFingerprint(target uint32) (frame.Frame, error) { + for _, f := range c.Profile.Frames { + if f.Fingerprint() == target { + return f, nil + } + } + return frame.Frame{}, frame.ErrFrameNotFound +} diff --git a/internal/frame/frame.go b/internal/frame/frame.go index 7a889f4..72fc1b7 100644 --- a/internal/frame/frame.go +++ b/internal/frame/frame.go @@ -3,6 +3,7 @@ package frame import ( "crypto/md5" "encoding/hex" + "errors" "fmt" "hash" "hash/fnv" @@ -21,6 +22,8 @@ var ( "Sentry": {}, "hermes": {}, } + + ErrFrameNotFound = errors.New("Unable to find matching frame") ) type ( diff --git a/internal/occurrence/regressed_frame.go b/internal/occurrence/regressed_frame.go index d68857f..14ca4de 100644 --- a/internal/occurrence/regressed_frame.go +++ b/internal/occurrence/regressed_frame.go @@ -3,132 +3,114 @@ package occurrence import ( "context" "errors" - "sync" "github.com/getsentry/sentry-go" - "github.com/getsentry/vroom/internal/nodetree" + "github.com/getsentry/vroom/internal/chunk" + "github.com/getsentry/vroom/internal/frame" + "github.com/getsentry/vroom/internal/platform" "github.com/getsentry/vroom/internal/profile" "github.com/getsentry/vroom/internal/storageutil" + "github.com/getsentry/vroom/internal/utils" "gocloud.dev/blob" ) type RegressedFunction struct { - OrganizationID uint64 `json:"organization_id"` - ProjectID uint64 `json:"project_id"` - ProfileID string `json:"profile_id"` - Fingerprint uint32 `json:"fingerprint"` - AbsolutePercentageChange float64 `json:"absolute_percentage_change"` - AggregateRange1 float64 `json:"aggregate_range_1"` - AggregateRange2 float64 `json:"aggregate_range_2"` - Breakpoint uint64 `json:"breakpoint"` - TrendDifference float64 `json:"trend_difference"` - TrendPercentage float64 `json:"trend_percentage"` - UnweightedPValue float64 `json:"unweighted_p_value"` - UnweightedTValue float64 `json:"unweighted_t_value"` + OrganizationID uint64 `json:"organization_id"` + ProjectID uint64 `json:"project_id"` + ProfileID string `json:"profile_id"` + Example utils.ExampleMetadata `json:"example"` + Fingerprint uint32 `json:"fingerprint"` + AbsolutePercentageChange float64 `json:"absolute_percentage_change"` + AggregateRange1 float64 `json:"aggregate_range_1"` + AggregateRange2 float64 `json:"aggregate_range_2"` + Breakpoint uint64 `json:"breakpoint"` + TrendDifference float64 `json:"trend_difference"` + TrendPercentage float64 `json:"trend_percentage"` + UnweightedPValue float64 `json:"unweighted_p_value"` + UnweightedTValue float64 `json:"unweighted_t_value"` } func ProcessRegressedFunction( ctx context.Context, profilesBucket *blob.Bucket, regressedFunction RegressedFunction, + jobs chan storageutil.ReadJob, ) (*Occurrence, error) { - s := sentry.StartSpan(ctx, "profile.read") - s.Description = "Read profile from GCS" - var p profile.Profile - objectName := profile.StoragePath( - regressedFunction.OrganizationID, - regressedFunction.ProjectID, - regressedFunction.ProfileID, - ) - err := storageutil.UnmarshalCompressed(ctx, profilesBucket, objectName, &p) - s.Finish() - if err != nil { - return nil, err + results := make(chan storageutil.ReadJobResult, 1) + defer close(results) + + if regressedFunction.ProfileID != "" { + // For back compat, we should be use the example moving forwards + jobs <- profile.ReadJob{ + Ctx: ctx, + OrganizationID: regressedFunction.OrganizationID, + ProjectID: regressedFunction.ProjectID, + ProfileID: regressedFunction.ProfileID, + Storage: profilesBucket, + Result: results, + } + } else if regressedFunction.Example.ProfileID != "" { + jobs <- profile.ReadJob{ + Ctx: ctx, + OrganizationID: regressedFunction.OrganizationID, + ProjectID: regressedFunction.ProjectID, + ProfileID: regressedFunction.Example.ProfileID, + Storage: profilesBucket, + Result: results, + } + } else { + jobs <- chunk.ReadJob{ + Ctx: ctx, + OrganizationID: regressedFunction.OrganizationID, + ProjectID: regressedFunction.ProjectID, + ProfilerID: regressedFunction.Example.ProfilerID, + ChunkID: regressedFunction.Example.ChunkID, + Storage: profilesBucket, + Result: results, + } } - s = sentry.StartSpan(ctx, "processing") - s.Description = "Generate call trees" - calltreesByTID, err := p.CallTrees() - s.Finish() - + res := <-results + platform, frame, err := getPlatformAndFrame(ctx, res, regressedFunction.Fingerprint) if err != nil { return nil, err } - - calltrees, exists := calltreesByTID[p.Transaction().ActiveThreadID] - if !exists { - return nil, errors.New("calltree not found") - } - - s = sentry.StartSpan(ctx, "processing") - s.Description = "Searching for fingerprint" - var node *nodetree.Node - for _, calltree := range calltrees { - node = calltree.FindNodeByFingerprint(regressedFunction.Fingerprint) - if node != nil { - break - } - } - s.Finish() - - if node == nil { - return nil, errors.New("fingerprint not found") - } - - return FromRegressedFunction(p.Platform(), regressedFunction, node.Frame), nil + return FromRegressedFunction(platform, regressedFunction, frame), nil } -func ProcessRegressedFunctions( +func getPlatformAndFrame( ctx context.Context, - hub *sentry.Hub, - profilesBucket *blob.Bucket, - regressedFunctions []RegressedFunction, - numWorkers int, -) []*Occurrence { - if len(regressedFunctions) < numWorkers { - numWorkers = len(regressedFunctions) - } - - var wg sync.WaitGroup - wg.Add(numWorkers) - - regressedChan := make(chan RegressedFunction, numWorkers) - occurrenceChan := make(chan *Occurrence) - - for i := 0; i < numWorkers; i++ { - go func() { - defer wg.Done() - for regressedFunction := range regressedChan { - occurrence, err := ProcessRegressedFunction(ctx, profilesBucket, regressedFunction) - if err != nil { - hub.CaptureException(err) - continue - } else if occurrence == nil { - continue - } + res storageutil.ReadJobResult, + target uint32, +) (platform.Platform, frame.Frame, error) { + var platform platform.Platform + var frame frame.Frame - occurrenceChan <- occurrence - } - }() + err := res.Error() + if err != nil { + return platform, frame, err } - go func() { - for _, regressedFunction := range regressedFunctions { - regressedChan <- regressedFunction - } - close(regressedChan) - - // wait until all the profiles have been processed - // then we can close the occurrence channel and collect - // any occurrences that have been created - wg.Wait() - close(occurrenceChan) - }() + s := sentry.StartSpan(ctx, "processing") + s.Description = "Searching for fingerprint" + defer s.Finish() - occurrences := []*Occurrence{} - for occurrence := range occurrenceChan { - occurrences = append(occurrences, occurrence) + if result, ok := res.(profile.ReadJobResult); ok { + platform = result.Profile.Platform() + frame, err = result.Profile.GetFrameWithFingerprint(target) + if err != nil { + return platform, frame, err + } + } else if result, ok := res.(chunk.ReadJobResult); ok { + platform = result.Chunk.GetPlatform() + frame, err = result.Chunk.GetFrameWithFingerprint(target) + if err != nil { + return platform, frame, err + } + } else { + // This should never happen + return platform, frame, errors.New("unexpected result from storage") } - return occurrences + return platform, frame, nil } diff --git a/internal/profile/android.go b/internal/profile/android.go index 0b23d77..d647e5f 100644 --- a/internal/profile/android.go +++ b/internal/profile/android.go @@ -597,3 +597,14 @@ func (p Android) ActiveThreadID() uint64 { } return 0 } + +func (p Android) GetFrameWithFingerprint(target uint32) (frame.Frame, error) { + for _, m := range p.Methods { + f := m.Frame() + if f.Fingerprint() == target { + return f, nil + } + } + // TODO: handle react native + return frame.Frame{}, frame.ErrFrameNotFound +} diff --git a/internal/profile/legacy.go b/internal/profile/legacy.go index 90e0c88..10938a6 100644 --- a/internal/profile/legacy.go +++ b/internal/profile/legacy.go @@ -363,6 +363,10 @@ func (p LegacyProfile) GetOptions() utils.Options { return p.Options } +func (p LegacyProfile) GetFrameWithFingerprint(target uint32) (frame.Frame, error) { + return p.Trace.GetFrameWithFingerprint(target) +} + // This is to be used for ReactNative JS profile only since it works based on the // assumption that we'll only have 1 thread in the JS profile, as is the case // for ReactNative. diff --git a/internal/profile/profile.go b/internal/profile/profile.go index a790a5a..b8d86f1 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -5,6 +5,7 @@ import ( "time" "github.com/getsentry/vroom/internal/debugmeta" + "github.com/getsentry/vroom/internal/frame" "github.com/getsentry/vroom/internal/measurements" "github.com/getsentry/vroom/internal/metadata" "github.com/getsentry/vroom/internal/nodetree" @@ -42,6 +43,7 @@ type ( IsSampled() bool SetProfileID(ID string) GetOptions() utils.Options + GetFrameWithFingerprint(uint32) (frame.Frame, error) } Profile struct { @@ -173,3 +175,7 @@ func (p *Profile) Measurements() map[string]measurements.Measurement { func (p *Profile) GetOptions() utils.Options { return p.profile.GetOptions() } + +func (p *Profile) GetFrameWithFingerprint(target uint32) (frame.Frame, error) { + return p.profile.GetFrameWithFingerprint(target) +} diff --git a/internal/profile/trace.go b/internal/profile/trace.go index 2c84103..a7c388b 100644 --- a/internal/profile/trace.go +++ b/internal/profile/trace.go @@ -1,6 +1,7 @@ package profile import ( + "github.com/getsentry/vroom/internal/frame" "github.com/getsentry/vroom/internal/nodetree" "github.com/getsentry/vroom/internal/speedscope" ) @@ -10,5 +11,6 @@ type ( ActiveThreadID() uint64 CallTrees() map[uint64][]*nodetree.Node Speedscope() (speedscope.Output, error) + GetFrameWithFingerprint(uint32) (frame.Frame, error) } ) diff --git a/internal/sample/sample.go b/internal/sample/sample.go index 69bcf0b..d1858dc 100644 --- a/internal/sample/sample.go +++ b/internal/sample/sample.go @@ -446,6 +446,15 @@ func (p *Profile) GetOptions() utils.Options { return p.Options } +func (p *Profile) GetFrameWithFingerprint(target uint32) (frame.Frame, error) { + for _, f := range p.Trace.Frames { + if f.Fingerprint() == target { + return f, nil + } + } + return frame.Frame{}, frame.ErrFrameNotFound +} + func (t Trace) SamplesByThreadD() ([]uint64, map[uint64][]*Sample) { samples := make(map[uint64][]*Sample) var threadIDs []uint64 From 4724d69624509901d43d68ea810b9637142aa61b Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 6 Nov 2024 13:42:21 -0500 Subject: [PATCH 2/4] remove another unused method --- internal/nodetree/nodetree.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/internal/nodetree/nodetree.go b/internal/nodetree/nodetree.go index 3d3f3e1..3a996ac 100644 --- a/internal/nodetree/nodetree.go +++ b/internal/nodetree/nodetree.go @@ -269,21 +269,6 @@ func (n *Node) Close(timestamp uint64) { } } -func (n *Node) FindNodeByFingerprint(target uint32) *Node { - if n.Frame.Fingerprint() == target { - return n - } - - for _, child := range n.Children { - node := child.FindNodeByFingerprint(target) - if node != nil { - return node - } - } - - return nil -} - func isSymbolicatedFrame(f frame.Frame) bool { // React-native case if f.Platform == platform.JavaScript && f.IsReactNative { From b846385ef85fba4d472e54dd3e0a0268205bb20b Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 6 Nov 2024 13:44:01 -0500 Subject: [PATCH 3/4] this is unused --- internal/occurrence/occurrence.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/occurrence/occurrence.go b/internal/occurrence/occurrence.go index f86f31f..5bd5085 100644 --- a/internal/occurrence/occurrence.go +++ b/internal/occurrence/occurrence.go @@ -237,7 +237,6 @@ func FromRegressedFunction( EvidenceData: map[string]interface{}{ "organization_id": regressed.OrganizationID, "project_id": regressed.ProjectID, - "profile_id": regressed.ProfileID, // frame info "file": f.File, From 79158b8b648230cb128d19721996ed756fc9c05f Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 6 Nov 2024 13:44:44 -0500 Subject: [PATCH 4/4] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d9fd6d..589efc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ - Forward SDK info for legacy profiles to Kafka. ([#515](https://github.com/getsentry/vroom/pull/515)) - Add more metadata fields to Chunk Kafka message. ([#518](https://github.com/getsentry/vroom/pull/518)) - Ingest Android profile chunks. ([#521](https://github.com/getsentry/vroom/pull/521)) +- Handle profile chunks in regressed endpoint. ([#527](https://github.com/getsentry/vroom/pull/527)) **Bug Fixes**: