Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opentelemetry tracer: add OTLP/HTTP exporter #29207

Merged

Conversation

joaopgrassi
Copy link
Contributor

@joaopgrassi joaopgrassi commented Aug 23, 2023

Commit Message:
Add OTLP/HTTP exporter to OpenTelemetry Tracer

Co-authored-by: Alex Ellis ellisonjtk@gmail.com

Additional Description: This PR refactors the OpenTelemetry tracer, allowing it to export OTLP data via HTTP. It refactors the existing gRPC exporter into a general exporter, so the functionality can be shared between the gRPC and the new HTTP exporters. The HTTP exporter is relatively simple: it accesses the thread local cluster for the configured cluster, then sends the OTLP request.

Risk Level: Low (new, optional functionality for existing tracing extension)

Testing: Unit test and manual tests performed, exporting spans to a local OTel collector and a public/external back-end via OTLP/HTTP. Also performed tests exporting to a local OTel collector via gRPC to ensure the existing feature is unaffected.

Fixes #26352

Note: This is a continuation of #28454 as previously discussed and agreed with @AlexanderEllis and @htuch.

For posterity, here's how a "full" configuration looks like:

tracing:
  provider:
    name: envoy.tracers.opentelemetry
    typed_config:
      "@type": type.googleapis.com/envoy.config.trace.v3.OpenTelemetryConfig
      service_name: envoy-HTTP-exporter
      http_config:
        cluster_name: my_backend
        traces_path: "/api/otlp/v1/traces"
        hostname: "my-backend.com"
        headers:
          - key: "Authorization"
            value: "auth-token"
          - key: "x-custom-header"
            value: "something-custom"
        timeout: 5s

AlexanderEllis and others added 16 commits August 23, 2023 10:08
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Alex Ellis <ellisonjtk@gmail.com>
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
@repokitteh-read-only
Copy link

Hi @joaopgrassi, 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: #29207 was opened by joaopgrassi.

see: more, trace.

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #29207 was opened by joaopgrassi.

see: more, trace.

Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
@phlax
Copy link
Member

phlax commented Aug 29, 2023

@AlexanderEllis @htuch @adisuissa seems this is wating on further review/feedback

/wait-any

Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
…continue

Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo open thread on header formatter.

Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
…continue

Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
@joaopgrassi
Copy link
Contributor Author

joaopgrassi commented Oct 17, 2023

/docs

@wbpcode @htuch I went with option 2 - modified the opentelemetry.proto docs to make it clear there's no formatting. All should be resolved and good to go now!

Link to the docs: https://storage.googleapis.com/envoy-pr/d57e0ea/docs/api-v3/config/trace/v3/opentelemetry.proto.html

@repokitteh-read-only
Copy link

Docs for this Pull Request will be rendered here:

https://storage.googleapis.com/envoy-pr/29207/docs/index.html

The docs are (re-)rendered each time the CI envoy-presubmit (precheck docs) job completes.

🐱

Caused by: a #29207 (comment) was created by @joaopgrassi.

see: more, trace.

Copy link
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Some new comments are added.

api/envoy/config/trace/v3/opentelemetry.proto Outdated Show resolved Hide resolved
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
@joaopgrassi
Copy link
Contributor Author

@wbpcode tried addressing the "copy" of headers. Please let me know what you think! thank you 🙌

wbpcode
wbpcode previously approved these changes Oct 18, 2023
Copy link
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@wbpcode wbpcode added this to the 1.28.0 milestone Oct 18, 2023
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
Copy link
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for sending OpenTelemetry Tracing over HTTP/Protobuf endpoint
9 participants