tracing: Add B3 support in OpenCensus driver. by g-easy · Pull Request #7800 · envoyproxy/envoy

g-easy · 2019-08-01T07:16:57Z

This makes it possible to propagate, generate, and translate
B3-format trace context headers.

Signed-off-by: Emil Mikulic g-easy@users.noreply.github.com

This makes it possible to propagate, generate, and translate B3-format trace context headers. Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

These come from tracing.grpc.pb.h which is autogenerated by the protobuf compiler. Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

repokitteh-read-only · 2019-08-01T08:41:58Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7800 was synchronize by g-easy.

see: more, trace.

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

repokitteh-read-only · 2019-08-01T09:04:24Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7800 was synchronize by g-easy.

see: more, trace.

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

repokitteh-read-only · 2019-08-02T07:14:47Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7800 was synchronize by g-easy.

see: more, trace.

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

repokitteh-read-only · 2019-08-06T04:55:59Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7800 was synchronize by g-easy.

see: more, trace.

htuch

LGTM modulo two small readability improvements.
/wait

source/extensions/tracers/opencensus/opencensus_tracer_impl.cc

test/extensions/tracers/opencensus/tracer_test.cc

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

repokitteh-read-only · 2019-08-07T06:07:17Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7800 was synchronize by g-easy.

see: more, trace.

g-easy · 2019-08-07T06:07:32Z

PTAL. :)

htuch

LGTM modulo last tiny nits.
/wait

test/extensions/tracers/opencensus/tracer_test.cc

htuch · 2019-08-07T13:21:50Z

test/extensions/tracers/opencensus/tracer_test.cc

+  registerSpanCatcher();
+  OpenCensusConfig oc_config;
+  NiceMock<LocalInfo::MockLocalInfo> local_info;
+  oc_config.add_incoming_trace_context(OpenCensusConfig::NONE);


Arguably this could be set in the TEST and be a bit narrower.

I would prefer to leave this as-is:

Reduces code duplication in TESTs.

Acts as a smoke-test for incoming headers that aren't found.

As an alternative: how about if I make TEST pass the incoming trace context type to testIncomingHeaders but still keep this code and all the outgoing contexts (because I want to test translation) in the latter?

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

repokitteh-read-only · 2019-08-08T01:48:20Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7800 was synchronize by g-easy.

see: more, trace.

htuch

Thanks!

htuch · 2019-08-08T16:36:28Z

/azp retest

azure-pipelines · 2019-08-08T16:36:32Z

Command 'retest' is not supported by Azure Pipelines. Supported commands help: Get descriptions, examples and documentation about supported commands Example: help "command_name" list: List all pipelines for this repository using a comment. Example: "list" run: Run all pipelines or a specific pipeline for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify a pipeline to run. Example: "run" or "run pipeline_name" where: Report back the Azure DevOps orgs that are related to this repository and org Example: "where" See additional documentation.

htuch · 2019-08-08T16:37:13Z

/retest

repokitteh-read-only · 2019-08-08T16:37:16Z

🤷‍♀️ nothing to rebuild.

🐱

Caused by: a #7800 (comment) was created by @htuch.

see: more, trace.

htuch · 2019-08-08T16:37:29Z

/azp run

azure-pipelines · 2019-08-08T16:37:40Z

Azure Pipelines successfully started running 2 pipeline(s).

g-easy · 2019-08-12T05:57:09Z

Friendly ping. :)

g-easy added 2 commits August 1, 2019 17:09

tracing: Add B3 support in OpenCensus driver.

9603803

This makes it possible to propagate, generate, and translate B3-format trace context headers. Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Don't error on unused-parameter warnings.

c5c53fa

These come from tracing.grpc.pb.h which is autogenerated by the protobuf compiler. Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Don't use deprecated API.

01ba657

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Merge branch 'master' of https://github.com/envoyproxy/envoy into b3

59db416

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Merge remote-tracking branch 'origin/master' into b3

bfa98d8

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

jmarantz requested a review from htuch August 6, 2019 14:35

jmarantz assigned htuch Aug 6, 2019

htuch suggested changes Aug 7, 2019

View reviewed changes

source/extensions/tracers/opencensus/opencensus_tracer_impl.cc Show resolved Hide resolved

test/extensions/tracers/opencensus/tracer_test.cc Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Aug 7, 2019

g-easy added 4 commits August 7, 2019 15:42

Merge remote-tracking branch 'origin/master' into b3

1d061d3

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Split PropagateTraceContext into per-header tests.

a5d2166

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Add comment about X-B3-Flags.

4e4f455

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Change test_outgoing helper to gunit MATCHER.

914a953

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

repokitteh-read-only bot removed the waiting label Aug 7, 2019

htuch suggested changes Aug 7, 2019

View reviewed changes

repokitteh-read-only bot added the waiting label Aug 7, 2019

g-easy added 2 commits August 8, 2019 11:31

Merge remote-tracking branch 'origin/master' into b3

f6da9e0

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Fix nits.

16d1ddc

Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

repokitteh-read-only bot removed the waiting label Aug 8, 2019

htuch approved these changes Aug 8, 2019

View reviewed changes

mattklein123 merged commit c3a7531 into envoyproxy:master Aug 12, 2019

g-easy deleted the b3 branch August 13, 2019 05:34

Conversation

g-easy commented Aug 1, 2019

Uh oh!

repokitteh-read-only bot commented Aug 1, 2019

Uh oh!

repokitteh-read-only bot commented Aug 1, 2019

Uh oh!

repokitteh-read-only bot commented Aug 2, 2019

Uh oh!

repokitteh-read-only bot commented Aug 6, 2019

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

repokitteh-read-only bot commented Aug 7, 2019

Uh oh!

g-easy commented Aug 7, 2019

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

htuch Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

g-easy Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

repokitteh-read-only bot commented Aug 8, 2019

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

htuch commented Aug 8, 2019

Uh oh!

azure-pipelines bot commented Aug 8, 2019

Uh oh!

htuch commented Aug 8, 2019

Uh oh!

repokitteh-read-only bot commented Aug 8, 2019

Uh oh!

htuch commented Aug 8, 2019

Uh oh!

azure-pipelines bot commented Aug 8, 2019

Uh oh!

g-easy commented Aug 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants