Add initial OpenCensus tracing support. #5387

g-easy · 2018-12-21T04:12:19Z

Addresses #2456

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

Otherwise urls regex matches "com_github_curl = dict" Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

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

stale · 2018-12-29T12:03:04Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

stale · 2019-01-05T16:19:49Z

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

htuch · 2019-01-07T00:18:21Z

Reopening, tagging myself for initial review @htuch

htuch

Looks good, this is heading in the right direction, seems a very clean integration. Some comments to get started, will review the actual tracer extension implementation next.

api/envoy/config/trace/v2/trace.proto

bazel/external/curl.BUILD

source/extensions/tracers/opencensus/config.cc

source/extensions/tracers/opencensus/opencensus_tracer_impl.cc

tools/check_repositories.sh

htuch · 2019-01-07T08:25:13Z

@goaway can you also look at the specific tracer extension integration?

stale · 2019-01-16T07:12:48Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

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

g-easy · 2019-06-04T18:21:43Z

Thanks for the review comments, @lizan!

lizan

LGTM, defer to @htuch.

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

source/extensions/tracers/opencensus/opencensus_tracer_impl.cc

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

htuch

LGTM modulo a few comments, ready to land when they are resolved. Thanks for all the work in tying this together!
/wait

source/common/common/base64.h

bazel/foreign_cc/io_opencensus_cpp.patch

bazel/repositories.bzl

source/extensions/tracers/opencensus/opencensus_tracer_impl.cc

g-easy · 2019-06-05T18:39:40Z

bazel/foreign_cc/io_opencensus_cpp.patch

+     visibility = ["//visibility:public"],
+     deps = [
+-        "//google/devtools/cloudtrace/v2:tracing_proto",
+        "@googleapis//:tracing_proto",


[in future we can drop this patch] except for this bit. We have to use a single tracing_proto target to avoid ODR (with lightstep).

The long term fix for this is to upgrade googleapis, but first we need to add Python language support for googleapis, but first we need an updated py_proto_library rule in bazel, per:

envoy/api/bazel/api_build_system.bzl

Lines 23 to 27 in 0739cd6

# TODO(htuch): Convert this to native py_proto_library once

# https://github.com/bazelbuild/bazel/issues/3935 and/or

# https://github.com/bazelbuild/bazel/issues/2626 are resolved.

def api_py_proto_library(name, srcs = [], deps = [], has_services = 0):

py_proto_library(

See also: grpc/grpc#19183

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

Only parent context can be invalid. Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

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

htuch

Thanks @g-easy! This was epic, very useful capability to land.

moderation · 2019-06-06T13:43:08Z

@g-easy this is awesome and with some preliminary testing I was able to simultaneously send traces to Stackdriver and a local Jaeger. Question re: curl version - is there any reason you've used 7.63.0? There have been a number of CVE's addressed in 7.64 and 7.65. Are there features of OpenCensus that require 7.63.0 or can I raise a PR to bump to the latest curl?

lizan · 2019-06-06T18:00:06Z

@moderation I think it is just the latest version when this PR is created. Feel free to open a PR to bump it. A quick skim for the CVEs I don't think any affect this use case though.

g-easy · 2019-06-06T18:09:07Z

@lizan is right. Please go ahead and upgrade curl. In OpenCensus, only the Zipkin exporter uses it.

g-easy added 2 commits December 21, 2018 15:05

Add initial OpenCensus tracing support.

e517ebf

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

tools/check_repositories.sh: make regexes look for word start.

de6b334

Otherwise urls regex matches "com_github_curl = dict" Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

g-easy mentioned this pull request Dec 21, 2018

Integrate with OpenCensus stats collection and distributed tracing framework #2456

Closed

Fix clang_tidy error.

0c17e0a

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

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 29, 2018

stale bot closed this Jan 5, 2019

htuch removed the stale stalebot believes this issue/PR has not been touched recently label Jan 7, 2019

htuch reopened this Jan 7, 2019

htuch self-requested a review January 7, 2019 00:18

htuch suggested changes Jan 7, 2019

View reviewed changes

htuch assigned htuch and goaway Jan 7, 2019

htuch mentioned this pull request Jan 9, 2019

grpc: Add AWS IAM grpc credentials extension #5546

Closed

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 16, 2019

g-easy added 3 commits January 18, 2019 13:40

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

461d9b0

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

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

0bcaf23

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

Fix mismerge.

ec0f25d

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

stale bot removed stale stalebot believes this issue/PR has not been touched recently labels Jan 22, 2019

g-easy added 6 commits January 22, 2019 15:20

Remove debugging printfs.

2f0d285

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

Add explicit break.

d96f105

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

Format fix.

0e28684

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

Add opencensus:config_test.

e54cadb

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

Don't apply empty TraceConfig.

1081de3

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

Add opencensus:tracer_test.

471dc28

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

Review comments.

f587ee8

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

lizan previously approved these changes Jun 4, 2019

View reviewed changes

grpc-trace-bin needs to be base64 encoded/decoded without padding.

b87a8e1

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

g-easy dismissed lizan’s stale review via b87a8e1 June 4, 2019 22:41

g-easy added 2 commits June 4, 2019 15:49

Finish todo: tests for grpc-trace-bin.

9ca882f

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

Spellcheck doesn't like the comment.

c8fc2c9

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

lizan reviewed Jun 4, 2019

View reviewed changes

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

g-easy added 2 commits June 4, 2019 16:45

Add decode_without_padding().

03df4ad

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

Use decode_without_padding().

2d9e96b

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

htuch suggested changes Jun 5, 2019

View reviewed changes

source/common/common/base64.h Outdated Show resolved Hide resolved

bazel/foreign_cc/io_opencensus_cpp.patch Show resolved Hide resolved

bazel/repositories.bzl Show resolved Hide resolved

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

repokitteh-read-only bot added the waiting label Jun 5, 2019

g-easy commented Jun 5, 2019

View reviewed changes

g-easy added 5 commits June 5, 2019 11:51

decodeWithoutPadding rename + take string_view

9d89449

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

Make ALWAYS_ON fallthrough.

4e3245c

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

Current span context is always valid.

4122637

Only parent context can be invalid. Signed-off-by: Emil Mikulic <g-easy@users.noreply.github.com>

Factor out SamplerTestHelper and increase coverage.

3b9ced4

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

speling

7385873

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

repokitteh-read-only bot removed the waiting label Jun 5, 2019

htuch approved these changes Jun 5, 2019

View reviewed changes

htuch merged commit 350a35f into envoyproxy:master Jun 5, 2019

g-easy deleted the oc branch June 6, 2019 00:01

lambdai mentioned this pull request Jun 7, 2019

Add opencensus library istio/proxy#2258

Closed

cce mentioned this pull request Jun 7, 2019

support of open tracing in clickhouse ClickHouse/ClickHouse#5182

Closed

mandarjog mentioned this pull request Jun 24, 2019

Expose stackdriver tracing for istio-proxy istio/istio#15117

Closed

objectiser mentioned this pull request Jun 28, 2019

tracing: General mechanism to avoid tracing health check requests #7422

Open

troshko111 mentioned this pull request Sep 4, 2019

Compile curl dependency with c-ares support #8140

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial OpenCensus tracing support. #5387

Add initial OpenCensus tracing support. #5387

g-easy commented Dec 21, 2018

stale bot commented Dec 29, 2018

stale bot commented Jan 5, 2019

htuch commented Jan 7, 2019

htuch left a comment

htuch commented Jan 7, 2019

stale bot commented Jan 16, 2019

g-easy commented Jun 4, 2019

lizan left a comment

htuch left a comment

g-easy Jun 5, 2019 •

edited

Loading

g-easy Jun 5, 2019

htuch left a comment

moderation commented Jun 6, 2019

lizan commented Jun 6, 2019

g-easy commented Jun 6, 2019

	# TODO(htuch): Convert this to native py_proto_library once
	# https://github.com/bazelbuild/bazel/issues/3935 and/or
	# https://github.com/bazelbuild/bazel/issues/2626 are resolved.
	def api_py_proto_library(name, srcs = [], deps = [], has_services = 0):
	py_proto_library(

Add initial OpenCensus tracing support. #5387

Add initial OpenCensus tracing support. #5387

Conversation

g-easy commented Dec 21, 2018

stale bot commented Dec 29, 2018

stale bot commented Jan 5, 2019

htuch commented Jan 7, 2019

htuch left a comment

Choose a reason for hiding this comment

htuch commented Jan 7, 2019

stale bot commented Jan 16, 2019

g-easy commented Jun 4, 2019

lizan left a comment

Choose a reason for hiding this comment

htuch left a comment

Choose a reason for hiding this comment

g-easy Jun 5, 2019 • edited Loading

Choose a reason for hiding this comment

g-easy Jun 5, 2019

Choose a reason for hiding this comment

htuch left a comment

Choose a reason for hiding this comment

moderation commented Jun 6, 2019

lizan commented Jun 6, 2019

g-easy commented Jun 6, 2019

g-easy Jun 5, 2019 •

edited

Loading