Addressing feedback

moving the generate span / trace id methods back to API. no longer needed due to #235

moving test service to it's own module.

modifying shell script to use bourne shell, using posix standard location

examples/opentelemetry-example-app/tests/test_flask_example.py

@@ -20,7 +20,7 @@
 from werkzeug.wrappers import BaseResponse
 
 import opentelemetry_example_app.flask_example as flask_example
-from opentelemetry import trace
+from opentelemetry.sdk import trace
 from opentelemetry.sdk.context.propagation import b3_format
 
 

examples/trace/server.py

@@ -54,27 +54,6 @@ def index():
     return "hello"
 
 
-@app.route("/verify-tracecontext", methods=["POST"])
-def verify_tracecontext():
-    """Upon reception of some payload, sends a request back to the designated url.
-
-    This route is designed to be testable with the w3c tracecontext server / client test.
-    """
-    for action in flask.request.json:
-        requests.post(
-            url=action["url"],
-            data=json.dumps(action["arguments"]),
-            headers={
-                "Accept": "application/json",
-                "Content-Type": "application/json; charset=utf-8",
-            },
-            timeout=5.0,
-        )
-    return "hello"
-
-
 if __name__ == "__main__":
-    try:
-        app.run(debug=True)
-    finally:
-        span_processor.shutdown()
+    app.run(debug=True)
+    span_processor.shutdown()

ext/opentelemetry-ext-http-requests/src/opentelemetry/ext/http_requests/__init__.py

@@ -65,7 +65,7 @@ def instrumented_request(self, method, url, *args, **kwargs):
                 path = "<URL parses to None>"
             path = parsed_url.path
 
-        with tracer.start_span(path, kind=SpanKind.CLIENT) as span:
+        with tracer.start_as_current_span(path, kind=SpanKind.CLIENT) as span:
             span.set_attribute("component", "http")
             span.set_attribute("http.method", method.upper())
             span.set_attribute("http.url", url)

opentelemetry-api/src/opentelemetry/context/propagation/tracecontexthttptextformat.py

@@ -35,11 +35,11 @@
 
 _KEY_FORMAT = _KEY_WITHOUT_VENDOR_FORMAT + "|" + _KEY_WITH_VENDOR_FORMAT
 _VALUE_FORMAT = (
-    r"[\x20-\x2b\x2d-\x3c\x3e-\x7e]{1,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]?"
+    r"[\x20-\x2b\x2d-\x3c\x3e-\x7e]{0,255}[\x21-\x2b\x2d-\x3c\x3e-\x7e]"
 )
 
 _DELIMITER_FORMAT = "[ \t]*,[ \t]*"
-_MEMBER_FORMAT = "({})(=)({})".format(_KEY_FORMAT, _VALUE_FORMAT)
+_MEMBER_FORMAT = "({})(=)({})[ \t]*".format(_KEY_FORMAT, _VALUE_FORMAT)
 
 _DELIMITER_FORMAT_RE = re.compile(_DELIMITER_FORMAT)
 _MEMBER_FORMAT_RE = re.compile(_MEMBER_FORMAT)
@@ -68,25 +68,25 @@ def extract(
         header = get_from_carrier(carrier, cls._TRACEPARENT_HEADER_NAME)
 
         if not header:
-            return trace.generate_span_context()
+            return trace.INVALID_SPAN_CONTEXT
 
         match = re.search(cls._TRACEPARENT_HEADER_FORMAT_RE, header[0])
         if not match:
-            return trace.generate_span_context()
+            return trace.INVALID_SPAN_CONTEXT
 
         version = match.group(1)
         trace_id = match.group(2)
         span_id = match.group(3)
         trace_options = match.group(4)
 
         if trace_id == "0" * 32 or span_id == "0" * 16:
-            return trace.generate_span_context()
+            return trace.INVALID_SPAN_CONTEXT
 
         if version == "00":
             if match.group(5):
-                return trace.generate_span_context()
+                return trace.INVALID_SPAN_CONTEXT
         if version == "ff":
-            return trace.generate_span_context()
+            return trace.INVALID_SPAN_CONTEXT
 
         tracestate_headers = get_from_carrier(
             carrier, cls._TRACESTATE_HEADER_NAME

opentelemetry-api/src/opentelemetry/trace/__init__.py

@@ -309,31 +309,6 @@ def format_span_id(span_id: int) -> str:
     return "0x{:016x}".format(span_id)
 
 
-def generate_span_id() -> int:
-    """Get a new random span ID.
-
-    Returns:
-        A random 64-bit int for use as a span ID
-    """
-    return random.getrandbits(64)
-
-
-def generate_trace_id() -> int:
-    """Get a new random trace ID.
-
-    Returns:
-        A random 128-bit int for use as a trace ID
-    """
-    return random.getrandbits(128)
-
-
-def generate_span_context() -> "SpanContext":
-    """Generate a valid SpanContext."""
-    return SpanContext(
-        trace_id=generate_trace_id(), span_id=generate_span_id()
-    )
-
-
 class SpanContext:
     """The state of a Span to propagate between processes.
 
@@ -364,11 +339,11 @@ def __init__(
         self.trace_state = trace_state
 
     def __repr__(self) -> str:
-        return "{}(trace_id={}, span_id={}, trace_state={})".format(
+        return "{}(trace_id={}, span_id={}, trace_state={!r})".format(
             type(self).__name__,
             format_trace_id(self.trace_id),
             format_span_id(self.span_id),
-            repr(self.trace_state),
+            self.trace_state,
         )
 
     def is_valid(self) -> bool:

opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

@@ -298,6 +298,22 @@ def set_status(self, status: trace_api.Status) -> None:
         self.status = status
 
 
+def generate_span_id() -> int:
+    """Get a new random span ID.
+    Returns:
+        A random 64-bit int for use as a span ID
+    """
+    return random.getrandbits(64)
+
+
+def generate_trace_id() -> int:
+    """Get a new random trace ID.
+    Returns:
+        A random 128-bit int for use as a trace ID
+    """
+    return random.getrandbits(128)
+
+
 class Tracer(trace_api.Tracer):
     """See `opentelemetry.trace.Tracer`.
 

opentelemetry-sdk/tests/context/propagation/test_b3_format.py

@@ -30,10 +30,10 @@ class TestB3Format(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls.serialized_trace_id = b3_format.format_trace_id(
-            trace_api.generate_trace_id()
+            trace.generate_trace_id()
         )
         cls.serialized_span_id = b3_format.format_span_id(
-            trace_api.generate_span_id()
+            trace.generate_span_id()
         )
 
     def test_extract_multi_header(self):

opentelemetry-sdk/tests/trace/test_trace.py

@@ -221,16 +221,16 @@ def test_span_members(self):
         tracer = trace.Tracer("test_span_members")
 
         other_context1 = trace_api.SpanContext(
-            trace_id=trace_api.generate_trace_id(),
-            span_id=trace_api.generate_span_id(),
+            trace_id=trace.generate_trace_id(),
+            span_id=trace.generate_span_id(),
         )
         other_context2 = trace_api.SpanContext(
-            trace_id=trace_api.generate_trace_id(),
-            span_id=trace_api.generate_span_id(),
+            trace_id=trace.generate_trace_id(),
+            span_id=trace.generate_span_id(),
         )
         other_context3 = trace_api.SpanContext(
-            trace_id=trace_api.generate_trace_id(),
-            span_id=trace_api.generate_span_id(),
+            trace_id=trace.generate_trace_id(),
+            span_id=trace.generate_span_id(),
         )
 
         self.assertIsNone(tracer.get_current_span())
@@ -362,8 +362,8 @@ def test_ended_span(self):
         tracer = trace.Tracer("test_ended_span")
 
         other_context1 = trace_api.SpanContext(
-            trace_id=trace_api.generate_trace_id(),
-            span_id=trace_api.generate_span_id(),
+            trace_id=trace.generate_trace_id(),
+            span_id=trace.generate_span_id(),
         )
 
         with tracer.start_as_current_span("root") as root:

scripts/tracecontext-integration-test.sh

@@ -1,5 +1,5 @@
-#!/bin/bash
-# set -e 
+#!/bin/sh
+set -e 
 # hard-coding the git tag to ensure stable builds.
 TRACECONTEXT_GIT_TAG="98f210efd89c63593dce90e2bae0a1bdcb986f51"
 # clone w3c tracecontext tests
@@ -9,18 +9,18 @@ git clone https://github.com/w3c/trace-context ./target/trace-context
 cd ./target/trace-context && git checkout $TRACECONTEXT_GIT_TAG && cd -
 # start example opentelemetry service, which propagates trace-context by 
 # default.
-python ./examples/trace/server.py 1>&2 &
+python ./tests/w3c_tracecontext_validation_server.py 1>&2 &
 EXAMPLE_SERVER_PID=$!
 # give the app server a little time to start up. Not adding some sort
 # of delay would cause many of the tracecontext tests to fail being
 # unable to connect.
 sleep 1
-function on-shutdown {
+function onshutdown {
     # send a sigint, to ensure
     # it is caught as a KeyboardInterrupt in the
     # example service.
-    kill -2 $EXAMPLE_SERVER_PID
+    kill $EXAMPLE_SERVER_PID
 }
-trap on-shutdown EXIT
+trap onshutdown EXIT
 cd ./target/trace-context/test
 python test.py http://127.0.0.1:5000/verify-tracecontext

tests/w3c_tracecontext_validation_server.py

@@ -0,0 +1,75 @@
+#!/usr/bin/env python3
+#
+# Copyright 2019, OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""
+This server is intended to be used with the W3C tracecontext validation
+Service. It implements the APIs needed to be exercised by the test bed.
+"""
+
+import json
+
+import flask
+import requests
+
+from opentelemetry import trace
+from opentelemetry.ext import http_requests
+from opentelemetry.ext.wsgi import OpenTelemetryMiddleware
+from opentelemetry.sdk.trace import Tracer
+from opentelemetry.sdk.trace.export import (
+    ConsoleSpanExporter,
+    SimpleExportSpanProcessor,
+)
+
+# The preferred tracer implementation must be set, as the opentelemetry-api
+# defines the interface with a no-op implementation.
+trace.set_preferred_tracer_implementation(lambda T: Tracer())
+
+# Integrations are the glue that binds the OpenTelemetry API and the
+# frameworks and libraries that are used together, automatically creating
+# Spans and propagating context as appropriate.
+http_requests.enable(trace.tracer())
+
+# SpanExporter receives the spans and send them to the target location.
+span_processor = SimpleExportSpanProcessor(ConsoleSpanExporter())
+trace.tracer().add_span_processor(span_processor)
+
+app = flask.Flask(__name__)
+app.wsgi_app = OpenTelemetryMiddleware(app.wsgi_app)
+
+
+@app.route("/verify-tracecontext", methods=["POST"])
+def verify_tracecontext():
+    """Upon reception of some payload, sends a request back to the designated url.
+
+    This route is designed to be testable with the w3c tracecontext server / client test.
+    """
+    for action in flask.request.json:
+        requests.post(
+            url=action["url"],
+            data=json.dumps(action["arguments"]),
+            headers={
+                "Accept": "application/json",
+                "Content-Type": "application/json; charset=utf-8",
+            },
+            timeout=5.0,
+        )
+    return "hello"
+
+
+if __name__ == "__main__":
+    try:
+        app.run(debug=True)
+    finally:
+        span_processor.shutdown()

tox.ini

@@ -117,7 +117,7 @@ commands =
 basepython: python3.7
 deps =
   # needed for tracecontext
-  aiohttp~=3.6 
+  aiohttp~=3.6
   # needed for example trace integration
   flask~=1.1
   requests~=2.7
@@ -128,5 +128,5 @@ commands_pre =
   pip install -e {toxinidir}/ext/opentelemetry-ext-http-requests
   pip install -e {toxinidir}/ext/opentelemetry-ext-wsgi
 
-commands = 
+commands =
   {toxinidir}/scripts/tracecontext-integration-test.sh