Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FXIOS-4785 [v106] Adds metrics to track number of history visits #11634

Merged
merged 1 commit into from
Aug 29, 2022
Merged

Add FXIOS-4785 [v106] Adds metrics to track number of history visits #11634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Carthage/
# Required for Glean
/Client/Generated
/Sync/Generated
/Storage/Generated
.venv/

ThirdParty/google-breakpad
Expand Down
35 changes: 35 additions & 0 deletions Client.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@
43F93C2627A86831009833D9 /* SDWebImage.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 4368F87B27966E810013419B /* SDWebImage.framework */; };
43F93C2827A8683E009833D9 /* RustMozillaAppServices.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 43BE578A278BA4D900491291 /* RustMozillaAppServices.framework */; };
43F93C2927A868FF009833D9 /* SDWebImage.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 4368F87B27966E810013419B /* SDWebImage.framework */; };
45CC573F28AD8B9B006D55AA /* Metrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45CC573E28AD8B9A006D55AA /* Metrics.swift */; };
4A59B58AD11B5EE1F80BBDEB /* TestHistory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A59BF410BBD9B3BE71F4C7C /* TestHistory.swift */; };
4F2A06BE26F8E46E0017DA05 /* TabCounterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F2A06BD26F8E46E0017DA05 /* TabCounterTests.swift */; };
4F514FD41ACD8F2C0022D7EA /* HistoryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F514FD31ACD8F2C0022D7EA /* HistoryTests.swift */; };
Expand Down Expand Up @@ -2444,6 +2445,8 @@
459C47419B3E2BD1BF4393F2 /* dsb */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = dsb; path = dsb.lproj/InfoPlist.strings; sourceTree = "<group>"; };
45AD4685A96876B6D2682E0F /* eu */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = eu; path = eu.lproj/FindInPage.strings; sourceTree = "<group>"; };
45CB4750BCEF0741D15058B0 /* de */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = de; path = de.lproj/Storage.strings; sourceTree = "<group>"; };
45CC573828AD8881006D55AA /* metrics.yaml */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.yaml; path = metrics.yaml; sourceTree = "<group>"; };
45CC573E28AD8B9A006D55AA /* Metrics.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Metrics.swift; sourceTree = "<group>"; };
45EE4AB495317A73B242DD2C /* sl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = sl; path = sl.lproj/Today.strings; sourceTree = "<group>"; };
45F244A7A34CE1404A014575 /* ur */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ur; path = ur.lproj/Intro.strings; sourceTree = "<group>"; };
46004CC8980B6CD80D910882 /* km */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = km; path = km.lproj/ClearPrivateDataConfirm.strings; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5474,6 +5477,8 @@
2FCAE21B1ABB51F800877008 /* Storage */ = {
isa = PBXGroup;
children = (
45CC573D28AD8B64006D55AA /* Generated */,
45CC573828AD8881006D55AA /* metrics.yaml */,
74B195431CF503FC007F36EF /* RecentlyClosedTabs.swift */,
2FCAE33D1ABB5F1800877008 /* Storage-Bridging-Header.h */,
D37DE2821CA2047500A5EC69 /* CertStore.swift */,
Expand Down Expand Up @@ -5806,6 +5811,14 @@
path = EnhancedTrackingProtection;
sourceTree = "<group>";
};
45CC573D28AD8B64006D55AA /* Generated */ = {
isa = PBXGroup;
children = (
45CC573E28AD8B9A006D55AA /* Metrics.swift */,
);
path = Generated;
sourceTree = "<group>";
};
5A271ABB2860B0BD00471CE4 /* WebServer */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -7947,6 +7960,7 @@
isa = PBXNativeTarget;
buildConfigurationList = 2FCAE2331ABB51F900877008 /* Build configuration list for PBXNativeTarget "Storage" */;
buildPhases = (
45CC573928AD89CB006D55AA /* Glean SDK Generator Script */,
2FCAE2151ABB51F800877008 /* Sources */,
2FCAE2161ABB51F800877008 /* Frameworks */,
);
Expand Down Expand Up @@ -9093,6 +9107,26 @@
shellPath = /bin/sh;
shellScript = "movedFrameworks=()\n cd \"${CODESIGNING_FOLDER_PATH}/Frameworks/\"\n for framework in *; do\n if [ -d \"$framework\" ]; then\n if [ -d \"${framework}/Frameworks\" ]; then\n echo \"Moving nested frameworks from ${framework}/Frameworks/ to ${PRODUCT_NAME}.app/Frameworks/\"\n \n cd \"${framework}/Frameworks/\"\n for nestedFramework in *; do\n echo \"- nested: ${nestedFramework}\"\n movedFrameworks+=(\"${nestedFramework}\")\n done\n cd ..\n cd ..\n \n cp -R \"${framework}/Frameworks/\" .\n rm -rf \"${framework}/Frameworks\"\n fi\n fi\n done\n \n if [ \"${CONFIGURATION}\" == \"Debug\" ] & [ \"${PLATFORM_NAME}\" != \"iphonesimulator\" ] ; then\n for movedFramework in \"${movedFrameworks[@]}\"\n do\n codesign --force --deep --sign \"${EXPANDED_CODE_SIGN_IDENTITY}\" --preserve-metadata=identifier,entitlements --timestamp=none \"${movedFramework}\"\n done\n else\n echo \"Info: CODESIGNING is only needed for Debug on device (will be re-signed anyway when archiving) \"\n fi\n";
};
45CC573928AD89CB006D55AA /* Glean SDK Generator Script */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
files = (
);
inputFileListPaths = (
);
inputPaths = (
"$(SRCROOT)/Storage/metrics.yaml",
);
name = "Glean SDK Generator Script";
outputFileListPaths = (
);
outputPaths = (
"$(SRCROOT)/Storage/Generated/Metrics.swift",
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "rm -rf .venv; bash $PWD/bin/sdk_generator.sh -g Glean -o $SRCROOT/Storage/Generated\n";
};
5FA2232B27F6FA00005B3D87 /* Glean SDK Generator Script */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
Expand Down Expand Up @@ -9419,6 +9453,7 @@
E6FF6ACA1D873CFF0070C294 /* PageMetadata.swift in Sources */,
2FCAE2611ABB531100877008 /* FileAccessor.swift in Sources */,
E677F0451D9423FB00ECF1FB /* SQLiteMetadata.swift in Sources */,
45CC573F28AD8B9B006D55AA /* Metrics.swift in Sources */,
D37DE2831CA2047500A5EC69 /* CertStore.swift in Sources */,
D018F93E1F44A71A0098F8CA /* Schema.swift in Sources */,
2FCAE2661ABB531100877008 /* Site.swift in Sources */,
Expand Down
30 changes: 30 additions & 0 deletions Storage/SQL/SQLiteHistory.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import Foundation
import Shared
import XCGLogger
import Glean

