always include delta metadata, additional logging (#3092)

Ensure exchange always includes the delta metadata entry, even if it has zero deltaURLs to record.

---

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

- [x]  No

#### Type of change

- [x] 🐛 Bugfix

#### Issue(s)

* fixes #3019

#### Test Plan

- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2023-04-11 18:43:55 -06:00 committed by GitHub
parent ab3011cab4
commit 30c86797d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 48 additions and 29 deletions

View File

@ -10,6 +10,7 @@ import (
"github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/logger"
"github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/selectors"
) )
@ -92,14 +93,15 @@ func PopulateExchangeContainerResolver(
// - the human-readable path using display names. // - the human-readable path using display names.
// - true if the path passes the scope comparison. // - true if the path passes the scope comparison.
func includeContainer( func includeContainer(
ctx context.Context,
qp graph.QueryParams, qp graph.QueryParams,
c graph.CachedContainer, c graph.CachedContainer,
scope selectors.ExchangeScope, scope selectors.ExchangeScope,
category path.CategoryType,
) (path.Path, *path.Builder, bool) { ) (path.Path, *path.Builder, bool) {
var ( var (
directory string directory string
locPath path.Path locPath path.Path
category = scope.Category().PathType()
pb = c.Path() pb = c.Path()
loc = c.Location() loc = c.Location()
) )
@ -154,5 +156,11 @@ func includeContainer(
return nil, nil, false return nil, nil, false
} }
logger.Ctx(ctx).With(
"included", ok,
"scope", scope,
"matches_input", directory,
).Debug("backup folder selection filter")
return pathRes, loc, ok return pathRes, loc, ok
} }

View File

@ -5,6 +5,7 @@ import (
"github.com/alcionai/clues" "github.com/alcionai/clues"
"github.com/alcionai/corso/src/internal/common/pii"
"github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/connector/exchange/api" "github.com/alcionai/corso/src/internal/connector/exchange/api"
"github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/graph"
@ -48,6 +49,7 @@ func filterContainersAndFillCollections(
// copy of previousPaths. any folder found in the resolver get // copy of previousPaths. any folder found in the resolver get
// deleted from this map, leaving only the deleted folders behind // deleted from this map, leaving only the deleted folders behind
tombstones = makeTombstones(dps) tombstones = makeTombstones(dps)
category = qp.Category
) )
logger.Ctx(ctx).Infow("filling collections", "len_deltapaths", len(dps)) logger.Ctx(ctx).Infow("filling collections", "len_deltapaths", len(dps))
@ -60,7 +62,7 @@ func filterContainersAndFillCollections(
return err return err
} }
ibt, err := itemerByType(ac, scope.Category().PathType()) ibt, err := itemerByType(ac, category)
if err != nil { if err != nil {
return err return err
} }
@ -75,28 +77,38 @@ func filterContainersAndFillCollections(
cID := ptr.Val(c.GetId()) cID := ptr.Val(c.GetId())
delete(tombstones, cID) delete(tombstones, cID)
currPath, locPath, ok := includeContainer(qp, c, scope) var (
dp = dps[cID]
prevDelta = dp.delta
prevPathStr = dp.path // do not log: pii; log prevPath instead
prevPath path.Path
ictx = clues.Add(
ctx,
"container_id", cID,
"previous_delta", pii.SafeURL{
URL: prevDelta,
SafePathElems: graph.SafeURLPathParams,
SafeQueryKeys: graph.SafeURLQueryParams,
})
)
currPath, locPath, ok := includeContainer(ictx, qp, c, scope, category)
// Only create a collection if the path matches the scope. // Only create a collection if the path matches the scope.
if !ok { if !ok {
continue continue
} }
var (
dp = dps[cID]
prevDelta = dp.delta
prevPathStr = dp.path
prevPath path.Path
)
if len(prevPathStr) > 0 { if len(prevPathStr) > 0 {
if prevPath, err = pathFromPrevString(prevPathStr); err != nil { if prevPath, err = pathFromPrevString(prevPathStr); err != nil {
logger.CtxErr(ctx, err).Error("parsing prev path") logger.CtxErr(ictx, err).Error("parsing prev path")
// if the previous path is unusable, then the delta must be, too. // if the previous path is unusable, then the delta must be, too.
prevDelta = "" prevDelta = ""
} }
} }
added, removed, newDelta, err := getter.GetAddedAndRemovedItemIDs(ctx, qp.ResourceOwner, cID, prevDelta) ictx = clues.Add(ictx, "previous_path", prevPath)
added, removed, newDelta, err := getter.GetAddedAndRemovedItemIDs(ictx, qp.ResourceOwner, cID, prevDelta)
if err != nil { if err != nil {
if !graph.IsErrDeletedInFlight(err) { if !graph.IsErrDeletedInFlight(err) {
el.AddRecoverable(clues.Stack(err).Label(fault.LabelForceNoBackupCreation)) el.AddRecoverable(clues.Stack(err).Label(fault.LabelForceNoBackupCreation))
@ -113,6 +125,8 @@ func filterContainersAndFillCollections(
if len(newDelta.URL) > 0 { if len(newDelta.URL) > 0 {
deltaURLs[cID] = newDelta.URL deltaURLs[cID] = newDelta.URL
} else if !newDelta.Reset {
logger.Ctx(ictx).Info("missing delta url")
} }
edc := NewCollection( edc := NewCollection(
@ -120,7 +134,7 @@ func filterContainersAndFillCollections(
currPath, currPath,
prevPath, prevPath,
locPath, locPath,
scope.Category().PathType(), category,
ibt, ibt,
statusUpdater, statusUpdater,
ctrlOpts, ctrlOpts,
@ -154,8 +168,10 @@ func filterContainersAndFillCollections(
return el.Failure() return el.Failure()
} }
ictx := clues.Add(ctx, "tombstone_id", id)
if collections[id] != nil { if collections[id] != nil {
el.AddRecoverable(clues.Wrap(err, "conflict: tombstone exists for a live collection").WithClues(ctx)) el.AddRecoverable(clues.Wrap(err, "conflict: tombstone exists for a live collection").WithClues(ictx))
continue continue
} }
@ -168,7 +184,7 @@ func filterContainersAndFillCollections(
prevPath, err := pathFromPrevString(p) prevPath, err := pathFromPrevString(p)
if err != nil { if err != nil {
// technically shouldn't ever happen. But just in case... // technically shouldn't ever happen. But just in case...
logger.CtxErr(ctx, err).Error("parsing tombstone prev path") logger.CtxErr(ictx, err).Error("parsing tombstone prev path")
continue continue
} }
@ -177,7 +193,7 @@ func filterContainersAndFillCollections(
nil, // marks the collection as deleted nil, // marks the collection as deleted
prevPath, prevPath,
nil, // tombstones don't need a location nil, // tombstones don't need a location
scope.Category().PathType(), category,
ibt, ibt,
statusUpdater, statusUpdater,
ctrlOpts, ctrlOpts,
@ -185,25 +201,20 @@ func filterContainersAndFillCollections(
collections[id] = &edc collections[id] = &edc
} }
entries := []graph.MetadataCollectionEntry{
graph.NewMetadataEntry(graph.PreviousPathFileName, currPaths),
}
logger.Ctx(ctx).Infow( logger.Ctx(ctx).Infow(
"adding metadata collection entries", "adding metadata collection entries",
"num_paths_entries", len(currPaths), "num_paths_entries", len(currPaths),
"num_deltas_entries", len(deltaURLs)) "num_deltas_entries", len(deltaURLs))
if len(deltaURLs) > 0 {
entries = append(entries, graph.NewMetadataEntry(graph.DeltaURLsFileName, deltaURLs))
}
col, err := graph.MakeMetadataCollection( col, err := graph.MakeMetadataCollection(
qp.Credentials.AzureTenantID, qp.Credentials.AzureTenantID,
qp.ResourceOwner, qp.ResourceOwner,
path.ExchangeService, path.ExchangeService,
qp.Category, qp.Category,
entries, []graph.MetadataCollectionEntry{
graph.NewMetadataEntry(graph.PreviousPathFileName, currPaths),
graph.NewMetadataEntry(graph.DeltaURLsFileName, deltaURLs),
},
statusUpdater) statusUpdater)
if err != nil { if err != nil {
return clues.Wrap(err, "making metadata collection") return clues.Wrap(err, "making metadata collection")

View File

@ -275,7 +275,7 @@ type LoggingMiddleware struct{}
// well-known path names used by graph api calls // well-known path names used by graph api calls
// used to un-hide path elements in a pii.SafeURL // used to un-hide path elements in a pii.SafeURL
var safePathParams = pii.MapWithPlurals( var SafeURLPathParams = pii.MapWithPlurals(
//nolint:misspell //nolint:misspell
"alltime", "alltime",
"analytics", "analytics",
@ -321,7 +321,7 @@ var safePathParams = pii.MapWithPlurals(
// well-known safe query parameters used by graph api calls // well-known safe query parameters used by graph api calls
// //
// used to un-hide query params in a pii.SafeURL // used to un-hide query params in a pii.SafeURL
var safeQueryParams = map[string]struct{}{ var SafeURLQueryParams = map[string]struct{}{
"deltatoken": {}, "deltatoken": {},
"startdatetime": {}, "startdatetime": {},
"enddatetime": {}, "enddatetime": {},
@ -335,8 +335,8 @@ var safeQueryParams = map[string]struct{}{
func LoggableURL(url string) pii.SafeURL { func LoggableURL(url string) pii.SafeURL {
return pii.SafeURL{ return pii.SafeURL{
URL: url, URL: url,
SafePathElems: safePathParams, SafePathElems: SafeURLPathParams,
SafeQueryKeys: safeQueryParams, SafeQueryKeys: SafeURLQueryParams,
} }
} }