http_service: finish supporting substitution formatter#44100
http_service: finish supporting substitution formatter#44100ggreenway merged 7 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
ggreenway
requested review from
mattklein123,
tyxia,
wbpcode,
yanavlasov and
yanjunxiang-google
as code owners
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for substitution formatters in HttpService request headers across OpenTelemetry access loggers, the ext_proc filter, OpenTelemetry stat sinks, and Zipkin tracers. This involves refactoring header application using HttpServiceHeadersApplicator and updating related configurations and tests. A critical review comment points out a potential deadlock in HttpAccessLoggerCacheImpl::getOrCreateApplicator due to shared_ptr construction within a locked section. Another comment suggests removing redundant code and improving test robustness in ZipkinHttpServiceIntegrationTest.
test/extensions/tracers/zipkin/zipkin_http_service_integration_test.cc
Outdated
Show resolved
Hide resolved
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of HttpService headers across multiple Envoy extensions, including OpenTelemetry access loggers, the ext_proc filter, OpenTelemetry stats sinks, and OpenTelemetry and Zipkin tracers. It introduces a new HttpServiceHeadersApplicator utility class to centralize the parsing and application of request_headers_to_add, ensuring consistent support for substitution formatters. The change updates the http_service.proto definition and changelog to reflect this broader support. Corresponding unit and integration tests have been added or modified to verify the correct application of formatter-based headers in each affected extension. No feedback to provide.
) This adds support for using the substitution formatter in all uses of `request_headers_to_add` in http_service. Fixes envoyproxy#43994 Signed-off-by: Greg Greenway <ggreenway@apple.com>
) This adds support for using the substitution formatter in all uses of `request_headers_to_add` in http_service. Fixes envoyproxy#43994 Signed-off-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: Xuyang Tao <taoxuy@google.com>
) This adds support for using the substitution formatter in all uses of `request_headers_to_add` in http_service. Fixes envoyproxy#43994 Signed-off-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: Jonathan Wu <jtwu@google.com>
3 participants
Additional Description: This adds support for using the substitution formatter in all uses of
request_headers_to_addin http_service.Risk Level: Low
Testing: tests added
Docs Changes:
Release Notes: added
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
Fixes #43994
Claude code was used in creating this PR.