-
Notifications
You must be signed in to change notification settings - Fork 687
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
Add propagation_modes for the Lightstep Tracer #4179
Add propagation_modes for the Lightstep Tracer #4179
Conversation
@@ -29,16 +29,20 @@ type TraceSampling struct { | |||
Overall *int `json:"overall,omitempty"` | |||
} | |||
|
|||
// +kubebuilder:validation:Enum=ENVOY;LIGHTSTEP;B3;TRACE_CONTEXT |
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.
Nit: I know some of the existing enums don't follow this, but I believe k8s style is to use PascalCase.
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.
I am following the expected values in the Envoy proto files https://github.com/envoyproxy/envoy/blob/96c317837ab7351f6264d25c6e19e09fae3a6054/api/envoy/config/trace/v3/lightstep.proto#L29
2907427
to
5c4f9db
Compare
The tests are based on test_shadow.py. If there is a cleaner way of initializing the confis, I am all ears :) |
5c4f9db
to
1b602d0
Compare
The Lightstep tracer supports multiple propagation modes. See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/trace/v3/lightstep.proto#enum-config-trace-v3-lightstepconfig-propagationmode With this PR it is now possible to set a list of propagation modes in the config field of the TracingService. Signed-off-by: Paul Salaberria <[email protected]>
1b602d0
to
66c957e
Compare
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.
This looks good -- thanks, @psalaberria002!! 🙂 I'm going to see about getting CI to run on it...
@LukeShu, I was feeling the same way about the enum values, until remembering that the driver_config
in the TracingService
is literally handed to Envoy after validation. 🙁 We need to fix that.
Signed-off-by: Paul Salaberria <[email protected]>
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.
We have a clean CI run in #4192, so off we go! Thanks @psalaberria002!
The Lightstep tracer supports multiple propagation modes.
See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/trace/v3/lightstep.proto#enum-config-trace-v3-lightstepconfig-propagationmode
With this PR it is now possible to set a list of propagation modes in
the config field of the TracingService.
I have verified it locally end to end. Seems to work.
However, I wasn't able to make tests work locally. Issues with a dummy-pod not coming up due to pull permissions, but it doesn't seem right. The cluster is able to pull images from the same registry...
Fixes #3905