Fix: Tracing broken in GCE (app engine flexible) - Trivial (#365)

* Update Readme to use OPENCENSUS_TRACE.EXPORTER

It looks like OPENCENSUS_TRACE.REPORTER is an old thing.

* Fix byte string concat problem

* Fix other case where byte to string conversion was necessary

* Testing

* Response to comments

* Fix super silly mistake in test assertion. Been a long week

Author: dhendry

opencensus/common/monitored_resource_util/gcp_metadata_config.py

@@ -149,4 +149,10 @@ def _get_attribute(attribute_key):
                                       _GKE_ATTRIBUTES[attribute_key],
                                       _GCP_METADATA_URI_HEADER)
 
+        if attribute_value is not None and isinstance(attribute_value, bytes):
+            # At least in python3, bytes are are returned from
+            # urllib (although the response is text), convert
+            # to a normal string:
+            attribute_value = attribute_value.decode('utf-8')
+
         return attribute_value

opencensus/trace/ext/requests/trace.py

@@ -36,7 +36,11 @@ def trace_integration(tracer=None):
     """Wrap the requests library to trace it."""
     log.info('Integrated module: {}'.format(MODULE_NAME))
 
-    execution_context.set_opencensus_tracer(tracer)
+    if tracer is not None:
+        # The execution_context tracer should never be None - if it has not
+        # been set it returns a no-op tracer. Most code in this library does
+        # not handle None being used in the execution context.
+        execution_context.set_opencensus_tracer(tracer)
 
     # Wrap the requests functions
     for func in REQUESTS_WRAP_METHODS:

tests/unit/common/monitored_resource_util/test_gcp_metadata_config.py

@@ -54,6 +54,40 @@ def assign_attribute_value(*args, **kwargs):
 
         self.assertEquals(labels_list, expected_labels)
 
+    @mock.patch('opencensus.common.monitored_resource_util.'
+                'gcp_metadata_config.get_request')
+    def test_get_gce_metadata_binary_strings(self, http_request_mock):
+        """
+        At least in python 3 binary strings are returned from urllib
+        """
+        def assign_attribute_value(*args, **kwargs):
+            attribute_uri = args[0].split('/')[-1]
+            if attribute_uri == 'id':
+                return b'my-instance'
+            elif attribute_uri == 'project-id':
+                return b'my-project'
+            elif attribute_uri == 'zone':
+                return b'us-east1'
+
+        http_request_mock.side_effect = assign_attribute_value
+        GcpMetadataConfig.inited = False
+        GcpMetadataConfig.is_running = False
+        gcp_metadata_config.gcp_metadata_map = {}
+
+        self.assertTrue(GcpMetadataConfig.is_running_on_gcp())
+
+        labels_list = GcpMetadataConfig().get_gce_metadata()
+
+        self.assertEquals(len(labels_list), 3)
+
+        expected_labels = {
+            'instance_id': 'my-instance',
+            'project_id': 'my-project',
+            'zone': 'us-east1'
+        }
+
+        self.assertEquals(labels_list, expected_labels)
+
     @mock.patch.dict(os.environ,
                      {'KUBERNETES_SERVICE_HOST': '127.0.0.1',
                       'CONTAINER_NAME': 'container',

tests/unit/trace/ext/requests/test_requests_trace.py

@@ -15,8 +15,9 @@
 import unittest
 
 import mock
+from opencensus.trace.tracers import noop_tracer
 
-from opencensus.trace import span as span_module
+from opencensus.trace import span as span_module, execution_context
 from opencensus.trace.ext.requests import trace
 
 
@@ -42,9 +43,37 @@ def test_trace_integration(self):
         with patch_wrap, patch_requests:
             trace.trace_integration()
 
+            self.assertIsInstance(execution_context.get_opencensus_tracer(),
+                                  noop_tracer.NoopTracer)
+
         for func in trace.REQUESTS_WRAP_METHODS:
             self.assertEqual(getattr(mock_requests, func), wrap_result)
 
+    def test_trace_integration_set_tracer(self):
+        mock_wrap = mock.Mock()
+        mock_requests = mock.Mock()
+
+        wrap_result = 'wrap result'
+        mock_wrap.return_value = wrap_result
+
+        for func in trace.REQUESTS_WRAP_METHODS:
+            mock_func = mock.Mock()
+            mock_func.__name__ = func
+            setattr(mock_requests, func, mock_func)
+
+        patch_wrap = mock.patch(
+            'opencensus.trace.ext.requests.trace.wrap_requests', mock_wrap)
+        patch_requests = mock.patch(
+            'opencensus.trace.ext.requests.trace.requests', mock_requests)
+
+        class TmpTracer(noop_tracer.NoopTracer):
+            pass
+
+        with patch_wrap, patch_requests:
+            trace.trace_integration(tracer=TmpTracer())
+
+            self.assertIsInstance(execution_context.get_opencensus_tracer(), TmpTracer)
+
     def test_wrap_requests(self):
         mock_return = mock.Mock()
         mock_return.status_code = 200