-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
opentelemetry tracer: add OTLP/HTTP exporter #29207
Conversation
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>
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. |
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
@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>
There was a problem hiding this 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>
/docs @wbpcode @htuch I went with option 2 - modified the Link to the docs: https://storage.googleapis.com/envoy-pr/d57e0ea/docs/api-v3/config/trace/v3/opentelemetry.proto.html |
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 |
There was a problem hiding this 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.
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
@wbpcode tried addressing the "copy" of headers. Please let me know what you think! thank you 🙌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
test/extensions/tracers/opentelemetry/http_trace_exporter_test.cc
Outdated
Show resolved
Hide resolved
Signed-off-by: Joao Grassi <joao.grassi@dynatrace.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
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: