Skip to content
This repository was archived by the owner on Jan 17, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
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
14 changes: 14 additions & 0 deletions docs/examples/webextension/background.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
fetch('https://testpilot.firefox.com/api/metrics/ping/testpilottest', {
method: 'POST',
mode: 'cors',
headers: {'Content-Type': 'application/json'},
body: JSON.stringify({
"some": "random",
"metrics": "ping",
"payload": "here"
})
}).then(resp => {
console.log('metrics ping success', resp);
}).catch(e => {
console.log('problem sending metrics ping', e);
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How does .catch().then() work?
I can't remember if that means if you did catch an error it'd fall through and, you'd get both messages:

  • "problems sending metrics ping" AND
  • "metrics ping success"

Or should we reverse the order so it's .then().catch()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it bails on the catch, but I swapped the order anyway

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Turns out the catch() in the middle lets you handle the error and continue, so it'd have done both .catch() and .then():

function promiseTest(data = new Error("uh oh!")) {
  return new Promise((resolve, reject) => {
    if (data instanceof Error) {
      throw data;
    }
    resolve(data);
  });
}

promiseTest( /* error */ )
  .then((data) => {
    console.log(`first then(): ${data}`);
    return data;
  }).catch((err) => {
    console.error(`An error? ${err}`);
    return err;
  }).then((data) => {
    console.info(`after catch()? ${data}`);
  });
$ node promise.test.js

An error? Error: uh oh!
after catch()? Error: uh oh!

Binary file added docs/examples/webextension/icons/icon-32.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
19 changes: 19 additions & 0 deletions docs/examples/webextension/manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"manifest_version": 2,
"name": "Test Pilot WebExtension Example",
"version": "1.0",
"description": "This is a WebExtension built as an example Test Pilot experiment",
"icons": {
"32": "icons/icon-32.png"
},
"permissions": ["background"],
"applications": {
"gecko": {
"id": "testpilotexample1@mozilla.org",
"strict_min_version": "45.0"
}
},
"background": {
"scripts": ["background.js"]
}
}
2 changes: 2 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,5 @@ decorator==3.3.2 \
--hash=sha256:c878e3c9a4015893fddcc7a145017bd54bd51cda0eb234cab6a20fa02540cb1f
django-mozilla-product-details==0.11.1 \
--hash=sha256:6a9bf2898deae722876bd19eb874bfdcbe884f99c8096c2ddd6cdea8b95d4fa0
django-cors-headers==1.1.0 \
--hash=sha256:fcd96e2be47c8eef34c650e007a6d546e19e7ee61041b89edbbbbe7619aa3987
Empty file added testpilot/metrics/__init__.py
Empty file.
90 changes: 90 additions & 0 deletions testpilot/metrics/tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
from django.test.utils import override_settings
from django.core.urlresolvers import reverse
from django.contrib.auth.models import User

import json

from mozilla_cloud_services_logger.formatters import JsonLogFormatter

from testfixtures import LogCapture
from ..utils import TestCase


class LogPingTestCase(TestCase):

def setUp(self):
super(LogPingTestCase, self).setUp()

self.handler = LogCapture()

self.username = 'johndoe2'
self.password = 'trustno1'
self.email = '%s@example.com' % self.username

self.user = User.objects.create_user(
username=self.username,
email=self.email,
password=self.password)

def tearDown(self):
self.handler.uninstall()

def test_404_ping(self):
"""POST to /api/metrics/foobar should be a 404"""
self.client.login(username=self.username,
password=self.password)
log_name = 'foobar'
url = reverse('log-ping', args=(log_name,))
resp = self.client.post(url, {})
self.assertEqual(404, resp.status_code)

def test_unauth_ping(self):
log_name = 'testpilottest'
data = {
"service": "wayforwardthing",
"agent": "frobnitz browser"
}
url = reverse('log-ping', args=(log_name,))

self.handler.records = []
resp = self.client.post(
url,
content_type='application/json',
data=json.dumps(data))

self.assertEqual(200, resp.status_code)

record = self.handler.records[0]
formatter = JsonLogFormatter(logger_name=record.name)
details = json.loads(formatter.format(record))
fields = details['Fields']

self.assertEqual(log_name, record.name)
self.assertEqual(fields['service'], data['service'])
self.assertEqual(fields['agent'], data['agent'])

@override_settings(LOG_PING_WHITELIST=['alpha', 'beta', 'delta'])
def test_auth_ping(self):
"""POST to /api/metrics/{name} should result in expected log event"""
self.client.login(username=self.username,
password=self.password)

for log_name in ['alpha', 'beta', 'delta']:
url = reverse('log-ping', args=(log_name,))

self.handler.records = []
resp = self.client.post(
url,
content_type='application/json',
data=json.dumps({'context': 'wheee'}))

self.assertEqual(200, resp.status_code)

record = self.handler.records[0]
formatter = JsonLogFormatter(logger_name=record.name)
details = json.loads(formatter.format(record))
fields = details['Fields']

self.assertEqual(log_name, record.name)
self.assertEqual(fields['uid'], self.user.id)
self.assertEqual(fields['context'], 'wheee')
8 changes: 8 additions & 0 deletions testpilot/metrics/urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from django.conf.urls import url, patterns

from . import views

urlpatterns = patterns(
'',
url(r'^ping/(?P<logger_name>.+)', views.log_ping, name='log-ping'),
)
30 changes: 30 additions & 0 deletions testpilot/metrics/views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import logging

from django.conf import settings
from django.http import Http404

from django.views.decorators.csrf import csrf_exempt
from rest_framework.response import Response
from rest_framework.decorators import permission_classes, api_view

DEFAULT_LOG_PING_WHITELIST = [
'testpilottest'
]


@csrf_exempt
@api_view(['POST'])
@permission_classes([])
def log_ping(request, logger_name):
"""Accept and log metrics pings"""
whitelist = getattr(settings, 'LOG_PING_WHITELIST', DEFAULT_LOG_PING_WHITELIST)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So we will need to add logger names for each experiment that uses this? I.e., for 404s and my "blok" add-on/experiment, we would need to have:
settings.py:

LOG_PING_WHITELIST = [
    'blok',
    'nomore404s'
]

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this should be like the general testpilottest ping in Telemetry. One log event type (and URL) for all experiments. You should include the experiment's add-on ID in the JSON payload. Don't really have a schema for that payload, but i figured it could look like the telemetry ping.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

FWIW, the whitelist is kind of a leftover from when we thought we'd be doing more of our own metrics with events like newuser and newinstall

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we update the docs/examples/webextension/background.js:fetch to include a more realistic example of the POST body with add-on ID?

  body: JSON.stringify({
    service: "blok@mozilla",
    msg: { domain: "economist.com", blockedTrackers: ["googleadservices.com", "facebook.net", "doubleclick.net"], reason: "broken"}
  })

Or whatever the add-ons need to POST that isn't auto-filled by the server?

if logger_name not in whitelist:
raise Http404

extra = {}
if request.user.is_authenticated():
extra['uid'] = request.user.id
extra.update(request.data)
logging.getLogger(logger_name).info('', extra=extra)

return Response({'status': 'ok'})
11 changes: 11 additions & 0 deletions testpilot/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,15 @@ def path(*args):
'testpilot.base',
'testpilot.frontend',
'testpilot.users',
'testpilot.metrics',
'testpilot.experiments',

# Third party apps
'django_jinja',
'django_cleanup',

'csp',
'corsheaders',
'colorfield',
'rest_framework',
'storages',
Expand Down Expand Up @@ -108,6 +110,7 @@ def path(*args):
MIDDLEWARE_CLASSES = (
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.locale.LocaleMiddleware',
'corsheaders.middleware.CorsMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
Expand Down Expand Up @@ -423,6 +426,9 @@ def lazy_langs():
'https://*.mozilla.net',
)

CORS_ORIGIN_ALLOW_ALL = True
CORS_URLS_REGEX = r'^/api/.*$'

LOGGING = {
'version': 1,
'disable_existing_loggers': False,
Expand Down Expand Up @@ -464,6 +470,11 @@ def lazy_langs():
'level': config('DJANGO_LOG_LEVEL', default='INFO'),
'propagate': True,
},
'testpilottest': {
'handlers': ['console'],
'level': 'INFO',
'propagate': True,
},
'request.summary': {
'handlers': ['console'],
'level': 'INFO',
Expand Down
1 change: 1 addition & 0 deletions testpilot/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
url(r'^admin/', include(admin.site.urls)),
url(r'^api-auth/', include('rest_framework.urls',
namespace='rest_framework')),
url(r'^api/metrics/', include('testpilot.metrics.urls')),
url(r'^api/experiments/', include('testpilot.experiments.urls')),
url(r'^api/', include(router.urls)),
# Catch-all fallback to frontend client view
Expand Down