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 map.load telemetry event #7431

Merged
merged 2 commits into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 2 additions & 1 deletion src/source/raster_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { extend, pick } from '../util/util';
import { getImage, ResourceType } from '../util/ajax';
import { Event, ErrorEvent, Evented } from '../util/evented';
import loadTileJSON from './load_tilejson';
import { normalizeTileURL as normalizeURL, postTurnstileEvent } from '../util/mapbox';
import { normalizeTileURL as normalizeURL, postTurnstileEvent, postMapLoadEvent } from '../util/mapbox';
import TileBounds from './tile_bounds';
import Texture from '../render/texture';

Expand Down Expand Up @@ -70,6 +70,7 @@ class RasterTileSource extends Evented implements Source {
if (tileJSON.bounds) this.tileBounds = new TileBounds(tileJSON.bounds, this.minzoom, this.maxzoom);

postTurnstileEvent(tileJSON.tiles);
postMapLoadEvent(tileJSON.tiles, this.map._getMapId());

// `content` is included here to prevent a race condition where `Style#_updateSources` is called
// before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives
Expand Down
3 changes: 2 additions & 1 deletion src/source/vector_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Event, ErrorEvent, Evented } from '../util/evented';

import { extend, pick } from '../util/util';
import loadTileJSON from './load_tilejson';
import { normalizeTileURL as normalizeURL, postTurnstileEvent } from '../util/mapbox';
import { normalizeTileURL as normalizeURL, postTurnstileEvent, postMapLoadEvent } from '../util/mapbox';
import TileBounds from './tile_bounds';
import { ResourceType } from '../util/ajax';
import browser from '../util/browser';
Expand Down Expand Up @@ -74,6 +74,7 @@ class VectorTileSource extends Evented implements Source {
if (tileJSON.bounds) this.tileBounds = new TileBounds(tileJSON.bounds, this.minzoom, this.maxzoom);

postTurnstileEvent(tileJSON.tiles);
postMapLoadEvent(tileJSON.tiles, this.map._getMapId());

// `content` is included here to prevent a race condition where `Style#_updateSources` is called
// before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives
Expand Down
14 changes: 13 additions & 1 deletion src/ui/map.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow

import { extend, bindAll, warnOnce } from '../util/util';
import { extend, bindAll, warnOnce, uniqueId } from '../util/util';

import browser from '../util/browser';
import window from '../util/window';
Expand Down Expand Up @@ -262,6 +262,7 @@ class Map extends Camera {
_collectResourceTiming: boolean;
_renderTaskQueue: TaskQueue;
_controls: Array<IControl>;
_mapId: number;

/**
* The map's {@link ScrollZoomHandler}, which implements zooming in and out with a scroll wheel or trackpad.
Expand Down Expand Up @@ -323,6 +324,7 @@ class Map extends Camera {
this._collectResourceTiming = options.collectResourceTiming;
this._renderTaskQueue = new TaskQueue();
this._controls = [];
this._mapId = uniqueId();

const transformRequestFn = options.transformRequest;
this._transformRequest = transformRequestFn ?
Expand Down Expand Up @@ -406,6 +408,16 @@ class Map extends Camera {
});
}

/*
* Returns a unique number for this map instance which is used for the MapLoadEvent
* to make sure we only fire one event per instantiated map object.
* @private
* @returns {number}
*/
_getMapId() {
return this._mapId;
}

/**
* Adds a {@link IControl} to the map, calling `control.onAdd(this)`.
*
Expand Down
145 changes: 115 additions & 30 deletions src/util/mapbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { RequestParameters } from './ajax';
import type { Cancelable } from '../types/cancelable';

const help = 'See https://www.mapbox.com/api-documentation/#access-tokens';
const turnstileEventStorageKey = 'mapbox.turnstileEventData';
const telemEventKey = 'mapbox.eventData';

type UrlObject = {|
protocol: string,
Expand Down Expand Up @@ -133,19 +133,122 @@ function formatUrl(obj: UrlObject): string {
return `${obj.protocol}://${obj.authority}${obj.path}${params}`;
}

export class TurnstileEvent {
class TelemetryEvent {
eventData: { anonId: ?string, lastSuccess: ?number, accessToken: ?string};
queue: Array<number>;
pending: boolean
queue: Array<any>;
pendingRequest: ?Cancelable;

constructor() {
this.eventData = { anonId: null, lastSuccess: null, accessToken: config.ACCESS_TOKEN};
this.queue = [];
this.pending = false;
this.pendingRequest = null;
}

fetchEventData() {
const isLocalStorageAvailable = storageAvailable('localStorage');
const storageKey = `${telemEventKey}:${config.ACCESS_TOKEN || ''}`;

if (isLocalStorageAvailable) {
//Retrieve cached data
try {
const data = window.localStorage.getItem(storageKey);
if (data) {
this.eventData = JSON.parse(data);
}
} catch (e) {
warnOnce('Unable to read from LocalStorage');
}
}
}

saveEventData() {
const isLocalStorageAvailable = storageAvailable('localStorage');
const storageKey = `${telemEventKey}:${config.ACCESS_TOKEN || ''}`;

if (isLocalStorageAvailable) {
try {
window.localStorage.setItem(storageKey, JSON.stringify(this.eventData));
Copy link
Contributor

Choose a reason for hiding this comment

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

This would override any previously stored event data. In the case of a turnstile request finishing before the map load request, the lastSuccess property of the turnstile event would be removed, and more than one turnstile request may be processed.

The event data could include separate objects for each event type or use a different key for each event type.

} catch (e) {
warnOnce('Unable to write to LocalStorage');
}
}

}

processRequests() {}

queueRequest(date: number | {id: number, timestamp: number}) {
this.queue.push(date);
Copy link
Contributor

Choose a reason for hiding this comment

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

id is not being pushed into the queue. so the shift in MapLoadEvent#processRequests will always get undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mmm I don't think that's true. date is one argument, for turnstile events it is just a number, for map load events it is the object w id and timestamp. its working correctly for me locally and is also verified in the tests 🕵️‍♀️

this.processRequests();
}
}

export class MapLoadEvent extends TelemetryEvent {
eventData: { anonId: ?string, lastSuccess: ?number, accessToken: ?string};
queue: Array<{ id: number, timestamp: number}>;
pendingRequest: ?Cancelable;
+success: {[number]: boolean};

constructor() {
super();
this.success = {};
}

postMapLoadEvent(tileUrls: Array<string>, mapId: number) {
//Enabled only when Mapbox Access Token is set and a source uses
// mapbox tiles.
if (config.ACCESS_TOKEN &&
Array.isArray(tileUrls) &&
tileUrls.some(url => isMapboxHTTPURL(url))) {
this.queueRequest({id: mapId, timestamp: Date.now()});
}
}

processRequests() {
if (this.pendingRequest || this.queue.length === 0) return;
const {id, timestamp} = this.queue.shift();

// Only one load event should fire per map
if (id && this.success[id]) return;

if (!this.eventData.anonId) {
this.fetchEventData();
}

if (!validateUuid(this.eventData.anonId)) {
this.eventData.anonId = uuid();
}

const eventsUrlObject: UrlObject = parseUrl(config.EVENTS_URL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Code to send the request is common for all telemetry events, maybe it could be pulled into TelemetryEvent with something like:

sendTelemetryEvent(eventId, timestamp, { ..other event properties }, callback).

It would also ensure that all events correctly save event data and process the queue

eventsUrlObject.params.push(`access_token=${config.ACCESS_TOKEN || ''}`);
const request: RequestParameters = {
url: formatUrl(eventsUrlObject),
headers: {
'Content-Type': 'text/plain' //Skip the pre-flight OPTIONS request
},
body: JSON.stringify([{
event: 'map.load',
created: new Date(timestamp).toISOString(),
sdkIdentifier: 'mapbox-gl-js',
sdkVersion: version,
userId: this.eventData.anonId
}])
};

this.pendingRequest = postData(request, (error) => {
this.pendingRequest = null;
if (!error) {
if (id) this.success[id] = true;
this.saveEventData();
this.processRequests();
}
});
}
}


export class TurnstileEvent extends TelemetryEvent {

postTurnstileEvent(tileUrls: Array<string>) {
//Enabled only when Mapbox Access Token is set and a source uses
// mapbox tiles.
Expand All @@ -156,34 +259,20 @@ export class TurnstileEvent {
}
}

queueRequest(date: number) {
this.queue.push(date);
this.processRequests();
}

processRequests() {
if (this.pendingRequest || this.queue.length === 0) {
return;
}
const storageKey = `${turnstileEventStorageKey}:${config.ACCESS_TOKEN || ''}`;
const isLocalStorageAvailable = storageAvailable('localStorage');
let dueForEvent = this.eventData.accessToken ? (this.eventData.accessToken !== config.ACCESS_TOKEN) : false;

let dueForEvent = this.eventData.accessToken ? (this.eventData.accessToken !== config.ACCESS_TOKEN) : false;
//Reset event data cache if the access token changed.
if (dueForEvent) {
this.eventData.anonId = this.eventData.lastSuccess = null;
}
if ((!this.eventData.anonId || !this.eventData.lastSuccess) &&
isLocalStorageAvailable) {
if (!this.eventData.anonId || !this.eventData.lastSuccess) {
//Retrieve cached data
try {
const data = window.localStorage.getItem(storageKey);
if (data) {
this.eventData = JSON.parse(data);
}
} catch (e) {
warnOnce('Unable to read from LocalStorage');
}
this.fetchEventData();
}

if (!validateUuid(this.eventData.anonId)) {
Expand Down Expand Up @@ -227,19 +316,15 @@ export class TurnstileEvent {
if (!error) {
this.eventData.lastSuccess = nextUpdate;
this.eventData.accessToken = config.ACCESS_TOKEN;
if (isLocalStorageAvailable) {
try {
window.localStorage.setItem(storageKey, JSON.stringify(this.eventData));
} catch (e) {
warnOnce('Unable to write to LocalStorage');
}
}
this.saveEventData();
this.processRequests();
}
});
}
}

const turnstileEvent_ = new TurnstileEvent();

export const postTurnstileEvent = turnstileEvent_.postTurnstileEvent.bind(turnstileEvent_);

const mapLoadEvent_ = new MapLoadEvent();
export const postMapLoadEvent = mapLoadEvent_.postMapLoadEvent.bind(mapLoadEvent_);
1 change: 1 addition & 0 deletions test/unit/source/raster_dem_tile_source.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ function createSource(options, transformCallback) {
const source = new RasterDEMTileSource('id', options, { send: function() {} }, options.eventedParent);
source.onAdd({
transform: { angle: 0, pitch: 0, showCollisionBoxes: false },
_getMapId: () => 1,
_transformRequest: transformCallback ? transformCallback : (url) => { return { url }; }
});

Expand Down
1 change: 1 addition & 0 deletions test/unit/source/raster_tile_source.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ function createSource(options, transformCallback) {
const source = new RasterTileSource('id', options, { send: function() {} }, options.eventedParent);
source.onAdd({
transform: { angle: 0, pitch: 0, showCollisionBoxes: false },
_getMapId: () => 1,
_transformRequest: transformCallback ? transformCallback : (url) => { return { url }; }
});

Expand Down
1 change: 1 addition & 0 deletions test/unit/source/vector_tile_source.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ function createSource(options, transformCallback) {
const source = new VectorTileSource('id', options, { send: function() {} }, options.eventedParent);
source.onAdd({
transform: { showCollisionBoxes: false },
_getMapId: () => 1,
_transformRequest: transformCallback ? transformCallback : (url) => { return { url }; }
});

Expand Down
4 changes: 4 additions & 0 deletions test/unit/style/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ class StubMap extends Evented {
_transformRequest(url) {
return { url };
}

_getMapId() {
return 1;
}
}

test('Style', (t) => {
Expand Down
3 changes: 2 additions & 1 deletion test/unit/ui/control/logo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ function createMap(t, logoPosition, logoRequired) {
function createSource(options, logoRequired) {
const source = new VectorTileSource('id', options, { send: function () {} });
source.onAdd({
transform: { angle: 0, pitch: 0, showCollisionBoxes: false }
transform: { angle: 0, pitch: 0, showCollisionBoxes: false },
_getMapId: () => 1
});
source.on('error', (e) => {
throw e.error;
Expand Down
Loading