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

Bug 1646706 - Manage log_pings from core and add docs about debugging through env vars #1058

Merged
merged 10 commits into from
Jul 15, 2020
Merged

Bug 1646706 - Manage log_pings from core and add docs about debugging through env vars #1058

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3456803
Add functionality to manage log_pings from the core
brizental Jul 10, 2020
73f191a
Allow the core to manage log_pings in Kotlin
brizental Jul 10, 2020
53727f3
Allow the core to manage log_ping in Swift
brizental Jul 10, 2020
28b0fc0
Allow the core to manage log_ping in C#
brizental Jul 10, 2020
9e8f1b2
Allow the core to manage log_ping in Python
brizental Jul 10, 2020
b429d78
Add some more logging to the debug module
brizental Jul 13, 2020
cc8db05
Add docs about debug features through env vars
brizental Jul 14, 2020
18f80af
Set different env vars for each test in debug.rs
brizental Jul 14, 2020
b509811
Attend to review comments
brizental Jul 15, 2020
bf10bf1
Add CHANGELOG.md entry
brizental Jul 15, 2020
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
25 changes: 0 additions & 25 deletions glean-core/ios/Glean/Config/Configuration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,11 @@
/// property for calculating the `FfiConfiguration`
public struct Configuration {
brizental marked this conversation as resolved.
Show resolved Hide resolved
let serverEndpoint: String
var logPings: Bool
let maxEvents: Int32?
let channel: String?

struct Constants {
static let defaultTelemetryEndpoint = "https://incoming.telemetry.mozilla.org"
static let defaultLogPings = false
}

/// This init is for internal testing setup only.
///
/// - parameters:
/// * serverEndpoint: A `String` representing the server the pings are sent to.
/// This should only be changed for tests.
/// * logPings: whether to log ping contents to the console.
/// This is only meant to be used internally by the `GleanDebugActivity`.
/// * maxEvents: the number of events to store before the events ping is sent.
/// * channel: the release channel the application is on, if known.
/// This will be sent along with all the pings, in the `client_info` section.
internal init(
serverEndpoint: String = Constants.defaultTelemetryEndpoint,
logPings: Bool = Constants.defaultLogPings,
maxEvents: Int32? = nil,
channel: String? = nil
) {
self.serverEndpoint = serverEndpoint
self.logPings = logPings
self.maxEvents = maxEvents
self.channel = channel
}

/// Create a new Glean `Configuration` object
Expand All @@ -49,7 +25,6 @@ public struct Configuration {
serverEndpoint: String? = nil
) {
self.serverEndpoint = serverEndpoint ?? Constants.defaultTelemetryEndpoint
self.logPings = Constants.defaultLogPings
self.maxEvents = maxEvents
self.channel = channel
}
Expand Down
4 changes: 2 additions & 2 deletions glean-core/ios/Glean/Debug/GleanDebugTools.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class GleanDebugUtility {
///
/// There are 3 available commands that you can use with the Glean SDK debug tools
///
/// - `logPings`: If "true", will cause pings that are submitted to also be echoed to the device's log
/// - `logPings`: If "true", will cause pings that are submitted successfully to also be echoed to the device's log
/// - `tagPings`: This command expects a string to tag the pings with and redirects them to the Glean Debug View
/// - `sendPing`: This command expects a string name of a ping to force immediate collection and submission of.
///
Expand Down Expand Up @@ -63,7 +63,7 @@ class GleanDebugUtility {
}

if let logPings = parsedCommands.logPings {
Glean.shared.configuration?.logPings = logPings
Glean.shared.setLogPings(logPings)
brizental marked this conversation as resolved.
Show resolved Hide resolved
logger.debug("Log pings set to: \(logPings)")
}

Expand Down
21 changes: 20 additions & 1 deletion glean-core/ios/Glean/Glean.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class Glean {
var initialized: Bool = false
private var uploadEnabled: Bool = true
private var debugViewTag: String?
var logPings: Bool?
brizental marked this conversation as resolved.
Show resolved Hide resolved
brizental marked this conversation as resolved.
Show resolved Hide resolved
var configuration: Configuration?
private var observer: GleanLifecycleObserver?

Expand Down Expand Up @@ -119,6 +120,10 @@ public class Glean {
_ = self.setDebugViewTag(self.debugViewTag!)
}

if self.logPings != nil {
_ = self.setLogPings(self.logPings!)
brizental marked this conversation as resolved.
Show resolved Hide resolved
}

// If any pings were registered before initializing, do so now
for ping in self.pingTypeQueue {
self.registerPingType(ping)
Expand Down Expand Up @@ -465,6 +470,19 @@ public class Glean {
}
}

/// Set the log_pings debug option,
/// when this option is `true` the pings that are successfully submitted get logged.
///
/// - parameters:
/// * value: The value of the option.
public func setLogPings(_ value: Bool) {
brizental marked this conversation as resolved.
Show resolved Hide resolved
if self.isInitialized() {
glean_set_log_pings(value.toByte())
travis79 marked this conversation as resolved.
Show resolved Hide resolved
} else {
logPings = value
}
}

/// When applications are launched using the custom URL scheme, this helper function will process
/// the URL and parse the debug commands
///
Expand All @@ -473,7 +491,6 @@ public class Glean {
///
/// There are 3 available commands that you can use with the Glean SDK debug tools
///
/// - `logPings`: If "true", will cause pings that are submitted to also be echoed to the device's log
/// - `tagPings`: This command expects a string to tag the pings with and redirects them to the Glean Debug View
/// - `sendPing`: This command expects a string name of a ping to force immediate collection and submission of.
///
Expand Down Expand Up @@ -570,5 +587,7 @@ public class Glean {
// Init Glean.
testDestroyGleanHandle()
initialize(uploadEnabled: uploadEnabled, configuration: configuration)
// Enable ping logging for all tests
setLogPings(true)
brizental marked this conversation as resolved.
Show resolved Hide resolved
}
}
2 changes: 1 addition & 1 deletion glean-core/ios/Glean/Net/HttpPingUploader.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public class HttpPingUploader {
var uploadFailures = 0
while uploadFailures < Constants.maxRetries {
var incomingTask = FfiPingUploadTask()
glean_get_upload_task(&incomingTask, config.logPings.toByte())
glean_get_upload_task(&incomingTask)
let task = incomingTask.toPingUploadTask()

switch task {
Expand Down
5 changes: 0 additions & 5 deletions glean-core/ios/GleanTests/Config/ConfigurationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ class ConfigurationTests: XCTestCase {
Configuration.Constants.defaultTelemetryEndpoint,
"Default endpoint is set"
)
XCTAssertEqual(
config?.logPings,
Configuration.Constants.defaultLogPings,
"Default log pings is set"
)
XCTAssertNil(
config?.maxEvents,
"Default max events are set"
Expand Down
23 changes: 11 additions & 12 deletions glean-core/ios/GleanTests/Debug/GleanDebugUtilityTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,37 @@ class GleanDebugUtilityTests: XCTestCase {
}

func testHandleCustomUrlLogPings() {
// Test initial state
XCTAssertFalse(Glean.shared.configuration!.logPings)
// We destroy the Glean handle so that Glean in in an unitialized state,
// this way it will save the value of `logPings` in the `logPings` prop.
Glean.shared.testDestroyGleanHandle()

// Test toggle true
var url = URL(string: "test://glean?logPings=true")
Glean.shared.handleCustomUrl(url: url!)
XCTAssertTrue(Glean.shared.configuration!.logPings)
XCTAssertTrue(Glean.shared.logPings!)
brizental marked this conversation as resolved.
Show resolved Hide resolved

// Test invalid value doesn't cause setting to toggle
var previousValue = Glean.shared.configuration?.logPings
var previousValue = Glean.shared.logPings
url = URL(string: "test://glean?logPings=Not-a-bool")
Glean.shared.handleCustomUrl(url: url!)
XCTAssertEqual(previousValue, Glean.shared.configuration!.logPings)
XCTAssertEqual(previousValue, Glean.shared.logPings)

// Test toggle false
url = URL(string: "test://glean?logPings=false")
Glean.shared.handleCustomUrl(url: url!)
XCTAssertFalse(Glean.shared.configuration!.logPings)
XCTAssertFalse(Glean.shared.logPings!)

// Test invalid value doesn't cause setting to toggle
previousValue = Glean.shared.configuration?.logPings
previousValue = Glean.shared.logPings
url = URL(string: "test://glean?logPings=Not-a-bool")
Glean.shared.handleCustomUrl(url: url!)
XCTAssertEqual(previousValue, Glean.shared.configuration!.logPings)
}
XCTAssertEqual(previousValue, Glean.shared.logPings)

func testHandleCustomUrlWrongHost() {
// This should NOT set the logPings to true or false because it doesn't
// match the required host "glean".
let url = URL(string: "test://not-glean?logPings=true")
url = URL(string: "test://not-glean?logPings=true")
brizental marked this conversation as resolved.
Show resolved Hide resolved
Glean.shared.handleCustomUrl(url: url!)
XCTAssertEqual(false, Glean.shared.configuration?.logPings)
XCTAssertEqual(previousValue, Glean.shared.logPings)
}

func testHandleCustomUrlSendPing() {
Expand Down
7 changes: 2 additions & 5 deletions glean-core/ios/GleanTests/Net/HttpPingUploaderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ class HttpPingUploaderTests: XCTestCase {
var expectation: XCTestExpectation?
private let testPath = "/some/random/path/not/important"
private let testPing = "{ \"ping\": \"test\" }"
private let testConfig = Configuration(
logPings: true
)

override func tearDown() {
// Reset expectations
Expand All @@ -30,7 +27,7 @@ class HttpPingUploaderTests: XCTestCase {

expectation = expectation(description: "Completed upload")

let httpPingUploader = HttpPingUploader(configuration: testConfig)
let httpPingUploader = HttpPingUploader(configuration: Configuration())
httpPingUploader.upload(path: testPath, data: Data(testPing.utf8), headers: [:]) { result in
testValue = result
self.expectation?.fulfill()
Expand All @@ -51,7 +48,7 @@ class HttpPingUploaderTests: XCTestCase {
// Build a request.
// We specify a single additional header here.
// In usual code they are generated on the Rust side.
let request = HttpPingUploader(configuration: testConfig)
let request = HttpPingUploader(configuration: Configuration())
.buildRequest(path: testPath, data: Data(testPing.utf8), headers: ["X-Client-Type": "Glean"])

XCTAssertEqual(
Expand Down