tracer: add CEL sampler to OpenTelemetry #38182
Conversation
Signed-off-by: zirain <[email protected]>
repokitteh-read-only
bot
added
api
deps
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: zirain <[email protected]>
Signed-off-by: zirain <[email protected]>
Signed-off-by: zirain <[email protected]>
Signed-off-by: zirain <[email protected]>
|
/lgtm deps Metadata changes only |
repokitteh-read-only
bot
removed
the
deps
|
ping @wbpcode |
|
@wbpcode PTAL |
api/envoy/extensions/tracers/opentelemetry/samplers/v3/cel_sampler.proto
Outdated
Show resolved
Hide resolved
|
/wait-any |
Signed-off-by: zirain <[email protected]>
repokitteh-read-only
bot
added
deps
api/envoy/extensions/tracers/opentelemetry/samplers/v3/cel_sampler.proto
Outdated
Show resolved
Hide resolved
Signed-off-by: zirain <[email protected]>
Signed-off-by: zirain <[email protected]>
Signed-off-by: zirain <[email protected]>
Signed-off-by: zirain <[email protected]>
|
|
||
| message CELSamplerConfig { | ||
| // Expression that, when evaluated, will be used to make sample decision. | ||
| string expression = 1 [(validate.rules).string = {min_len: 1}]; |
There was a problem hiding this comment.
Sorry, just noticed. Elsewhere in Envoy we are using this proto https://github.com/cncf/xds/blob/main/xds/type/v3/cel.proto for CEL expressions.
The reason is that when control plane is used, the CEL expression will be parsed by the control plane and errors are detected before it reaches Envoy.
I think it is also useful to have an option to just write the expression as is for simpler cases, where control plane is not used.
My suggestion is to use the xds.type.v3.CelExpression here, instead of bare string and add bare string to the xds.type.v3.CelExpression proto.
I know this is more work, but otherwise this feature would be difficult to use with complex control planes.
There was a problem hiding this comment.
should we do same thing for https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/access_loggers/filters/cel/v3/cel.proto?
There was a problem hiding this comment.
should we do same thing for https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/access_loggers/filters/cel/v3/cel.proto?
Yes, we should. It was not done quite right.
Thanks for driving cncf/xds#115 I think this is the right approach to configure CEL
|
/wait-any |
Commit Message: add CEL sampler to OpenTelemetry tracer
Additional Description: This is initial PR, there are plans to support request headers in the next step, and any suggestions are welcome. cc @wbpcode
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]