From 3ee1a8d1661f6254cf649534e71a2560067af490 Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Mon, 4 Dec 2023 18:18:13 -0800 Subject: [PATCH] Minor url cache logging & counting improvements (#4801) 1. Move cache miss logs to debug, as it can be chatty sometimes. We will leverage the end of operation `URLCacheMiss` counter to get this information instead. 2. Counter renaming. --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Issue(s) * # #### Test Plan - [x] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/m365/collection/drive/collection.go | 11 +++++------ src/internal/m365/collection/drive/url_cache.go | 4 ++-- src/pkg/count/keys.go | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/internal/m365/collection/drive/collection.go b/src/internal/m365/collection/drive/collection.go index cc6f75751..bc912c8dd 100644 --- a/src/internal/m365/collection/drive/collection.go +++ b/src/internal/m365/collection/drive/collection.go @@ -379,11 +379,10 @@ func downloadContent( return content, nil } - // Consider cache errors(including deleted items) as cache misses. This is - // to preserve existing behavior. Fallback to refetching the item using the - // API. - logger.CtxErr(ctx, err).Info("url cache miss: refetching from API") - counter.Inc(count.ItemDownloadURLRefetch) + // Consider cache errors(including deleted items) as cache misses. + // Fallback to refetching the item using the graph API. + logger.CtxErr(ctx, err).Debug("url cache miss: refetching from API") + counter.Inc(count.URLCacheMiss) di, err := iaag.GetItem(ctx, driveID, ptr.Val(item.GetId())) if err != nil { @@ -423,7 +422,7 @@ func readItemContents( rc, err := downloadFile(ctx, iaag, props.downloadURL) if graph.IsErrUnauthorizedOrBadToken(err) { - logger.CtxErr(ctx, err).Info("stale item in cache") + logger.CtxErr(ctx, err).Debug("stale item in cache") } if err != nil { diff --git a/src/internal/m365/collection/drive/url_cache.go b/src/internal/m365/collection/drive/url_cache.go index ba082ac9c..e89099823 100644 --- a/src/internal/m365/collection/drive/url_cache.go +++ b/src/internal/m365/collection/drive/url_cache.go @@ -35,7 +35,7 @@ type itemProps struct { var _ getItemPropertyer = &urlCache{} -// urlCache caches download URLs for drive items +// urlCache caches download URLs for drive file items type urlCache struct { driveID string prevDelta string @@ -207,7 +207,7 @@ func (uc *urlCache) readCache( props, ok := uc.idToProps[itemID] if !ok { - uc.counter.Inc(count.URLCacheMiss) + uc.counter.Inc(count.URLCacheItemNotFound) return itemProps{}, clues.NewWC(ctx, "item not found in cache") } diff --git a/src/pkg/count/keys.go b/src/pkg/count/keys.go index 93c5c4347..1896a7db6 100644 --- a/src/pkg/count/keys.go +++ b/src/pkg/count/keys.go @@ -38,7 +38,6 @@ const ( DriveTombstones Key = "drive-tombstones" Files Key = "files" Folders Key = "folders" - ItemDownloadURLRefetch Key = "item-download-url-refetch" ItemsAdded Key = "items-added" ItemsRemoved Key = "items-removed" LazyDeletedInFlight Key = "lazy-deleted-in-flight" @@ -68,6 +67,7 @@ const ( TotalContainersSkipped Key = "total-containers-skipped" URLCacheMiss Key = "url-cache-miss" URLCacheRefresh Key = "url-cache-refresh" + URLCacheItemNotFound Key = "url-cache-item-not-found" ) // Total___Processed counts are used to track raw processing numbers