Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Refactor telemetry keys #3620

Closed
wants to merge 9 commits into from
Closed

Refactor telemetry keys #3620

wants to merge 9 commits into from

Conversation

friedbunny
Copy link
Contributor

  • Add pluggedIn key to non-location events
  • Add enabled.telemetry key to turnstile (supercedes Add telemetry on/off status to turnstile event #3496)
  • Remove instance
  • Remove bundleId
  • Remove lat,lng from interaction events
  • Remove obsolete enabled.email key
  • Make pushEnabled an instance method
  • Move most static keys to map.load

/cc @1ec5 @mapbox/mobiledata @boundsj

@friedbunny friedbunny added iOS Mapbox Maps SDK for iOS refactor telemetry Integration with Mapbox Telemetry libraries in progress labels Jan 19, 2016
@friedbunny friedbunny self-assigned this Jan 19, 2016
@friedbunny friedbunny added this to the ios-v3.1.0 milestone Jan 19, 2016
@friedbunny friedbunny mentioned this pull request Jan 19, 2016
Closed
@friedbunny
Copy link
Contributor Author

This should also bump one or both of the version numbers. @camilleanne how about setting the user agent to MapboxEventsiOS/2.0 and version to 2?

@camilleanne
Copy link

@friedbunny could you post an example of what each of the new events will look like? I'm having a hard time reading through all this quickly. Also, yes, the version numbers should bump. When will they go live? I would need to set up validation in api-events for a v2

@friedbunny
Copy link
Contributor Author

@camilleanne Sure, I'll get you JSON. This will go live in a couple weeks when v3.1.0 is released.

1ec5
1ec5 reviewed Jan 22, 2016
View reviewed changes
platform/ios/src/MGLMapView.mm
@@ -1474,15 +1466,11 @@ - (BOOL)gestureRecognizer:(UIGestureRecognizer *)gestureRecognizer shouldRecogni
return ([validSimultaneousGestures containsObject:gestureRecognizer] && [validSimultaneousGestures containsObject:otherGestureRecognizer]);
}

- (void)trackGestureEvent:(NSString *)gestureID forRecognizer:(UIGestureRecognizer *)recognizer
- (void)trackGestureEvent:(NSString *)gestureID forRecognizer:(__unused UIGestureRecognizer *)recognizer
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this parameter is unused, just remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e87edeb

@friedbunny friedbunny modified the milestones: ios-v3.2.0, ios-v3.1.0 Jan 25, 2016
@boundsj boundsj mentioned this pull request Feb 23, 2016
Closed
@boundsj
Copy link
Contributor

boundsj commented Feb 23, 2016

Updates:

  • We can reduce lat/lng precision to 7 digits.
  • It turns out that the version = 1 in mapbox events is no longer required because we no longer need to send that along in each event. I'm going to ignore it for now and come back to it when I work on telem diet / V2 events.
  • We should not Remove lat,lng from interaction events as noted above - we decided to keep Lat, Lon, and Zoom for Gesture events provided that the unique id is not part of the event bundle
  • We will update the user agent to include the SDK

In general, because so much has moved on in mapview and events, this PR should probably serve as a reference and we can cherry pick in the still relevant parts to a fresh branch.

@boundsj boundsj assigned boundsj and unassigned friedbunny Feb 24, 2016
@boundsj boundsj mentioned this pull request Mar 1, 2016
Closed
14 tasks
@boundsj boundsj modified the milestones: ios-v3.2.0, ios-v3.3.0 Mar 3, 2016
@boundsj
Copy link
Contributor

boundsj commented Mar 4, 2016

Subsumed by #4164

@boundsj boundsj closed this Mar 4, 2016
@friedbunny friedbunny deleted the fb-telemetry-key-diet branch March 11, 2016 15:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@boundsj boundsj

Labels
iOS Mapbox Maps SDK for iOS refactor telemetry Integration with Mapbox Telemetry libraries
Projects
None yet
Milestone
ios-v3.2.0
Development

Successfully merging this pull request may close these issues.
5 participants
@friedbunny @camilleanne @boundsj @1ec5 @jfirebaugh