Track when we skip merging due to DoNotMerge (#3938)

Add some more nuanced tracking that takes into account the DoNotMerge flag and the New state of collections.

---

#### Does this PR need a docs update or release note?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No

#### Type of change

- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [x] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #3929

#### Test Plan

- [x] 💪 Manual
- [ ]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-07-31 15:36:36 -07:00 committed by GitHub
parent 5b455bfc4a
commit 2596fb9104
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -897,6 +897,8 @@ func traverseBaseDir(
currentPath = currentPath.Append(dirName) currentPath = currentPath.Append(dirName)
} }
var explicitMention bool
if upb, ok := updatedPaths[oldDirPath.String()]; ok { if upb, ok := updatedPaths[oldDirPath.String()]; ok {
// This directory was deleted. // This directory was deleted.
if upb == nil { if upb == nil {
@ -908,21 +910,16 @@ func traverseBaseDir(
// unchanged) location is in upb. // unchanged) location is in upb.
currentPath = upb.ToBuilder() currentPath = upb.ToBuilder()
if oldDirPath.String() == currentPath.String() { // Below we check if the collection was marked as new or DoNotMerge which
stats.Inc(statNoMove) // disables merging behavior. That means we can't directly update stats
} else { // here else we'll miss delta token refreshes and whatnot. Instead note
stats.Inc(statMove) // that we did see the path explicitly so it's not counted as a recursive
} // operation.
explicitMention = true
} }
} else { } else if currentPath == nil {
// Just stats tracking stuff. // Just stats tracking stuff.
if currentPath == nil { stats.Inc(statRecursiveDel)
stats.Inc(statRecursiveDel)
} else if oldDirPath.String() == currentPath.String() {
stats.Inc(statNoMove)
} else {
stats.Inc(statRecursiveMove)
}
} }
ctx = clues.Add(ctx, "new_path", currentPath) ctx = clues.Add(ctx, "new_path", currentPath)
@ -981,9 +978,20 @@ func traverseBaseDir(
// directories. The expected usecase for this is delta token expiry in M365. // directories. The expected usecase for this is delta token expiry in M365.
if node.collection != nil && if node.collection != nil &&
(node.collection.DoNotMergeItems() || node.collection.State() == data.NewState) { (node.collection.DoNotMergeItems() || node.collection.State() == data.NewState) {
stats.Inc(statSkipMerge)
return nil return nil
} }
// Just stats tracking stuff.
if oldDirPath.String() == currentPath.String() {
stats.Inc(statNoMove)
} else if explicitMention {
stats.Inc(statMove)
} else {
stats.Inc(statRecursiveMove)
}
curP, err := path.FromDataLayerPath(currentPath.String(), false) curP, err := path.FromDataLayerPath(currentPath.String(), false)
if err != nil { if err != nil {
return clues.New("converting current path to path.Path").WithClues(ctx) return clues.New("converting current path to path.Path").WithClues(ctx)
@ -1037,6 +1045,9 @@ const (
// statRecursiveDel denotes a directory that was deleted because one or more // statRecursiveDel denotes a directory that was deleted because one or more
// of its ancestors was deleted and it wasn't explicitly mentioned. // of its ancestors was deleted and it wasn't explicitly mentioned.
statRecursiveDel = "directories_recursively_deleted" statRecursiveDel = "directories_recursively_deleted"
// statSkipMerge denotes the number of directories that weren't merged because
// they were marked either DoNotMerge or New.
statSkipMerge = "directories_skipped_merging"
) )
func inflateBaseTree( func inflateBaseTree(
@ -1130,7 +1141,8 @@ func inflateBaseTree(
statMove, stats.Get(statMove), statMove, stats.Get(statMove),
statRecursiveMove, stats.Get(statRecursiveMove), statRecursiveMove, stats.Get(statRecursiveMove),
statDel, stats.Get(statDel), statDel, stats.Get(statDel),
statRecursiveDel, stats.Get(statRecursiveDel)) statRecursiveDel, stats.Get(statRecursiveDel),
statSkipMerge, stats.Get(statSkipMerge))
} }
return nil return nil