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.

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 metrics logging for time to search in project #1078

Merged
merged 6 commits into from May 8, 2019
Merged
Changes from 3 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -0,0 +1,23 @@
# Events specification

This document specifies all the data (along with the format) which gets sent from the Find and Replace package to the GitHub analytics pipeline. This document follows the same format and nomenclature as the [Atom Core Events spec](https://github.com/atom/metrics/blob/master/docs/events.md).

## Counters

Currently Find and Replace does not log any counter events.

## Timing events

#### Time to search on a project

* **eventType**: `find-and-replace-v1`
* **metadata**

| field | value |
|-------|-------|
| `ec` | `time-to-search`
| `ev` | Number of found results

## Standard events

Currently Find and Replace does not log any standard events.
@@ -9,6 +9,9 @@ FindView = require './find-view'
ProjectFindView = require './project-find-view'
ResultsModel = require './project/results-model'
ResultsPaneView = require './project/results-pane'
ReporterProxy = require './reporter-proxy'

metricsReporter = new ReporterProxy()

module.exports =
activate: ({findOptions, findHistory, replaceHistory, pathsHistory}={}) ->
@@ -28,7 +31,7 @@ module.exports =

@findOptions = new FindOptions(findOptions)
@findModel = new BufferSearch(@findOptions)
@resultsModel = new ResultsModel(@findOptions)
@resultsModel = new ResultsModel(@findOptions, metricsReporter)

@subscriptions.add atom.workspace.getCenter().observeActivePaneItem (paneItem) =>
@subscriptions.delete @currentItemSub
@@ -132,6 +135,11 @@ module.exports =
'find-and-replace:select-skip': (event) ->
selectNextObjectForEditorElement(this).skipCurrentSelection()

consumeMetricsReporter: (service) ->
metricsReporter.setReporter(service)
new Disposable ->
metricsReporter.unsetReporter()

consumeElementIcons: (service) ->
getIconServices().setElementIcons service
new Disposable ->
@@ -27,7 +27,8 @@ class Result {
}

module.exports = class ResultsModel {
constructor (findOptions) {
constructor (findOptions, metricsReporter) {
this.metricsReporter = metricsReporter
this.onContentsModified = this.onContentsModified.bind(this)
this.findOptions = findOptions
this.emitter = new Emitter()
@@ -172,6 +173,9 @@ module.exports = class ResultsModel {

const leadingContextLineCount = atom.config.get('find-and-replace.searchContextLineCountBefore')
const trailingContextLineCount = atom.config.get('find-and-replace.searchContextLineCountAfter')

const startTime = Date.now()

this.inProgressSearchPromise = atom.workspace.scan(
this.regex,
{
@@ -195,8 +199,11 @@ module.exports = class ResultsModel {
if (message === 'cancelled') {
this.emitter.emit('did-cancel-searching')
} else {
const resultsSummary = this.getResultsSummary()

this.metricsReporter.sendSearchEvent(Date.now() - startTime, resultsSummary.matchCount)
this.inProgressSearchPromise = null
this.emitter.emit('did-finish-searching', this.getResultsSummary())
this.emitter.emit('did-finish-searching', resultsSummary)
}
})
}
@@ -0,0 +1,38 @@
module.exports = class ReporterProxy {
constructor () {
this.reporter = null
this.timingsQueue = []

this.eventType = 'find-and-replace-v1'
}

setReporter (reporter) {
this.reporter = reporter
let timingsEvent

while ((timingsEvent = this.timingsQueue.shift())) {
this.reporter.addTiming(this.eventType, timingsEvent[0], timingsEvent[1])
}
}

unsetReporter () {
delete this.reporter
}

sendSearchEvent (duration, numResults) {
const metadata = {
ec: 'time-to-search',
ev: numResults
}

this._addTiming(duration, metadata)
}

_addTiming (duration, metadata) {
if (this.reporter) {
this.reporter.addTiming(this.eventType, duration, metadata)
} else {
this.timingsQueue.push([duration, metadata])

This comment has been minimized.

Copy link
@nathansobo

nathansobo May 8, 2019

Contributor

This is subjective and minor, but pushing { duration, metadata } would have made setReporter slightly more readable for me.

}
}
}

Some generated files are not rendered by default. Learn more.

@@ -40,7 +40,8 @@
},
"devDependencies": {
"coffeelint": "^1.9.7",
"dedent": "^0.6.0"
"dedent": "^0.6.0",
"sinon": "1.17.4"
},
"consumedServices": {
"atom.file-icons": {
@@ -57,6 +58,11 @@
"versions": {
"1.0.0": "consumeElementIcons"
}
},
"metrics-reporter": {
"versions": {
"^1.1.0": "consumeMetricsReporter"
}
}
},
"providedServices": {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.