First pull request #2

isaachier · 2017-10-19T15:47:28Z

Entire C++ client here.

yurishkuro · 2017-10-19T23:38:06Z

.travis.yml

+      - MATRIX_EVAL="CC=clang-4.0 && CXX=clang++-4.0"
+
+branches:
+  only:


The matrix is mostly for later so I can use different compilers for testing. Also potentially for Mac building. Right now, even basic build is failing because of broken dependency I've been fixing. Once that is fixed, will introduce more builds.

Signed-off-by: Isaac Hier <ihier@uber.com>

Signed-off-by: Isaac Hier <isaachier@gmail.com>

…ransport Signed-off-by: Isaac Hier <isaachier@gmail.com>

Signed-off-by: Isaac Hier <isaachier@gmail.com>

codecov · 2017-10-25T12:20:36Z

Codecov Report

❗ No coverage uploaded for pull request base (master@b8f0e1d). Click here to learn what that means.
The diff coverage is 88.11%.

@@            Coverage Diff            @@
##             master       #2   +/-   ##
=========================================
  Coverage          ?   88.04%           
=========================================
  Files             ?       91           
  Lines             ?     2116           
  Branches          ?      185           
=========================================
  Hits              ?     1863           
  Misses            ?      189           
  Partials          ?       64

Impacted Files	Coverage Δ
src/jaegertracing/utils/UDPClient.h	`73.91% <ø> (ø)`
src/jaegertracing/utils/ErrorUtil.h	`100% <ø> (ø)`
src/jaegertracing/utils/YAML.h	`100% <ø> (ø)`
src/jaegertracing/utils/RateLimiter.h	`86.66% <ø> (ø)`
src/jaegertracing/utils/UDPClient.cpp	`100% <ø> (ø)`
src/jaegertracing/utils/HexParsing.h	`82.75% <ø> (ø)`
src/jaegertracing/reporters/Reporter.h	`100% <100%> (ø)`
src/jaegertracing/TraceID.cpp	`100% <100%> (ø)`
src/jaegertracing/reporters/NullReporter.h	`100% <100%> (ø)`
src/jaegertracing/metrics/Metrics.cpp	`100% <100%> (ø)`
... and 81 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8f0e1d...66de9d2. Read the comment docs.

Signed-off-by: Isaac Hier <isaachier@gmail.com>

isaachier · 2017-10-25T12:29:49Z

Finally got the coverage working. Not 100%, but getting there. Let me know what you think.

Signed-off-by: Isaac Hier <ihier@uber.com>

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Signed-off-by: Isaac Hier <ihier@uber.com>

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Signed-off-by: Isaac Hier <ihier@uber.com>

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Signed-off-by: Isaac Hier <ihier@uber.com>

black-adder · 2017-11-03T17:54:55Z

CMakeLists.txt

+    endforeach()
+endif()
+
+set(SRC


does Hunter automatically generate this for us or do we need to append any new file to this list when we create one?

black-adder · 2017-11-06T16:01:40Z

scripts/update-license.py

+
+CURRENT_YEAR = datetime.today().year
+
+MIT_LICENSE_BLOB = """Copyright (c) %d, Uber Technologies, Inc


is this reference to the MIT license still needed?

Good question. I think I took this from the Go client. Seems the script there deleted those lines in jaegertracing/jaeger-client-go@cddab85. Will do the same.

black-adder · 2017-11-06T16:02:49Z

src/jaegertracing/Constants.h.in

@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2017, Uber Technologies, Inc


this file is the only file using MIT license. I think it has to be regenerated?

Oh lol fair point. I think the .in screwed up the pattern. Will change the script to get this file too.

Signed-off-by: Isaac Hier <ihier@uber.com>

Signed-off-by: Isaac Hier <isaachier@gmail.com>

isaachier · 2017-11-08T13:23:21Z

@black-adder can you accept?

isaachier · 2017-11-08T20:08:31Z

Yay! Thanks!

`Span::isFinished()` returns true iff the span's duration is zero. So, if `steady_clock::now()` returns the same value at span creation as at `Finish()` (unlikely, but possible, especially in virtualized environments), the following scenario can happen: - `Span::FinishWithOptions` sets `_duration` to zero - and then calls `reportSpan`, which calls `RemoteReporter::report` - which acquires the reporter mutex and - adds the span to the reporter's `_queue` Later: - `RemoteReporter::sweepQueue` acquires the reporter mutex - `RemoteReporter::sweepQueue` makes a copy of the span (https://github.com/jaegertracing/jaeger-client-cpp/blob/7e9a135b362c47f7e4d42e0693b05b91a70e6888/src/jaegertracing/reporters/RemoteReporter.cpp#L95) - The span is serialized in `RemoteReporter::sendSpan` - The span (the copy made above) needs to be destroyed because the block in `sweepQueue` ends. - The span's destructor is called, which calls `FinishWithOptions` again - ... which sees that `_duration` is zero, so `isFinished()` returns false: https://github.com/jaegertracing/jaeger-client-cpp/blob/7e9a135b362c47f7e4d42e0693b05b91a70e6888/src/jaegertracing/Span.cpp#L94 - so it tries to call `reportSpan` again - which tries to acquire the reporter's mutex - but we're holding the lock in `sweepQueue`, so - deadlock. Relevant stack trace: ``` (gdb) bt (gdb) bt #0 __lll_lock_wait () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135 jaegertracing#1 0x00007fb09c722023 in __GI___pthread_mutex_lock (mutex=0x7fb088b1c1c0) at ../nptl/pthread_mutex_lock.c:78 jaegertracing#2 0x00007fb08860987b in jaegertracing::reporters::RemoteReporter::report(jaegertracing::Span const&) () from /usr/local/lib/libjaegertracing_plugin.so jaegertracing#3 0x00007fb0885b6506 in jaegertracing::Span::FinishWithOptions(opentracing::v2::FinishSpanOptions const&) [clone .localalias.361] () from /usr/local/lib/libjaegertracing_plugin.so jaegertracing#4 0x00007fb08860849a in jaegertracing::Span::~Span() [clone .constprop.253] () from /usr/local/lib/libjaegertracing_plugin.so jaegertracing#5 0x00007fb08860a8fa in jaegertracing::reporters::RemoteReporter::sweepQueue() () from /usr/local/lib/libjaegertracing_plugin.so jaegertracing#6 0x00007fb0886f2f5f in execute_native_thread_routine () from /usr/local/lib/libjaegertracing_plugin.so jaegertracing#7 0x00007fb09c71f6db in start_thread (arg=0x7fb087574700) at pthread_create.c:463 jaegertracing#8 0x00007fb0989ae88f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 ``` Fixed by enforcing a minimum span duration of 1 `SteadyClock` tick. Signed-off-by: Tudor Bosman <tudor@rockset.com>

* Fix deadlock if steady_clock::now() returns the same value twice `Span::isFinished()` returns true iff the span's duration is zero. So, if `steady_clock::now()` returns the same value at span creation as at `Finish()` (unlikely, but possible, especially in virtualized environments), the following scenario can happen: - `Span::FinishWithOptions` sets `_duration` to zero - and then calls `reportSpan`, which calls `RemoteReporter::report` - which acquires the reporter mutex and - adds the span to the reporter's `_queue` Later: - `RemoteReporter::sweepQueue` acquires the reporter mutex - `RemoteReporter::sweepQueue` makes a copy of the span (https://github.com/jaegertracing/jaeger-client-cpp/blob/7e9a135b362c47f7e4d42e0693b05b91a70e6888/src/jaegertracing/reporters/RemoteReporter.cpp#L95) - The span is serialized in `RemoteReporter::sendSpan` - The span (the copy made above) needs to be destroyed because the block in `sweepQueue` ends. - The span's destructor is called, which calls `FinishWithOptions` again - ... which sees that `_duration` is zero, so `isFinished()` returns false: https://github.com/jaegertracing/jaeger-client-cpp/blob/7e9a135b362c47f7e4d42e0693b05b91a70e6888/src/jaegertracing/Span.cpp#L94 - so it tries to call `reportSpan` again - which tries to acquire the reporter's mutex - but we're holding the lock in `sweepQueue`, so - deadlock. Relevant stack trace: ``` (gdb) bt (gdb) bt #0 __lll_lock_wait () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135 #1 0x00007fb09c722023 in __GI___pthread_mutex_lock (mutex=0x7fb088b1c1c0) at ../nptl/pthread_mutex_lock.c:78 #2 0x00007fb08860987b in jaegertracing::reporters::RemoteReporter::report(jaegertracing::Span const&) () from /usr/local/lib/libjaegertracing_plugin.so #3 0x00007fb0885b6506 in jaegertracing::Span::FinishWithOptions(opentracing::v2::FinishSpanOptions const&) [clone .localalias.361] () from /usr/local/lib/libjaegertracing_plugin.so #4 0x00007fb08860849a in jaegertracing::Span::~Span() [clone .constprop.253] () from /usr/local/lib/libjaegertracing_plugin.so #5 0x00007fb08860a8fa in jaegertracing::reporters::RemoteReporter::sweepQueue() () from /usr/local/lib/libjaegertracing_plugin.so #6 0x00007fb0886f2f5f in execute_native_thread_routine () from /usr/local/lib/libjaegertracing_plugin.so #7 0x00007fb09c71f6db in start_thread (arg=0x7fb087574700) at pthread_create.c:463 #8 0x00007fb0989ae88f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 ``` Fixed by enforcing a minimum span duration of 1 `SteadyClock` tick. Signed-off-by: Tudor Bosman <tudor@rockset.com> * fix typo Signed-off-by: Tudor Bosman <tudor@rockset.com>

yurishkuro reviewed Oct 19, 2017

View reviewed changes

isaachier force-pushed the master branch from f3b704c to a902570 Compare October 23, 2017 20:39

Basic C++ client implementation

16b5c84

Signed-off-by: Isaac Hier <ihier@uber.com>

isaachier force-pushed the master branch from a902570 to 16b5c84 Compare October 23, 2017 20:45

Fix deadlock and other bugs in Span

270fde6

Signed-off-by: Isaac Hier <isaachier@gmail.com>

isaachier force-pushed the master branch from d55f958 to 270fde6 Compare October 24, 2017 01:19

isaachier added 5 commits October 24, 2017 06:49

Add more unit tests, found issue in Tracer causing bad_alloc in UDP t…

3a92295

…ransport Signed-off-by: Isaac Hier <isaachier@gmail.com>

Fix function to find size of serialized Thrift messages

39ba2bb

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Fix random 64 bit ID generation

b8e9af7

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Fix UDPTransport

b7f3116

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Get coverage working

db64ef1

Signed-off-by: Isaac Hier <isaachier@gmail.com>

isaachier force-pushed the master branch from 17388ec to 860ce0b Compare October 25, 2017 12:27

Add coverage to README

0dedf96

Signed-off-by: Isaac Hier <isaachier@gmail.com>

isaachier force-pushed the master branch from 860ce0b to 0dedf96 Compare October 25, 2017 12:28

Fix UDP packet size for macOS and span references in Tracer::StartSpan

05d7da7

Signed-off-by: Isaac Hier <ihier@uber.com>

isaachier force-pushed the master branch from bbc04df to 05d7da7 Compare October 25, 2017 18:43

Add Finish to Span destructor

fe27f15

Signed-off-by: Isaac Hier <ihier@uber.com>

isaachier force-pushed the master branch from a02cdab to fe27f15 Compare October 25, 2017 19:10

Isaac Hier added 2 commits October 25, 2017 16:02

Simplify test cases for instantiating tracer

2b35a71

Signed-off-by: Isaac Hier <ihier@uber.com>

Undo UDP change for macOS

db3fc83

Signed-off-by: Isaac Hier <ihier@uber.com>

isaachier force-pushed the master branch from 8802419 to db3fc83 Compare October 25, 2017 20:17

isaachier and others added 2 commits October 26, 2017 07:34

Fix bugs and improve coverage in propagation

8d3c512

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Use Hunter cache repo

92fb4fb

Signed-off-by: Isaac Hier <ihier@uber.com>

isaachier force-pushed the master branch from 2727c01 to 92fb4fb Compare October 26, 2017 19:29

Add test case for HTTP headers propagation

ef609f8

Signed-off-by: Isaac Hier <ihier@uber.com>

isaachier force-pushed the master branch from 51fe805 to ef609f8 Compare October 26, 2017 19:49

Isaac Hier and others added 2 commits October 26, 2017 15:54

Fix default reporting port number

c120a49

Signed-off-by: Isaac Hier <ihier@uber.com>

Improve scripts, enable shared library build

04313b9

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Isaac Hier added 2 commits November 1, 2017 11:51

Formatting changes

6729d56

Signed-off-by: Isaac Hier <ihier@uber.com>

Add 0.0.3 to CMakeLists

fe24c27

Signed-off-by: Isaac Hier <ihier@uber.com>

isaachier force-pushed the master branch from cd7f8a8 to fcce61c Compare November 1, 2017 18:02

Fix flush in RemoteReporter

6b4d803

Signed-off-by: Isaac Hier <ihier@uber.com>

isaachier force-pushed the master branch from fcce61c to 6b4d803 Compare November 1, 2017 18:24

Fix bug with system vs. steady time

b2b4420

Signed-off-by: Isaac Hier <isaachier@gmail.com>

isaachier force-pushed the master branch 2 times, most recently from b021847 to 383c917 Compare November 2, 2017 23:50

Add boost::regex to support gcc 4.8

7ad0e51

Signed-off-by: Isaac Hier <isaachier@gmail.com>

isaachier force-pushed the master branch from 383c917 to 7ad0e51 Compare November 2, 2017 23:53

isaachier and others added 3 commits November 2, 2017 20:20

Release v0.0.5

93d1cc3

Signed-off-by: Isaac Hier <isaachier@gmail.com>

Change macros to namespace aliases

183cf36

Signed-off-by: Isaac Hier <ihier@uber.com>

cmakelint fixes

bf09ad5

Signed-off-by: Isaac Hier <ihier@uber.com>

black-adder approved these changes Nov 6, 2017

View reviewed changes

Isaac Hier and others added 4 commits November 6, 2017 13:00

Automatically find Hunter installation directory for YCM

4d723fe

Signed-off-by: Isaac Hier <ihier@uber.com>

Fix licenses

6e43d75

Signed-off-by: Isaac Hier <ihier@uber.com>

Formatting changes

05be022

Signed-off-by: Isaac Hier <ihier@uber.com>

Catch exceptions in threads, fix YCM config

66de9d2

Signed-off-by: Isaac Hier <isaachier@gmail.com>

black-adder merged commit dd369e2 into jaegertracing:master Nov 8, 2017

ankit-varma10 mentioned this pull request Apr 2, 2018

Unhandled exception - Abort raised when write to the jaeger-agent fails #72

Closed

SEJeff mentioned this pull request May 15, 2018

Add support for overriding hostname and ip fields #105

Closed

dengguojie mentioned this pull request Dec 7, 2018

invoke set_tag from lua core in jaegertracing::Span::SetTag(opentracing::v2::string_view, opentracing::v2::Value const&) () #139

Closed

DavitSo mentioned this pull request Feb 8, 2019

StartSpan crashes #148

Closed

EasonFeng5870 mentioned this pull request Jul 7, 2020

cmake is too slow #232

Closed

mirage111 mentioned this pull request Sep 14, 2020

Client hangs when sending spans to collector directly(without agent) during Jaeger server restart #244

Closed

jcx-vmw mentioned this pull request Oct 26, 2020

data race observed for jaeger span finish operation #251

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First pull request #2

First pull request #2

isaachier commented Oct 19, 2017

yurishkuro Oct 19, 2017

isaachier Oct 20, 2017

codecov bot commented Oct 25, 2017 •

edited

Loading

isaachier commented Oct 25, 2017

black-adder Nov 3, 2017

black-adder Nov 6, 2017

isaachier Nov 6, 2017

black-adder Nov 6, 2017

isaachier Nov 6, 2017

isaachier commented Nov 8, 2017

isaachier commented Nov 8, 2017


		CURRENT_YEAR = datetime.today().year

		MIT_LICENSE_BLOB = """Copyright (c) %d, Uber Technologies, Inc

		@@ -0,0 +1,49 @@
		/*
		* Copyright (c) 2017, Uber Technologies, Inc

First pull request #2

First pull request #2

Conversation

isaachier commented Oct 19, 2017

yurishkuro Oct 19, 2017

Choose a reason for hiding this comment

isaachier Oct 20, 2017

Choose a reason for hiding this comment

codecov bot commented Oct 25, 2017 • edited Loading

Codecov Report

isaachier commented Oct 25, 2017

black-adder Nov 3, 2017

Choose a reason for hiding this comment

black-adder Nov 6, 2017

Choose a reason for hiding this comment

isaachier Nov 6, 2017

Choose a reason for hiding this comment

black-adder Nov 6, 2017

Choose a reason for hiding this comment

isaachier Nov 6, 2017

Choose a reason for hiding this comment

isaachier commented Nov 8, 2017

isaachier commented Nov 8, 2017

codecov bot commented Oct 25, 2017 •

edited

Loading