private let log = Logger.syncLogger
public let TopSiteCacheSize: Int32 = 16
Expand Down Expand Up @@ -144,6 +145,17 @@ open class SQLiteHistory {
self.favicons = SQLiteFavicons(db: self.db)
self.prefs = prefs
self.notificationCenter = notificationCenter

// We report the number of visits a user has
// this is helpful in determining what the size of users' history visits
// is like, to help guide testing the migration to the
// application-services implementation and testing the
// performance of the awesomebar.
self.countVisits { numVisits in
tarikeshaq marked this conversation as resolved.
Show resolved Hide resolved
if let numVisits = numVisits {
GleanMetrics.History.numVisits.set(Int64(numVisits))
}
}
}

public func getSites(forURLs urls: [String]) -> Deferred<Maybe<Cursor<Site?>>> {
Expand All @@ -157,6 +169,24 @@ open class SQLiteHistory {
let args: Args = []
return db.runQueryConcurrently(sql, args: args, factory: SQLiteHistory.iconHistoryColumnFactory)
}

public func countVisits(callback: @escaping (Int?) -> Void) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tarikeshaq is this a method we plan on keeping for long or the idea is to get rid of it once you have the required data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan is to keep the data for long so we can monitor the sizes of databases and compare this implementation and the application-services one. But we will be getting rid of this method when we swap to using application services history

That said, also with the application services history we will want to keep track of the number of visits, but that will probably happen in Rust as opposed to here in the app

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tarikeshaq good to merge, let me know if you have proper permission

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nbhasin2 thanks a ton!!! unfortunely I don't :( Is there a way I can get the permission to land a PR once it's approved?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged!!

let sql = "SELECT COUNT(*) FROM visits"
db.runQueryConcurrently(sql, args: nil, factory: SQLiteHistory.countAllVisitsFactory).uponQueue(.main) { result in
guard result.isSuccess else {
callback(nil)
return
}
// The result of a count query is only one row
if let res = result.successValue?.asArray().first {
if let res = res {
callback(res)
return
}
}
callback(nil)
}
}
}

private let topSitesQuery = """
Expand Down
4 changes: 4 additions & 0 deletions Storage/SQL/SQLiteHistoryFactories.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,8 @@ extension SQLiteHistory {
site.metadata = pageMetadataColumnFactory(row)
return site
}

class func countAllVisitsFactory(_ row: SDRow) -> Int? {
return row[0] as? Int
}
}
22 changes: 22 additions & 0 deletions Storage/metrics.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
---

$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0

# Metrics specific to the history storage
history:
num_visits:
type: quantity
unit: visit
description: >
The number of visits in a User's history database
bugs:
- https://mozilla-hub.atlassian.net/browse/SYNC-3260
data_reviews:
- TODO
notification_emails:
- [email protected]
- [email protected]
expires: never