Skip to content

Commit

Permalink
[QL] Measure API Request Latency for PayPal Flows (#1292)
Browse files Browse the repository at this point in the history
* Send start_time, end_time, and endpoint to FPTI for tracking API request latency in the PayPal flow
* Add new BTCoreAnalytics enum
* Add new BTAPITimingDelegate for sending start/end timing from BTHTTP to BTAPIClient
* Update UnitTests

Co-authored-by: Sammy Cannillo <[email protected]>
  • Loading branch information
jaxdesmarais and scannillo authored May 15, 2024
1 parent d344976 commit 862455c
Show file tree
Hide file tree
Showing 14 changed files with 163 additions and 14 deletions.
18 changes: 15 additions & 3 deletions Braintree.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
80CD34012A604307009545F5 /* MockCardinalSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80CD33FD2A603892009545F5 /* MockCardinalSession.swift */; };
80CF988529DB64D400D51979 /* BTLocalPaymentError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80CF988429DB64D400D51979 /* BTLocalPaymentError.swift */; };
80D1638729E75CF1001D880E /* BTThreeDSecureClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80D1638529E75766001D880E /* BTThreeDSecureClient.swift */; };
80D280CD2BE9354B00762D27 /* Date+MillisecondTimestamp.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80D280CC2BE9354B00762D27 /* Date+MillisecondTimestamp.swift */; };
80E2460F29E492DF00945A1D /* BTLocalPaymentClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80E2460E29E492DF00945A1D /* BTLocalPaymentClient.swift */; };
80F86FF029FC2ED6003297FF /* Encodable+Dictionary.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80F86FEF29FC2ED6003297FF /* Encodable+Dictionary.swift */; };
80FF7D1A25881C03001C32EF /* BTThreeDSecureV2UICustomization_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80FF7D1925881C03001C32EF /* BTThreeDSecureV2UICustomization_Tests.swift */; };
Expand Down Expand Up @@ -304,6 +305,8 @@
BEF1089129019DB9003B2FDA /* BTAPIClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 570B93AF2853980C0041BAFE /* BTAPIClient.swift */; };
BEF177A62B51C7ED004F0F35 /* SEPADebitAccountsRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEF177A52B51C7ED004F0F35 /* SEPADebitAccountsRequest.swift */; };
BEF177A82B51D6A7004F0F35 /* SEPADebitRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEF177A72B51D6A7004F0F35 /* SEPADebitRequest.swift */; };
BEF388C12BE52CD2000965C8 /* BTCoreAnalytics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEF388C02BE52CD2000965C8 /* BTCoreAnalytics.swift */; };
BEF388C32BE9340C000965C8 /* BTAPITimingDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEF388C22BE9340C000965C8 /* BTAPITimingDelegate.swift */; };
BEF3F17E27CE9EDF00072467 /* BTSEPADirectDebitClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEF3F17127CE9E6C00072467 /* BTSEPADirectDebitClient.swift */; };
BEF5D2E6294A18B300FFD56D /* BTPayPalLineItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEF5D2E5294A18B300FFD56D /* BTPayPalLineItem.swift */; };
BEFD1AF4284F92130070076E /* PPRiskMagnes.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = 9C36BD4226B30AA600F0A559 /* PPRiskMagnes.xcframework */; };
Expand Down Expand Up @@ -748,6 +751,7 @@
80CD33FF2A6042FC009545F5 /* CardinalSessionTestable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CardinalSessionTestable.swift; sourceTree = "<group>"; };
80CF988429DB64D400D51979 /* BTLocalPaymentError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTLocalPaymentError.swift; sourceTree = "<group>"; };
80D1638529E75766001D880E /* BTThreeDSecureClient.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTThreeDSecureClient.swift; sourceTree = "<group>"; };
80D280CC2BE9354B00762D27 /* Date+MillisecondTimestamp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Date+MillisecondTimestamp.swift"; sourceTree = "<group>"; };
80DBE69423A931A600373230 /* Helpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Helpers.swift; sourceTree = "<group>"; };
80E2460E29E492DF00945A1D /* BTLocalPaymentClient.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTLocalPaymentClient.swift; sourceTree = "<group>"; };
80F86FEF29FC2ED6003297FF /* Encodable+Dictionary.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Encodable+Dictionary.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -930,6 +934,8 @@
BEEB565A2AE9B3030029F264 /* BTIntegrationTestsConstants.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTIntegrationTestsConstants.swift; sourceTree = "<group>"; };
BEF177A52B51C7ED004F0F35 /* SEPADebitAccountsRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SEPADebitAccountsRequest.swift; sourceTree = "<group>"; };
BEF177A72B51D6A7004F0F35 /* SEPADebitRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SEPADebitRequest.swift; sourceTree = "<group>"; };
BEF388C02BE52CD2000965C8 /* BTCoreAnalytics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTCoreAnalytics.swift; sourceTree = "<group>"; };
BEF388C22BE9340C000965C8 /* BTAPITimingDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTAPITimingDelegate.swift; sourceTree = "<group>"; };
BEF3F17127CE9E6C00072467 /* BTSEPADirectDebitClient.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTSEPADirectDebitClient.swift; sourceTree = "<group>"; };
BEF3F17C27CE9EC600072467 /* BraintreeSEPADirectDebit.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = BraintreeSEPADirectDebit.framework; sourceTree = BUILT_PRODUCTS_DIR; };
BEF5D2E5294A18B300FFD56D /* BTPayPalLineItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTPayPalLineItem.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1206,12 +1212,14 @@
isa = PBXGroup;
children = (
80F4F4D529F8A628003EACB1 /* Analytics */,
BEBC6E5D2927CF59004E25A0 /* Braintree.h */,
570B93AF2853980C0041BAFE /* BTAPIClient.swift */,
BE24C67428E7491E0067B11A /* BTAPIClientAuthorization.swift */,
BE24C66D28E49A730067B11A /* BTAPIClientError.swift */,
BE24C67228E73E810067B11A /* BTAPIClientHTTPType.swift */,
BE9EC0972899CF040022EC63 /* BTAPIPinnedCertificates.swift */,
8075CBED2B1B735200CA6265 /* BTAPIRequest.swift */,
BEF388C22BE9340C000965C8 /* BTAPITimingDelegate.swift */,
579DAEC6286E064500FCE87F /* BTAppContextSwitchClient.swift */,
579DAEC4286E04A700FCE87F /* BTAppContextSwitcher.swift */,
BED00CAD28A5419900D74AEC /* BTBinData.swift */,
Expand Down Expand Up @@ -1243,12 +1251,12 @@
BEBD05212A1FE8BE0003C15C /* BTWebAuthenticationSession.swift */,
BEB9BF522A26872B00A3673E /* BTWebAuthenticationSessionClient.swift */,
BE698EA128AA8EEA001D9B10 /* ConfigurationCache.swift */,
80D280CC2BE9354B00762D27 /* Date+MillisecondTimestamp.swift */,
80F86FEF29FC2ED6003297FF /* Encodable+Dictionary.swift */,
80A6C6182B21205900416D50 /* UIApplication+URLOpener.swift */,
8053F05829FB2F700076F988 /* URL+IsPayPal.swift */,
BE7FA4C5291AC9B6003C6B63 /* Info.plist */,
BEBC6E5D2927CF59004E25A0 /* Braintree.h */,
62A746402B9255AC003D32FF /* PrivacyInfo.xcprivacy */,
80A6C6182B21205900416D50 /* UIApplication+URLOpener.swift */,
8053F05829FB2F700076F988 /* URL+IsPayPal.swift */,
);
path = BraintreeCore;
sourceTree = "<group>";
Expand Down Expand Up @@ -1329,6 +1337,7 @@
children = (
BEE2E4E329007FF100C03FDD /* BTAnalyticsService.swift */,
BEE2E4E5290080BD00C03FDD /* BTAnalyticsServiceError.swift */,
BEF388C02BE52CD2000965C8 /* BTCoreAnalytics.swift */,
800E78C329E0DD5300D1B0FC /* FPTIBatchData.swift */,
);
path = Analytics;
Expand Down Expand Up @@ -2816,6 +2825,7 @@
579DAEC7286E064500FCE87F /* BTAppContextSwitchClient.swift in Sources */,
8053F05929FB2F700076F988 /* URL+IsPayPal.swift in Sources */,
BEC3F11328A4401E0074DF0F /* BTHTTPError.swift in Sources */,
80D280CD2BE9354B00762D27 /* Date+MillisecondTimestamp.swift in Sources */,
574891EB286F7E4F0020DA36 /* BTClientMetadataIntegration.swift in Sources */,
8075CBEE2B1B735200CA6265 /* BTAPIRequest.swift in Sources */,
BE698EA428AD2C10001D9B10 /* BTCoreConstants.swift in Sources */,
Expand All @@ -2833,12 +2843,14 @@
BE24C67328E73E810067B11A /* BTAPIClientHTTPType.swift in Sources */,
BE9EC0982899CF040022EC63 /* BTAPIPinnedCertificates.swift in Sources */,
BEF1089029019CAE003B2FDA /* BTAnalyticsService.swift in Sources */,
BEF388C32BE9340C000965C8 /* BTAPITimingDelegate.swift in Sources */,
574891E9286F7D300020DA36 /* BTClientMetadataSource.swift in Sources */,
BED7493628579BAC0074C818 /* BTURLUtils.swift in Sources */,
BEBD05222A1FE8BE0003C15C /* BTWebAuthenticationSession.swift in Sources */,
BE698EA028AA8DCB001D9B10 /* BTHTTP.swift in Sources */,
BC17F9B428D23C5C004B18CC /* BTGraphQLMultiErrorNode.swift in Sources */,
5708E0A828809BC6007946B9 /* BTJSONError.swift in Sources */,
BEF388C12BE52CD2000965C8 /* BTCoreAnalytics.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
## 6.18.1 (2024-05-06)
* Remove throttle delay in accessing configuration, added in v5.9.0
* Move from URLCache to NSCache for configuration caching
* BraintreePayPal
* Send `start_time`, `end_time`, and `endpoint` to FPTI for tracking API request latency

## 6.18.0 (2024-04-25)
* BraintreePayPalNativeCheckout
Expand Down
20 changes: 16 additions & 4 deletions Sources/BraintreeCore/Analytics/BTAnalyticsService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,23 @@ class BTAnalyticsService: Equatable {
func sendAnalyticsEvent(
_ eventName: String,
correlationID: String? = nil,
endpoint: String? = nil,
endTime: Int? = nil,
errorDescription: String? = nil,
linkType: String? = nil,
payPalContextID: String? = nil
payPalContextID: String? = nil,
startTime: Int? = nil
) {
Task(priority: .background) {
await performEventRequest(
eventName,
correlationID: correlationID,
endpoint: endpoint,
endTime: endTime,
errorDescription: errorDescription,
linkType: linkType,
payPalContextID: payPalContextID
payPalContextID: payPalContextID,
startTime: startTime
)
}
}
Expand All @@ -49,17 +55,23 @@ class BTAnalyticsService: Equatable {
func performEventRequest(
_ eventName: String,
correlationID: String? = nil,
endpoint: String? = nil,
endTime: Int? = nil,
errorDescription: String? = nil,
linkType: String? = nil,
payPalContextID: String? = nil
payPalContextID: String? = nil,
startTime: Int? = nil
) async {
let timestampInMilliseconds = Int(round(Date().timeIntervalSince1970 * 1000))
let timestampInMilliseconds = Date().utcTimestampMilliseconds
let event = FPTIBatchData.Event(
correlationID: correlationID,
endpoint: endpoint,
endTime: endTime,
errorDescription: errorDescription,
eventName: eventName,
linkType: linkType,
payPalContextID: payPalContextID,
startTime: startTime,
timestamp: String(timestampInMilliseconds)
)

Expand Down
6 changes: 6 additions & 0 deletions Sources/BraintreeCore/Analytics/BTCoreAnalytics.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import Foundation

enum BTCoreAnalytics {

static let apiRequestLatency = "core:api-request-latency"
}
6 changes: 6 additions & 0 deletions Sources/BraintreeCore/Analytics/FPTIBatchData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@ struct FPTIBatchData: Codable {
struct Event: Codable {

let correlationID: String?
let endpoint: String?
let endTime: Int?
let errorDescription: String?
let eventName: String
/// The type of link the SDK will be handling, currently deeplink or universal
let linkType: String?
/// Used for linking events from the client to server side request
/// This value will be PayPal Order ID, Payment Token, EC token, Billing Agreement, or Venmo Context ID depending on the flow
let payPalContextID: String?
let startTime: Int?
let timestamp: String
let tenantName: String = "Braintree"
let venmoInstalled: Bool = isVenmoAppInstalled()
Expand All @@ -47,6 +50,9 @@ struct FPTIBatchData: Codable {
case payPalContextID = "paypal_context_id"
case timestamp = "t"
case tenantName = "tenant_name"
case startTime = "start_time"
case endTime = "end_time"
case endpoint = "endpoint"
case venmoInstalled = "venmo_installed"
}
}
Expand Down
25 changes: 24 additions & 1 deletion Sources/BraintreeCore/BTAPIClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Foundation

/// This class acts as the entry point for accessing the Braintree APIs via common HTTP methods performed on API endpoints.
/// - Note: It also manages authentication via tokenization key and provides access to a merchant's gateway configuration.
@objcMembers public class BTAPIClient: NSObject {
@objcMembers public class BTAPIClient: NSObject, BTHTTPNetworkTiming {

/// :nodoc: This typealias is exposed for internal Braintree use only. Do not use. It is not covered by Semantic Versioning and may change or be removed at any time.
@_documentation(visibility: private)
Expand All @@ -19,6 +19,10 @@ import Foundation
/// Client metadata that is used for tracking the client session
public private(set) var metadata: BTClientMetadata

/// :nodoc: This property is exposed for internal Braintree use only. Do not use. It is not covered by Semantic Versioning and may change or be removed at any time.
@_documentation(visibility: private)
public var shouldSendAPIRequestLatency: Bool = false

// MARK: - Internal Properties

var configurationHTTP: BTHTTP?
Expand Down Expand Up @@ -65,12 +69,14 @@ import Foundation

tokenizationKey = authorization
configurationHTTP = BTHTTP(url: baseURL, tokenizationKey: authorization)
configurationHTTP?.networkTimingDelegate = self
case .clientToken:
do {
clientToken = try BTClientToken(clientToken: authorization)

guard let clientToken else { return nil }
configurationHTTP = try BTHTTP(clientToken: clientToken)
configurationHTTP?.networkTimingDelegate = self
} catch {
print(errorString + " Missing analytics session metadata - will not send event " + error.localizedDescription)
return nil
Expand Down Expand Up @@ -470,6 +476,8 @@ import Foundation
} else if let tokenizationKey, let baseURL {
http = BTHTTP(url: baseURL, tokenizationKey: tokenizationKey)
}

http?.networkTimingDelegate = self
}

if graphQLHTTP == nil {
Expand All @@ -482,4 +490,19 @@ import Foundation
}
}
}

// MARK: BTAPITimingDelegate conformance

func fetchAPITiming(path: String, startTime: Int, endTime: Int) {
let cleanedPath = path.replacingOccurrences(of: "/merchants/([A-Za-z0-9]+)/client_api", with: "", options: .regularExpression)

if (shouldSendAPIRequestLatency || path.contains("v1/configuration")) && cleanedPath != "/v1/tracking/batch/events" {
analyticsService?.sendAnalyticsEvent(
BTCoreAnalytics.apiRequestLatency,
endpoint: cleanedPath,
endTime: endTime,
startTime: startTime
)
}
}
}
5 changes: 5 additions & 0 deletions Sources/BraintreeCore/BTAPITimingDelegate.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import Foundation

protocol BTHTTPNetworkTiming: AnyObject {
func fetchAPITiming(path: String, startTime: Int, endTime: Int)
}
22 changes: 19 additions & 3 deletions Sources/BraintreeCore/BTHTTP.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Foundation
import Security

/// Performs HTTP methods on the Braintree Client API
class BTHTTP: NSObject, NSCopying, URLSessionDelegate {
class BTHTTP: NSObject, NSCopying, URLSessionTaskDelegate {

typealias RequestCompletion = (BTJSON?, HTTPURLResponse?, Error?) -> Void

Expand All @@ -14,12 +14,14 @@ class BTHTTP: NSObject, NSCopying, URLSessionDelegate {

/// An array of pinned certificates, each an NSData instance consisting of DER encoded x509 certificates
let pinnedCertificates: [NSData] = BTAPIPinnedCertificates.trustedCertificates()
let baseURL: URL

/// DispatchQueue on which asynchronous code will be executed. Defaults to `DispatchQueue.main`.
var dispatchQueue: DispatchQueue = DispatchQueue.main
let baseURL: URL
var clientAuthorization: ClientAuthorization?

weak var networkTimingDelegate: BTHTTPNetworkTiming?

/// Session exposed for testing
lazy var session: URLSession = {
let configuration: URLSessionConfiguration = URLSessionConfiguration.ephemeral
Expand Down Expand Up @@ -425,7 +427,7 @@ class BTHTTP: NSObject, NSCopying, URLSessionDelegate {
}
}

// MARK: - URLSessionDelegate conformance
// MARK: - URLSessionTaskDelegate conformance

func urlSession(_ session: URLSession, didReceive challenge: URLAuthenticationChallenge, completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void) {
if challenge.protectionSpace.authenticationMethod == NSURLAuthenticationMethodServerTrust {
Expand All @@ -449,4 +451,18 @@ class BTHTTP: NSObject, NSCopying, URLSessionDelegate {
completionHandler(.performDefaultHandling, nil)
}
}

func urlSession(_ session: URLSession, task: URLSessionTask, didFinishCollecting metrics: URLSessionTaskMetrics) {
metrics.transactionMetrics.forEach { transaction in
if let startDate = transaction.fetchStartDate,
let endDate = transaction.responseEndDate,
let path = transaction.request.url?.path {
networkTimingDelegate?.fetchAPITiming(
path: path,
startTime: startDate.utcTimestampMilliseconds,
endTime: endDate.utcTimestampMilliseconds
)
}
}
}
}
8 changes: 8 additions & 0 deletions Sources/BraintreeCore/Date+MillisecondTimestamp.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import Foundation

extension Date {

var utcTimestampMilliseconds: Int {
Int(round(timeIntervalSince1970 * 1000))
}
}
2 changes: 2 additions & 0 deletions Sources/BraintreePayPal/BTPayPalClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import BraintreeDataCollector
/// - Parameter apiClient: The API Client
@objc(initWithAPIClient:)
public init(apiClient: BTAPIClient) {
apiClient.shouldSendAPIRequestLatency = true

self.apiClient = apiClient
self.webAuthenticationSession = BTWebAuthenticationSession()

Expand Down
14 changes: 14 additions & 0 deletions UnitTests/BraintreeCoreTests/Analytics/FPTIBatchData_Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,24 @@ final class FPTIBatchData_Tests: XCTestCase {
let eventParams = [
FPTIBatchData.Event(
correlationID: "fake-correlation-id-1",
endpoint: "/v1/paypal_hermes/setup_billing_agreement",
endTime: 111222333444555,
errorDescription: "fake-error-description-1",
eventName: "fake-event-1",
linkType: "universal",
payPalContextID: "fake-order-id",
startTime: 999888777666,
timestamp: "fake-time-1"
),
FPTIBatchData.Event(
correlationID: nil,
endpoint: nil,
endTime: nil,
errorDescription: nil,
eventName: "fake-event-2",
linkType: nil,
payPalContextID: "fake-order-id-2",
startTime: nil,
timestamp: "fake-time-2"
)
]
Expand Down Expand Up @@ -93,5 +99,13 @@ final class FPTIBatchData_Tests: XCTestCase {
XCTAssertNil(eventParams[1]["error_desc"])
XCTAssertEqual(eventParams[0]["correlation_id"] as? String, "fake-correlation-id-1")
XCTAssertNil(eventParams[1]["correlation_id"])
XCTAssertEqual(eventParams[0]["endpoint"] as? String, "/v1/paypal_hermes/setup_billing_agreement")
XCTAssertNil(eventParams[1]["endpoint"])
XCTAssertEqual(eventParams[0]["end_time"] as? Int, 111222333444555)
XCTAssertNil(eventParams[1]["end_time"])
XCTAssertEqual(eventParams[0]["start_time"] as? Int, 999888777666)
XCTAssertNil(eventParams[1]["start_time"])
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,20 @@ import Foundation
@testable import BraintreeCore

class FakeAnalyticsService: BTAnalyticsService {
var lastEvent: String = ""
var lastEvent: String? = nil
var endpoint: String? = nil

override func sendAnalyticsEvent(
_ eventName: String,
correlationID: String? = nil,
endpoint: String? = nil,
endTime: Int? = nil,
errorDescription: String? = nil,
linkType: String? = nil,
payPalContextID: String? = nil
payPalContextID: String? = nil,
startTime: Int? = nil
) {
self.lastEvent = eventName
self.endpoint = endpoint
}
}
Loading

0 comments on commit 862455c

Please sign in to comment.