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
+92 −0
Merged

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

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
Adds metrics to track number of history visits
Tarik Eshaq committed Aug 17, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
commit 10edc01b9e39f9fd118986ef46e9ff947a2e2a00
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
@@ -68,6 +68,7 @@ Carthage/
# Required for Glean
/Client/Generated
/Sync/Generated
/Storage/Generated
.venv/

ThirdParty/google-breakpad
35 changes: 35 additions & 0 deletions Client.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
@@ -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 */; };
@@ -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>"; };
@@ -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 */,
@@ -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 = (
@@ -7947,6 +7960,7 @@
isa = PBXNativeTarget;
buildConfigurationList = 2FCAE2331ABB51F900877008 /* Build configuration list for PBXNativeTarget "Storage" */;
buildPhases = (
45CC573928AD89CB006D55AA /* Glean SDK Generator Script */,
2FCAE2151ABB51F800877008 /* Sources */,
2FCAE2161ABB51F800877008 /* Frameworks */,
);
@@ -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;
@@ -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 */,
30 changes: 30 additions & 0 deletions Storage/SQL/SQLiteHistory.swift
View file
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
import Foundation
import Shared
import XCGLogger
import Glean

private let log = Logger.syncLogger
public let TopSiteCacheSize: Int32 = 16
@@ -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
if let numVisits = numVisits {
GleanMetrics.History.numVisits.set(Int64(numVisits))
}
}
}

public func getSites(forURLs urls: [String]) -> Deferred<Maybe<Cursor<Site?>>> {
@@ -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 = """
4 changes: 4 additions & 0 deletions Storage/SQL/SQLiteHistoryFactories.swift
View file
Original file line number Diff line number Diff line change
@@ -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
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