Monitoring proxy for time series data

appengine/chrome_infra_mon_proxy/vm_module.py

Index: appengine/chrome_infra_mon_proxy/vm_module.py
diff --git a/appengine/chrome_infra_mon_proxy/vm_module.py b/appengine/chrome_infra_mon_proxy/vm_module.py
new file mode 100644
index 0000000000000000000000000000000000000000..4e5e85f2bca980dac7ded8c8f1f3d6a0cad9e127
--- /dev/null
+++ b/appengine/chrome_infra_mon_proxy/vm_module.py
@@ -0,0 +1,70 @@
+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import googleapiclient.http
+import json
+import httplib2

[ghost stip (do not use), 2015/04/14 00:36:43]

nit: alphabetize

[Sergey Berezin (google), 2015/04/16 04:39:07]

Done.

+import logging
+import os
+import traceback
+import webapp2
+
+from google.appengine.api import app_identity
+from google.appengine.ext import ndb
+
+import common

[ghost stip (do not use), 2015/04/14 00:36:44]

you should rename this to be more specific. common what?

[Sergey Berezin (google), 2015/04/16 04:39:07]

I agree it's a lousy name. I just can't think of a better one right now :-)
common_routines? utils? (well, it's not just utils). Any suggestions?

+
+
+def is_development_server():
+  return os.environ['SERVER_SOFTWARE'].startswith('Development')
+
+
+class VMHandler(webapp2.RequestHandler):

[ghost stip (do not use), 2015/04/14 00:36:44]

it pleases me how small this file is :)

[Sergey Berezin (google), 2015/04/16 04:39:08]

You should've seen the perl version :-)

+  def get(self):
+    msg = 'This endpoint is for internal POST requests only.\n'
+    self.response.headers['Content-Type'] = 'text/plain'
+    self.response.out.write(msg)

[ghost stip (do not use), 2015/04/14 00:36:43]

self.response.set_status(403) ?

[Sergey Berezin (google), 2015/04/16 04:39:08]

Actually, I just removed get(). It was there for debugging.

+
+  def post(self):
+    requester_id = self.request.headers.get('X-Appengine-Inbound-Appid', None)

[ghost stip (do not use), 2015/04/14 00:36:44]

isn't None default? I think you can just do
self.request.headers.get('X-Appengine-Inbound-Appid')

[Sergey Berezin (google), 2015/04/16 04:39:08]

Done.

+    task_name = self.request.headers.get('X-AppEngine-TaskName', None)

[ghost stip (do not use), 2015/04/14 00:36:44]

same

[Sergey Berezin (google), 2015/04/16 04:39:08]

Done.

+    my_id = app_identity.get_application_id()
+    authorized = is_development_server() or task_name or requester_id == my_id

[ghost stip (do not use), 2015/04/14 00:36:43]

nit: might want to make the 'is me' check more obvious:

def requester_is_me(requester_id):
  return requester_id == app_identity.get_application_id()

[Sergey Berezin (google), 2015/04/16 04:39:08]

Done.

+    if not authorized:
+      self.abort(403)
+    # logging.debug('Received POST /vm: %s',

[ghost stip (do not use), 2015/04/14 00:36:43]

nit: remove commented code

[Sergey Berezin (google), 2015/04/16 04:39:08]

Done.

+    #               common.payload_stats(self.request.body))
+    data = common.get_data()

[ghost stip (do not use), 2015/04/14 00:36:43]

can you rename get_data to be more specific? what does it do? where does the
data come from?

[Sergey Berezin (google), 2015/04/16 04:39:07]

Renamed, and moved the function to vm_module.py.

+    if not data:
+      self.abort_admin_error('Endpoint data is not set')
+    if not all(f in data for f in ['credentials', 'url']):
+      self.abort_admin_error('Missing required fields: credentials, url')
+
+    url = data['url']
+    http_auth = httplib2.Http()
+    if not is_development_server():
+      credentials = common.get_credentials(data['credentials'], data['scopes'])

[ghost stip (do not use), 2015/04/14 00:36:43]

if data came from common, why  do I have to pass it back into common again?
shouldn't I be able to just call common.get_credentials() ?

[Sergey Berezin (google), 2015/04/16 04:39:07]

This function is now also in vm_module.py. It's not used anywhere else.

+      http_auth = credentials.authorize(http_auth)
+    def callback(_response, _content):

[ghost stip (do not use), 2015/04/14 00:36:44]

I'm surprised this is needed. can you just set the callback to be None? do we
want to check response for error and log it?

[Sergey Berezin (google), 2015/04/16 04:39:08]

Good idea, added logging for an error in response. I don't remember about None,
I think I tried it and it crashed...

+      pass
+    # Important: set content-type to binary, otherwise httplib2 mangles it.
+    data.setdefault('headers', {}).update({
+        'content-length': str(len(self.request.body)),
+        'content-type': 'application/x-protobuf',
+    })
+    request = googleapiclient.http.HttpRequest(
+        http_auth, callback, url, method='POST', body=self.request.body,
+        headers=data['headers'])
+    request.execute()
+
+  def abort_admin_error(self, message):
+      logging.error('%s; please visit https://%s/admin/',
+                    message, app_identity.get_default_version_hostname())
+      self.abort(500)
+
+
+logging.basicConfig(level=logging.DEBUG)
+app = webapp2.WSGIApplication([
+    (r'/vm', VMHandler),

[ghost stip (do not use), 2015/04/14 00:36:44]

probably don't need the r here

[Sergey Berezin (google), 2015/04/16 04:39:08]

Done.

+    ], debug=True)