extensions: add OpenTelemetry tracer extension for exporting OTLP traces #20281

AlexanderEllis · 2022-03-10T00:37:36Z

Commit Message: Add OpenTelemetry Tracer Extension for exporting OTLP traces
Additional Description: This adds a new OpenTelemetry Tracer that exports OTLP traces via Envoy's async gRPC mechanism. It includes traceparent header parsing, buffering, and basic stats.
Risk Level: Medium (new extension that is not enabled)
Testing: Added unit tests for the new tracer and performed manual testing with the OpenTelemetry Collector
Docs Changes: TODO
Release Notes: opentelemetry: add OpenTelemetry tracer extension
Platform Specific Features: N/A
Part of #9958

Possible fix for #20641

Signed-off-by: Alex Ellis <alexellis@google.com>

repokitteh-read-only · 2022-03-10T00:37:39Z

Hi @AlexanderEllis, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #20281 was opened by AlexanderEllis.

see: more, trace.

repokitteh-read-only · 2022-03-10T00:37:40Z

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #20281 was opened by AlexanderEllis.

see: more, trace.

repokitteh-read-only · 2022-03-10T00:37:48Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @lizan
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @phlax

🐱

Caused by: #20281 was opened by AlexanderEllis.

see: more, trace.

source/extensions/tracers/opentelemetry/tracer.cc

moderation · 2022-03-10T11:40:33Z

Fantastic to see this! As part of this change can you update to the latest version of OpenTelemetry. We are currently on version 0.11.0 - the latest is 0.14.0

https://github.com/envoyproxy/envoy/blob/main/api/bazel/repository_locations.bzl#L99-L109

Signed-off-by: Alex Ellis <alexellis@google.com>

Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>

yanavlasov · 2022-06-22T13:18:28Z

/retest

repokitteh-read-only · 2022-06-22T13:18:33Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #20281 (comment) was created by @yanavlasov.

see: more, trace.

mattklein123

Hurray!

phlax

lgtm, thanks so much @AlexanderEllis for staying the journey

landing cos i was asked, and im drunk enuf 8/

cc @moderation

K-Prabha · 2022-06-22T21:24:28Z

🎉 Thank you @AlexanderEllis for the persistence and thank you community for helping this push thru. Much appreciated

phlax · 2022-06-22T22:14:34Z

eek @AlexanderEllis looks like i broke main landing this - i reckon minor hiccup - any ideas on how we can fix forward ?

unknown file: Failure
C++ exception with description "Protobuf message (type opentelemetry.proto.collector.logs.v1.ExportLogsServiceRequest reason INVALID_ARGUMENT:(resource_logs.instrumentation_library_logs[0]) logs: Cannot find field.) has unknown fields" thrown in the test body.
test/extensions/access_loggers/open_telemetry/grpc_access_log_impl_test.cc:51: Failure
Actual function call count doesn't match EXPECT_CALL(*async_client, startRaw(_, _, _, _))...
         Expected: to be called once
           Actual: never called - unsatisfied and active

i guess some change in config

https://dev.azure.com/cncf/envoy/_build/results?buildId=111237&view=logs&j=8c169225-0ae8-53bd-947f-07cb81846cb5&t=6b0ace90-28b2-51d9-2951-b1fd35e67dec&l=1233

…OTLP traces (envoyproxy#20281)" This reverts commit f7a8db0. Signed-off-by: Ryan Northey <ryan@synca.io>

AlexanderEllis · 2022-06-22T22:59:16Z

Ah thanks for calling that out @phlax . I see there's a PR to revert already, which SGTM.

As I mentioned there, I believe I have a quick forward fix in #21842 if that's easier — looks like it was a new test (as fresh as this morning, heh) with a field name that the updated OTel library had changed (and I had updated the other tests in this PR).

Also happy to wait and revert the revert if you want too 👍

phlax · 2022-06-22T23:22:30Z

ill let you and @ggreenway figure it out - defo preferable to fix forward imho - opened the revert as landing broke main

moderation · 2022-06-24T16:28:28Z

@AlexanderEllis fantastic to see this land. I've been doing some testing sending from a bunch of Envoy's to Uptrace. On the OpenTelemetry Collector I have debug logging installed and it doesn't look like the traces are carrying any information. Similarly on the Uptrace side the spans are empty.

Resource SchemaURL: 
ScopeSpans #0
ScopeSpans SchemaURL: 
InstrumentationScope  
Span #0
    Trace ID       : bec0620d406d5a2c63bede10ee2e29d6
    Parent ID      : 0cc2228c878e7a98
    ID             : d29c47da16a3c7f4
    Name           : router sync_service egress
    Kind           : SPAN_KIND_CLIENT
    Start time     : 2022-06-24 16:18:05.707695875 +0000 UTC
    End time       : 2022-06-24 16:18:06.370910412 +0000 UTC
    Status code    : STATUS_CODE_UNSET
    Status message : 
Span #1
    Trace ID       : bec0620d406d5a2c63bede10ee2e29d6
    Parent ID      : 
    ID             : 0cc2228c878e7a98
    Name           : egress xxxxx.xxxx.xx:9006
    Kind           : SPAN_KIND_CLIENT
    Start time     : 2022-06-24 16:18:05.707584509 +0000 UTC
    End time       : 2022-06-24 16:18:06.370936071 +0000 UTC
    Status code    : STATUS_CODE_UNSET
    Status message :

In contrast my old Lightstep setup carried a bunch of info

Any idea on what is happening? Am I missing a config?

AlexanderEllis · 2022-06-24T20:50:30Z

Nothing wrong on your end! I'll be following this up with a few more PRs to flesh out the tracer, one of which will be the attribute setting correctly in the span (stubbed out span.setTag for now, but adding that should allow these attributes to be set correctly in http_tracer_impl.cc). I'll also add an example setup with the collector and a few docs as well.

Should hopefully be smooth(er) sailing now that the main PR is merged :)

