Please add B3 header support :)

Please see this doc for the problem statement and specifics: https://docs.google.com/document/d/1ukvnSoXkavcojlVXpCRzep8PS_guXly_NbbhHIDgbFY/edit

CC: @Carlosalbert, @tedsuo

lightstep requires thrift version 0.10. Is it possible to upgrade to the latest version 0.11?

LightStep version 4.2.0, OS X

[feel free to move this to OpenTracing Python if necessary]

I've constructed a minimal reproduction of the issue here. When send_spans_memory_leak is called repeatedly, the program has a memory leak (see graph below). When send_spans_stable is called repeatedly, the program memory footprint is stable.

Although the code in send_spans_memory_leak is illogical (it never actually ends the scope), I can imagine end users writing code like this by mistake. Nevertheless, I think that incorrectly instrumenting an application shouldn't chew up this much memory.

import lightstep

def send_spans_memory_leak(tracer):
    with tracer.start_span(operation_name='make_some_request') as client_span:
        tracer.scope_manager.activate(client_span, True)
        pass

def send_spans_stable(tracer):
    with tracer.start_active_span(operation_name='make_some_request') as client_span:
        pass

# setup tracer
tracer = lightstep.Tracer(
    component_name='isaac_service',
    collector_port=8360,
    collector_host='localhost',
    collector_encryption='none',
    use_http=True,
    access_token='developer',
    max_span_records=100,
)

while True:
    send_spans_memory_leak(tracer)

Setting thrift as the default value will fail because we check for http transport first (the default) - this should be changed so that either transport is mutually exclusive.

Looks like the latest version of opentracing has no module called logs, when I updated to latest version of lightstep I got the following error:
import lightstep /usr/local/lib/python3.7/site-packages/lightstep/__init__.py:3: in <module> from .tracer import Tracer # noqa /usr/local/lib/python3.7/site-packages/lightstep/tracer.py:17: in <module> from .recorder import Recorder /usr/local/lib/python3.7/site-packages/lightstep/recorder.py:17: in <module> from opentracing.logs import ERROR_KIND, STACK, ERROR_OBJECT E ModuleNotFoundError: No module named 'opentracing.logs'

I believe the issue is that https://github.com/lightstep/lightstep-tracer-python/blob/master/lightstep/crouton/ttypes.py was generated with 0.9.2, but lightstep now requires 0.10.0. Accessing the accelerated module appears to have changed since 0.10.0 (apache/thrift#844). I recommend you re-generate the thrift code with a newer version of the Thrift compiler.

Recently our Python-based project upgraded a lot of our dependencies, including lightstep, protobuf, and grpcio. Since then when we run our unit tests, we see several warning messages from lightstep like these:

.tox/py39-unit/lib/python3.9/site-packages/lightstep/lightstep_carrier_pb2.py:18
  /Users/ed/projects/ngc-cli/.tox/py39-unit/lib/python3.9/site-packages/lightstep/lightstep_carrier_pb2.py:18: DeprecationWarning: Call to deprecated create function FileDescriptor(). Note: Create unlinked descriptors is going to go away. Please use get/find descriptors from generated code or query the descriptor_pool.
    DESCRIPTOR = _descriptor.FileDescriptor(

.tox/py39-unit/lib/python3.9/site-packages/lightstep/lightstep_carrier_pb2.py:36
  /Users/ed/projects/ngc-cli/.tox/py39-unit/lib/python3.9/site-packages/lightstep/lightstep_carrier_pb2.py:36: DeprecationWarning: Call to deprecated create function FieldDescriptor(). Note: Create unlinked descriptors is going to go away. Please use get/find descriptors from generated code or query the descriptor_pool.
    _descriptor.FieldDescriptor(

.tox/py39-unit/lib/python3.9/site-packages/lightstep/lightstep_carrier_pb2.py:43
  /Users/ed/projects/ngc-cli/.tox/py39-unit/lib/python3.9/site-packages/lightstep/lightstep_carrier_pb2.py:43: DeprecationWarning: Call to deprecated create function FieldDescriptor(). Note: Create unlinked descriptors is going to go away. Please use get/find descriptors from generated code or query the descriptor_pool.
    _descriptor.FieldDescriptor(

.tox/py39-unit/lib/python3.9/site-packages/lightstep/lightstep_carrier_pb2.py:29
  /Users/ed/projects/ngc-cli/.tox/py39-unit/lib/python3.9/site-packages/lightstep/lightstep_carrier_pb2.py:29: DeprecationWarning: Call to deprecated create function Descriptor(). Note: Create unlinked descriptors is going to go away. Please use get/find descriptors from generated code or query the descriptor_pool.
    _BINARYCARRIER = _descriptor.Descriptor(

The tests continue to pass, but I wanted to a) be sure you are aware of these deprecations, and b) was curious about any plans to remediate them.

What is the intended functionality around the sampled span context? I've noticed that it's propagated from parent spans and it's serialized into carriers, but I couldn't find code to read it during recording or sending spans to the collector.

Considering this and this, the non-zero bits are being truncated here.

For example:

>>> from random import Random
>>> format(Random().getrandbits(64) - 1, "032x")
'0000000000000000bff1e96093d533ac'
>>> format(Random().getrandbits(64) - 1, "032x")[:16]
'0000000000000000'
>>> format(Random().getrandbits(64) - 1, "032x")[16:]
'cd60576d046d97a7'

We are having random results when it comes to performance testing. Research for a possible solution that enables performance testing to have a reliable testing base that produces repeatable performance testing results.

Please add trace-context header support :)

Please see this doc for the problem statement and specifics: https://docs.google.com/document/d/1ukvnSoXkavcojlVXpCRzep8PS_guXly_NbbhHIDgbFY/edit

We've instrumented our service with LightStep and OpenTracing but have run into a snag when using B3 propagation.

    opentracing.tracer = lightstep.Tracer(
        component_name=c.SERVICE_NAME,
        access_token=OPEN_TRACE_ACCESS_TOKEN,
        tags=tags
    )
    opentracing.tracer.register_propagator(Format.TEXT_MAP, B3Propagator())
    opentracing.tracer.register_propagator(
        Format.HTTP_HEADERS, B3Propagator()
    )

this is the error I'm seeing in developer satellite:

error
     message:
     "Value for header {x-b3-sampled: 1} must be of type str or bytes, not <class 'int'>"
     error.object:
     "Value for header {x-b3-sampled: 1} must be of type str or bytes, not <class 'int'>"
     error.kind:
     "InvalidHeader"

stack trace:

"line 51, in get response = requests.get( File "/usr/local/lib/python3.8/site-packages/requests/api.py", line 75, in get return request('get', url, params=params, **kwargs) File "/usr/local/lib/python3.8/site-packages/requests/api.py", line 60, in request return session.request(method=method, url=url, **kwargs) File "/usr/local/lib/python3.8/site-packages/requests/sessions.py", line 519, in request prep = self.prepare_request(req) File "/usr/local/lib/python3.8/site-packages/requests/sessions.py", line 452, in prepare_request p.prepare( File "/usr/local/lib/python3.8/site-packages/requests/models.py", line 314, in prepare self.prepare_headers(headers) File "/usr/local/lib/python3.8/site-packages/requests/models.py", line 448, in prepare_headers check_header_validity(header) File "/usr/local/lib/python3.8/site-packages/requests/utils.py", line 944, in check_header_validity raise InvalidHeader("Value for header {%s: %s} must be of type str or " "

Appears to be failing python requests library's header validation

Issue:

Our team has started integrating python Lightstep support within aiohttp (asyncio) services.
We noticed when doing cross language traces that the python generated span timelines were skewed compared to other spans emitted in the same trace

Span start times as reported in Lightstep UI are always truncated to the second. This is visible when observing the microseconds stamp in Lightstep UI (see below). Note the trailing 000000.

Digging into the Lightstep code, I believe is due to the following code line in http_converter.py

start_timestamp=Timestamp(seconds=int(span.start_time)),

Which casts the span.start_time (float) to the second (int) and sets it on the Timestamp object.
Since spans sent via proto are now shifted to the second, the resulting finish time (which appears to be calculated start_time + duration) are marked as finishing earlier than they actually did.
The result of this was we saw spans emitted by downstream traced services appear later in the trace timeline than the parent calling span in python

Potential Solution:

Looking at the google docs on Timestamp, it appears we can supply both the seconds and nanos as a parameter to the object. An example can be found here:

now = time.time()
seconds = int(now)
nanos = int((now - seconds) * 10**9)
timestamp = Timestamp(seconds=seconds, nanos=nanos)

After locally applying the suggested code to the http_converter.py, traces emitted now appear to have the correct resolution.

Setup:

python==3.6.6
opentracing==2.0.0
lightstep==4.0.0
googleapis-common-protos==1.5.3

Can you confirm this is unintentional behaviour and whether our proposed solution would be accepted?

Thanks

We came across an issue internally where we have services which had a declared dependency on protobuf==3.0.0. When the lightstep dependency was introduced and imported we received an exception TypeError: __init__() got an unexpected keyword argument 'serialized_options'

It would appear the lightstep protos have been compiled with protoc 3.6.0 or greater which introduced the serialised_options property which isn't available in earlier versions of protobuf.

Dependencies of the lightstep library which require protobuf declare a required version of 3.0.0 or greater. Lightstep does not have a dependency stated for protobuf==3.6.0

$ pipdeptree
lightstep==4.0.1
  - basictracer [required: >=3.0,<3.1, installed: 3.0.0]
    - opentracing [required: ==2.0.0, installed: 2.0.0]
    - protobuf [required: >=3.0.0b2.post2, installed: 3.0.0]
      - setuptools [required: Any, installed: 39.0.1]
      - six [required: >=1.9, installed: 1.11.0]
    - six [required: >=1.10.0,<2.0, installed: 1.11.0]
  - googleapis-common-protos [required: ==1.5.3, installed: 1.5.3]
    - protobuf [required: >=3.0.0, installed: 3.0.0]
      - setuptools [required: Any, installed: 39.0.1]
      - six [required: >=1.9, installed: 1.11.0]

Repo Steps:

pip install protobuf==3.0.0 lightstep

$ python
Python 3.6.6 (default, Oct 17 2018, 16:21:40)
[GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import lightstep
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/michaelb/basement/skyscanner/forks/lightstep-tracer-python/lightstep/__init__.py", line 3, in <module>
    from .tracer import Tracer  # noqa
  File "/Users/michaelb/basement/skyscanner/forks/lightstep-tracer-python/lightstep/tracer.py", line 17, in <module>
    from .recorder import Recorder
  File "/Users/michaelb/basement/skyscanner/forks/lightstep-tracer-python/lightstep/recorder.py", line 18, in <module>
    from lightstep.http_converter import HttpConverter
  File "/Users/michaelb/basement/skyscanner/forks/lightstep-tracer-python/lightstep/http_converter.py", line 1, in <module>
    from lightstep.collector_pb2 import Auth, ReportRequest, Span, Reporter, KeyValue, Reference, SpanContext
  File "/Users/michaelb/basement/skyscanner/forks/lightstep-tracer-python/lightstep/collector_pb2.py", line 26, in <module>
    dependencies=[google_dot_protobuf_dot_timestamp__pb2.DESCRIPTOR,google_dot_api_dot_annotations__pb2.DESCRIPTOR,])
TypeError: __init__() got an unexpected keyword argument 'serialized_options'
>>>

For now we have forced dependencies to resolve to using protobuf==3.6.0 by adding it as a top level requirement in order to be able to import lightstep.
I would propose this be declared in this libraries setup.py instead though?

It's not guaranteed that successive calls to time.time return increasing values.
From https://docs.python.org/3/library/time.html#time.time:

While this function normally returns non-decreasing values, it can return a lower value than a previous call if the system clock has been set back between the two calls.

When this happens, the tracer crashes

22:19:35.163 ERROR benchmark.controller.python_client:   File "/usr/local/lib/python3.7/dist-packages/opentracing/scope.py", line 84, in __exit__
22:19:35.164 ERROR benchmark.controller.python_client:     self.close()
22:19:35.166 ERROR benchmark.controller.python_client:   File "/usr/local/lib/python3.7/dist-packages/opentracing/scope_managers/__init__.py", line 77, in close
22:19:35.167 ERROR benchmark.controller.python_client:     self.span.finish()
22:19:35.169 ERROR benchmark.controller.python_client:   File "/usr/local/lib/python3.7/dist-packages/basictracer/span.py", line 56, in finish
22:19:35.181 ERROR benchmark.controller.python_client:     self._tracer.record(self)
22:19:35.184 ERROR benchmark.controller.python_client:   File "/usr/local/lib/python3.7/dist-packages/basictracer/tracer.py", line 131, in record
22:19:35.187 ERROR benchmark.controller.python_client:     self.recorder.record_span(span)
22:19:35.192 ERROR benchmark.controller.python_client:   File "/usr/local/lib/python3.7/dist-packages/lightstep/recorder.py", line 150, in record_span
22:19:35.195 ERROR benchmark.controller.python_client:     span_record = self.converter.create_span_record(span, self.guid)
22:19:35.196 ERROR benchmark.controller.python_client:   File "/usr/local/lib/python3.7/dist-packages/lightstep/http_converter.py", line 48, in create_span_record
22:19:35.197 ERROR benchmark.controller.python_client:     duration_micros=int(util._time_to_micros(span.duration)))
22:19:35.198 ERROR benchmark.controller.python_client: ValueError: Value out of range: -35253

When an exception is raised as part of handling another exception, it gets a __cause__ attribute set, so that one can determine the cause.

Example:

try:
    raise ValueError("some value")
except ValueError:
    raise TypeError("some type")

If you execute this, it creates an exception with traceback that includes the cause:

$ python bar.py 
Traceback (most recent call last):
  File "bar.py", line 2, in <module>
    raise ValueError("some value")
ValueError: some value

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "bar.py", line 4, in <module>
    raise TypeError("some type")
TypeError: some type

I would expect the traceback in spans to also include the cause simply because that matches interpreter behavior when printing tracebacks and also matches how logging works. Unfortunately it doesn't.

Tracebacks are formatted in

Amending the example used earlier to print the traceback using the same formatting used in spans shows that the cause is missing

import sys
import traceback


try:
    try:
        raise ValueError("some value")
    except ValueError:
        raise TypeError("some type")
except Exception as exc:
    exc_value, exc_type, exc_tb = sys.exc_info()
    print("".join(traceback.format_tb(exc_tb)))

Output:

  File "foo.py", line 9, in <module>
    raise TypeError("some type")

It would be great, if this could be changed to include information about the cause.

Hola crew!

I would like to propose the tag 'lightstep.hostname' is appended to this library. It is present in the Go and JavaScript libraries, and would be useful such that we do not have to append that tag to the tracing constructor manually.

See:

• https://github.com/lightstep/lightstep-tracer-go/blob/eb40dd366f0f992fd08eb681d18c3898fcd68a64/options.go#L41
• https://github.com/lightstep/lightstep-tracer-javascript/blob/e9db0768cf7f3e063b738da479d734844092311e/src/imp/runtime_imp.js#L57

The practical value is to allow us to identify sick nodes for inspection. Whether it is the nodes themselves that are sick, or whether it is some condition about those nodes that is otherwise invisible to the trace (i.e. AZ, connection state or so) -- it's still useful to be able to see an identity to start the inspection.

Recently switched over to using the LightstepBinaryPropagator to get a python app playing nice with Envoy which uses Lightstep's C++ tracer with single key propagation on.

It seemed a bit silly to have to define the header constant ourselves since this library does not appear to define it anywhere. (Apologizes if I just missed it in my search)

Adding a constant similar to https://github.com/lightstep/lightstep-tracer-cpp/blob/master/src/tracer/propagation.cpp#L26 in propagation.py would be appreciated.

PR #68 has been merged to master and is waiting for a release that will support the latest version of basictracer.

The OpenTracing specification contains some standard fields for logging exceptions, and seems to encourage tracer implementations to extract information from these standard fields:

This scheme allows Tracer implementations to extract what information they need from the actual error object when it's available.

The opentracing Python library helpfully catches exceptions and adds the standard log fields for us.

In particular, the above code makes it clear that the intent of the stack field is to hold the actual traceback object itself.

But when I tie all this together and go look at a sample trace in the Lightstep UI... I get this:

stack: "<traceback object at 0x7fe002d01ac8>"

Which is not very useful to me! I would be better off manually doing something like this:

import traceback
[...]
catch Exception as e:
    span.set_tag('traceback', ''.join(traceback.format_tb(e.__traceback__)))

It seems to me that ideally Lightstep would be doing something here. What do you think?

Hello there 😄 Thank you for your contribution to open source. In your code I observed that you have locked the version of thrift between 0.10 and 0.12. Recently, I scanned one my projects with Snyk and was presented with the following vulnerability report. Are there any plans of upgrading thrift to version 13?

This issue was created to track and discuss the eventual 4.0.0 release.

This version will include Span propagation through ScopeManager, pointing the to 2.0.0rc1 and 3.0.0rc1 versions of opentracing and basictracer, respectively.

Current status

• The current effort lives in the carlos/ot-upgrade-2.0 branch.
• The lightstep tracer takes an optional scope_manager argument, falling back to a thread-local one (provided by basictracer).
• Examples have been ported to handle Python 3.4, and a Changelog draft will be added (@katiabazzi)

Questions

• Should we do a Release Candidates? For OT it makes perfect sense as many different vendors may want to try the API out in their own codebases, and validate it. For the Lightstep tracer, we may instead decide to keep the effort in a branch for now, and release when the final OT packages are ready. Else, do RC iterations as opentracing and basictracer do.
• It looks the officially supported Python versions are 2.7 and 3.4. My impression is that we should stick to those ones, and, whenever we want to support 3.6 (probably after the 4.0.0 release), do another iteration for it.
• Should we create maybe a v4.0.0 branch to hold the next release, instead of having the changes in carlos/ot-upgrade-2.0?

@katiabazzi @frenchfrywpepper @tedsuo