-
Notifications
You must be signed in to change notification settings - Fork 204
Implement envoy route config for opentelemetry-collector #5016
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
Conversation
Signed-off-by: Shinnosuke Sawada <[email protected]>
Signed-off-by: Shinnosuke Sawada <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5016 +/- ##
=======================================
Coverage 22.47% 22.47%
=======================================
Files 519 519
Lines 56792 56792
=======================================
Hits 12762 12762
Misses 43003 43003
Partials 1027 1027 ☔ View full report in Codecov by Sentry. |
khanhtc1202
left a comment
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 👍
| - match: # We want to protect the opentelemetry service with envoy ext_authz, so this route must not turn off ext_authz. | ||
| prefix: "/opentelemetry.proto.collector.trace.v1.TraceService/" |
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.
[ask] Is this it? Just to be sure.
https://github.com/open-telemetry/opentelemetry-proto/blob/ff457cecf46cf219602e587d86d66f3b8cb3efe6/opentelemetry/proto/collector/trace/v1/trace_service.proto#L30
Piped calls the rpc method in the opentelemetry.proto.collector.trace.v1.TraceService to send the data to opentelemetry-collector.
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.
Yes. You are right!
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, I got it.
ffjlabo
left a comment
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.
🚀
* Implement envoy route config for opentelemetry-collector Signed-off-by: Shinnosuke Sawada <[email protected]> * Add annotation for gateway to rollout when its config changes Signed-off-by: Shinnosuke Sawada <[email protected]> --------- Signed-off-by: Shinnosuke Sawada <[email protected]> Signed-off-by: kumo-rn5s <[email protected]>
* Implement envoy route config for opentelemetry-collector Signed-off-by: Shinnosuke Sawada <[email protected]> * Add annotation for gateway to rollout when its config changes Signed-off-by: Shinnosuke Sawada <[email protected]> --------- Signed-off-by: Shinnosuke Sawada <[email protected]> Signed-off-by: khanhtc1202 <[email protected]>
What this PR does / why we need it:
This PR modifies the envoy config to add a route to the OpenTelemetry Collector.
Which issue(s) this PR fixes:
Part of #4977
Does this PR introduce a user-facing change?: