From 30c86797d3e82acba5848f33b618a5109a8d6058 Mon Sep 17 00:00:00 2001 From: Keepers Date: Tue, 11 Apr 2023 18:43:55 -0600 Subject: [PATCH] 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_entry: No #### Type of change - [x] :bug: Bugfix #### Issue(s) * fixes #3019 #### Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- .../connector/exchange/service_functions.go | 10 +++- .../connector/exchange/service_iterators.go | 59 +++++++++++-------- src/internal/connector/graph/service.go | 8 +-- 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/src/internal/connector/exchange/service_functions.go b/src/internal/connector/exchange/service_functions.go index 5fd1a0845..286c1c4cf 100644 --- a/src/internal/connector/exchange/service_functions.go +++ b/src/internal/connector/exchange/service_functions.go @@ -10,6 +10,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/pkg/account" "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/selectors" ) @@ -92,14 +93,15 @@ func PopulateExchangeContainerResolver( // - the human-readable path using display names. // - true if the path passes the scope comparison. func includeContainer( + ctx context.Context, qp graph.QueryParams, c graph.CachedContainer, scope selectors.ExchangeScope, + category path.CategoryType, ) (path.Path, *path.Builder, bool) { var ( directory string locPath path.Path - category = scope.Category().PathType() pb = c.Path() loc = c.Location() ) @@ -154,5 +156,11 @@ func includeContainer( return nil, nil, false } + logger.Ctx(ctx).With( + "included", ok, + "scope", scope, + "matches_input", directory, + ).Debug("backup folder selection filter") + return pathRes, loc, ok } diff --git a/src/internal/connector/exchange/service_iterators.go b/src/internal/connector/exchange/service_iterators.go index 29551b21b..0c1f93dac 100644 --- a/src/internal/connector/exchange/service_iterators.go +++ b/src/internal/connector/exchange/service_iterators.go @@ -5,6 +5,7 @@ import ( "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/connector/exchange/api" "github.com/alcionai/corso/src/internal/connector/graph" @@ -48,6 +49,7 @@ func filterContainersAndFillCollections( // copy of previousPaths. any folder found in the resolver get // deleted from this map, leaving only the deleted folders behind tombstones = makeTombstones(dps) + category = qp.Category ) logger.Ctx(ctx).Infow("filling collections", "len_deltapaths", len(dps)) @@ -60,7 +62,7 @@ func filterContainersAndFillCollections( return err } - ibt, err := itemerByType(ac, scope.Category().PathType()) + ibt, err := itemerByType(ac, category) if err != nil { return err } @@ -75,28 +77,38 @@ func filterContainersAndFillCollections( cID := ptr.Val(c.GetId()) 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. if !ok { continue } - var ( - dp = dps[cID] - prevDelta = dp.delta - prevPathStr = dp.path - prevPath path.Path - ) - if len(prevPathStr) > 0 { 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. 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 !graph.IsErrDeletedInFlight(err) { el.AddRecoverable(clues.Stack(err).Label(fault.LabelForceNoBackupCreation)) @@ -113,6 +125,8 @@ func filterContainersAndFillCollections( if len(newDelta.URL) > 0 { deltaURLs[cID] = newDelta.URL + } else if !newDelta.Reset { + logger.Ctx(ictx).Info("missing delta url") } edc := NewCollection( @@ -120,7 +134,7 @@ func filterContainersAndFillCollections( currPath, prevPath, locPath, - scope.Category().PathType(), + category, ibt, statusUpdater, ctrlOpts, @@ -154,8 +168,10 @@ func filterContainersAndFillCollections( return el.Failure() } + ictx := clues.Add(ctx, "tombstone_id", id) + 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 } @@ -168,7 +184,7 @@ func filterContainersAndFillCollections( prevPath, err := pathFromPrevString(p) if err != nil { // 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 } @@ -177,7 +193,7 @@ func filterContainersAndFillCollections( nil, // marks the collection as deleted prevPath, nil, // tombstones don't need a location - scope.Category().PathType(), + category, ibt, statusUpdater, ctrlOpts, @@ -185,25 +201,20 @@ func filterContainersAndFillCollections( collections[id] = &edc } - entries := []graph.MetadataCollectionEntry{ - graph.NewMetadataEntry(graph.PreviousPathFileName, currPaths), - } - logger.Ctx(ctx).Infow( "adding metadata collection entries", "num_paths_entries", len(currPaths), "num_deltas_entries", len(deltaURLs)) - if len(deltaURLs) > 0 { - entries = append(entries, graph.NewMetadataEntry(graph.DeltaURLsFileName, deltaURLs)) - } - col, err := graph.MakeMetadataCollection( qp.Credentials.AzureTenantID, qp.ResourceOwner, path.ExchangeService, qp.Category, - entries, + []graph.MetadataCollectionEntry{ + graph.NewMetadataEntry(graph.PreviousPathFileName, currPaths), + graph.NewMetadataEntry(graph.DeltaURLsFileName, deltaURLs), + }, statusUpdater) if err != nil { return clues.Wrap(err, "making metadata collection") diff --git a/src/internal/connector/graph/service.go b/src/internal/connector/graph/service.go index ab2d890fc..33dda9fd0 100644 --- a/src/internal/connector/graph/service.go +++ b/src/internal/connector/graph/service.go @@ -275,7 +275,7 @@ type LoggingMiddleware struct{} // well-known path names used by graph api calls // used to un-hide path elements in a pii.SafeURL -var safePathParams = pii.MapWithPlurals( +var SafeURLPathParams = pii.MapWithPlurals( //nolint:misspell "alltime", "analytics", @@ -321,7 +321,7 @@ var safePathParams = pii.MapWithPlurals( // well-known safe query parameters used by graph api calls // // used to un-hide query params in a pii.SafeURL -var safeQueryParams = map[string]struct{}{ +var SafeURLQueryParams = map[string]struct{}{ "deltatoken": {}, "startdatetime": {}, "enddatetime": {}, @@ -335,8 +335,8 @@ var safeQueryParams = map[string]struct{}{ func LoggableURL(url string) pii.SafeURL { return pii.SafeURL{ URL: url, - SafePathElems: safePathParams, - SafeQueryKeys: safeQueryParams, + SafePathElems: SafeURLPathParams, + SafeQueryKeys: SafeURLQueryParams, } }