Feat add request id extension settings in ctp #7421
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7421 +/- ##
==========================================
- Coverage 72.33% 72.33% -0.01%
==========================================
Files 232 232
Lines 34114 34128 +14
==========================================
+ Hits 24678 24686 +8
- Misses 7666 7671 +5
- Partials 1770 1771 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/retest |
1 similar comment
|
/retest |
Inode1
force-pushed
the
feat-request-id-extension
branch
from
c163766 to
c5b435d
Compare
|
/retest |
sudiptob2
left a comment
sudiptob2
left a comment
There was a problem hiding this comment.
Hey Thanks!
In the issue you discussed about adding this under EnvoyProxy.spec.telemetry. But this implementation adds it under ClientTrafficPolicy. Is this expected?
Add opt-out field in EnvoyProxy.spec.telemetry.tracing:
Signed-off-by: i.makarychev <[email protected]> Signed-off-by: i.makarychev <[email protected]>
zirain
force-pushed
the
feat-request-id-extension
branch
from
c5b435d to
fef8a88
Compare
| // RequestIDExtension defines configuration for Envoy's request ID extension. | ||
| // | ||
| // +optional | ||
| RequestIDExtension *RequestIDExtensionSettings `json:"requestIdExtension,omitempty"` |
There was a problem hiding this comment.
imo this should live in EnvoyProxy.Spec.Telemetry.Tracing
There was a problem hiding this comment.
is this tracing specific? if not perfer to .spec.telemetry.
There was a problem hiding this comment.
If we put this field under EnvoyProxy.Spec.Telemetry.Tracing, we must also set a provider,
otherwise it will fail here. That’s why I initially handled it in CTP (to set it per listener), but if that’s not ideal, I agree with @zirain that placing this field under EnvoyProxy.Spec.spec.telemetry is the better option.
WDYT?
There was a problem hiding this comment.
I'll bring this up in the community meeting tomorrow of whether to add this in CTP where we have
or EP where telemetry existsThere was a problem hiding this comment.
we're trying to build consensus on whether x-request-id setting should like here in CTP or should be under telemetry in EnvoyProxy, ptal @envoyproxy/gateway-maintainers
|
/retest |
4 participants
What type of PR is this?
What this PR does / why we need it:
Currently, Envoy Gateway configures the HTTP listener with UuidRequestIdConfig and pack_trace_reason set to true by default. This changes the generated x-request-id format from UUID v8 to UUID v4.
Add options to settings requestidextension in CTP
Which issue(s) this PR fixes:
Fixes #6750
Release Notes: Yes/No