Edit: for posterity and a PSA, I fleshed out a few of the TODOs in #9958 (comment)

…ces (envoyproxy#20281) Signed-off-by: Alex Ellis <ellisonjtk@gmail.com> Signed-off-by: Amila Senadheera <amila.15@cse.mrt.ac.lk>

nareddyt · 2022-12-14T22:57:32Z

https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/trace/v3/opentelemetry.proto mentions the following

This extension is work-in-progress. Functionality is incomplete and it is not intended for production use. This extension has an unknown security posture and should only be used in deployments where both the downstream and upstream are trusted.

Are there any tangible steps to get this extension ready for production use? Is it a matter of adhering to Envoy's extension guidelines, or does it also involve changes to the OpenTelemetry libraries?

https://github.com/envoyproxy/envoy/blob/main/EXTENSION_POLICY.md#extension-stability-and-security-posture

htuch · 2023-01-06T04:45:42Z

@itamarkam @yanavlasov can you address #20281 (comment)? Thanks.

WojciechKuk · 2023-01-09T14:02:08Z

source/extensions/tracers/opentelemetry/tracer.cc

+  // Create an Tracers::OpenTelemetry::Span class that will contain the OTel span.
+  Span new_span = Span(config, operation_name, start_time, time_source_, *this);
+  new_span.setSampled(tracing_decision.traced);
+  uint64_t trace_id_high = random_.random();


Is it possible to make it "less random"? i.e.
hex timestamp (32b) + hex ms (16b) + hex instance's IP (32b) + some internal counter?

itamarkam · 2023-01-09T14:08:26Z

itamark

@AlexanderEllis Any thoughts here?

Alex Ellis added 16 commits March 9, 2022 19:25

Add build rule for OpenTelemetry trace proto

c52b955

Signed-off-by: Alex Ellis <alexellis@google.com>

Added OTLP gRPC trace exporter client

4d4f4b8

Signed-off-by: Alex Ellis <alexellis@google.com>

Add OpenTelemetry Tracer config proto

2ca1521

Signed-off-by: Alex Ellis <alexellis@google.com>

Add config for otlp tracer

db054ce

Signed-off-by: Alex Ellis <alexellis@google.com>

Add Driver class and the start of its methods

a4391b0

Signed-off-by: Alex Ellis <alexellis@google.com>

Add tracer, span definition, and span context

5a15738

Signed-off-by: Alex Ellis <alexellis@google.com>

Add initial tests for config and tracer_impl

183a0c3

Signed-off-by: Alex Ellis <alexellis@google.com>

Add OpenTelemetry tracer extension

3001b34

Signed-off-by: Alex Ellis <alexellis@google.com>

Add full exporting test and tie in runtime and gRPC client

1c4e3df

Signed-off-by: Alex Ellis <alexellis@google.com>

Fix parsing trace-flags for sampled decision

6f5d636

Signed-off-by: Alex Ellis <alexellis@google.com>

Break out span_context_extractor into separate file with its own tests

173be07

Signed-off-by: Alex Ellis <alexellis@google.com>

Add span flushing timer and tests for it and the min_flush_spans

348bf8f

Signed-off-by: Alex Ellis <alexellis@google.com>

Add propeer Span::spanChild implementation and add test

04905bc

Signed-off-by: Alex Ellis <alexellis@google.com>

Add counters for spans_sent and timer_flushed

f4be107

Signed-off-by: Alex Ellis <alexellis@google.com>

Clean up code and remove unused variables

d372f9d

Signed-off-by: Alex Ellis <alexellis@google.com>

Ran check_format

bf373d6

Signed-off-by: Alex Ellis <alexellis@google.com>

repokitteh-read-only bot added api deps Approval required for changes to Envoy's external dependencies labels Mar 10, 2022

repokitteh-read-only bot assigned lizan and phlax Mar 10, 2022

phlax reviewed Mar 10, 2022

View reviewed changes

source/extensions/tracers/opentelemetry/tracer.cc Outdated Show resolved Hide resolved

Alex Ellis added 5 commits March 10, 2022 17:36

Add OpenTelemetry words to dictionary

b971f90

Signed-off-by: Alex Ellis <alexellis@google.com>

Add extension metadata

5dba33a

Signed-off-by: Alex Ellis <alexellis@google.com>

End files with newlines

bfaf415

Signed-off-by: Alex Ellis <alexellis@google.com>

Bump OpenTelemetry Proto to 0.14.0

a91f927

Signed-off-by: Alex Ellis <alexellis@google.com>

Remove trailing whitespace

ee559c8

Signed-off-by: Alex Ellis <alexellis@google.com>

Update span context to return const ref to attributes

d9b1257

Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>

repokitteh-read-only bot removed the waiting label Jun 22, 2022

yanavlasov approved these changes Jun 22, 2022

View reviewed changes

mattklein123 approved these changes Jun 22, 2022

View reviewed changes

repokitteh-read-only bot removed api deps Approval required for changes to Envoy's external dependencies labels Jun 22, 2022

phlax approved these changes Jun 22, 2022

View reviewed changes

phlax merged commit f7a8db0 into envoyproxy:main Jun 22, 2022

phlax added a commit to phlax/envoy that referenced this pull request Jun 22, 2022

Revert "extensions: add OpenTelemetry tracer extension for exporting …

75e6958

…OTLP traces (envoyproxy#20281)" This reverts commit f7a8db0. Signed-off-by: Ryan Northey <ryan@synca.io>

phlax mentioned this pull request Jun 22, 2022

Revert "extensions: add OpenTelemetry tracer extension for exporting … #21840

Closed

AlexanderEllis mentioned this pull request Jun 25, 2022

tracing: Transition to OpenTelemetry from OpenTracing and OpenCensus #9958

Closed

zirain mentioned this pull request Jun 27, 2022

Automator: update istio@ test reference istio/istio.io#11496

Closed

AlexanderEllis deleted the add-opentelemetry-tracer branch July 7, 2022 01:01

douglas-reid mentioned this pull request Jul 20, 2022

FR: Handle transition from Lightstep to OpenTelemetry istio/istio#40027

Closed

moderation mentioned this pull request Jul 26, 2022

[WIP] deps: Bump com_github_msgpack_msgpack_c -> 4.0.2 #18476

Closed

yangyy93 mentioned this pull request Dec 8, 2022

design: add tracing design doc projectcontour/contour#3802

Closed

WojciechKuk reviewed Jan 9, 2023

View reviewed changes

joaopgrassi mentioned this pull request Nov 20, 2023

opentelemetry tracer: Move to alpha stability? #30976

Closed

extensions: add OpenTelemetry tracer extension for exporting OTLP traces #20281

extensions: add OpenTelemetry tracer extension for exporting OTLP traces #20281

Uh oh!

Conversation

AlexanderEllis commented Mar 10, 2022 • edited by phlax Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Mar 10, 2022

Uh oh!

repokitteh-read-only bot commented Mar 10, 2022

Uh oh!

repokitteh-read-only bot commented Mar 10, 2022

Uh oh!

Uh oh!

moderation commented Mar 10, 2022

Uh oh!

yanavlasov commented Jun 22, 2022

Uh oh!

repokitteh-read-only bot commented Jun 22, 2022

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

phlax left a comment

Choose a reason for hiding this comment

Uh oh!

K-Prabha commented Jun 22, 2022

Uh oh!

phlax commented Jun 22, 2022

Uh oh!

AlexanderEllis commented Jun 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phlax commented Jun 22, 2022

Uh oh!

moderation commented Jun 24, 2022

Uh oh!

AlexanderEllis commented Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nareddyt commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

htuch commented Jan 6, 2023

Uh oh!

WojciechKuk Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

itamarkam commented Jan 9, 2023

Uh oh!

Uh oh!

AlexanderEllis commented Mar 10, 2022 •

edited by phlax

Loading

AlexanderEllis commented Jun 22, 2022 •

edited

Loading

AlexanderEllis commented Jun 24, 2022 •

edited

Loading

nareddyt commented Dec 14, 2022 •

edited

Loading