Generalize the gRPC access logger base classes #14469
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/assign yanavlasov |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 14469 in repo envoyproxy/envoy |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
yanavlasov
approved these changes
Dec 22, 2020
|
@yanavlasov seems like presubmits are still failing (doesn't seem related to my change) - can you please re-run them. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
yanavlasov
previously approved these changes
Dec 22, 2020
|
@itamarkam would you be able to merge master into this please? Just to see if it fixes the build issue. |
Done. |
|
@itamarkam would you be able to merge master again please? I think the coverage issue was fixed. |
Temporary commit Small fixes Signed-off-by: Itamar Kaminski <itamark@google.com> Testing skeleton Signed-off-by: Itamar Kaminski <itamark@google.com> Test skeleton Signed-off-by: Itamar Kaminski <itamark@google.com> Tests and Access logger cache generalization Signed-off-by: Itamar Kaminski <itamark@google.com> Tests done Signed-off-by: Itamar Kaminski <itamark@google.com> Tests done after horrible merge Signed-off-by: Itamar Kaminski <itamark@google.com> Uncomment something Signed-off-by: Itamar Kaminski <itamark@google.com> Cleanups and dependency fixes Signed-off-by: Itamar Kaminski <itamark@google.com> Format fix Signed-off-by: Itamar Kaminski <itamark@google.com>
Signed-off-by: Itamar Kaminski <itamark@google.com>
|
Rebased on the master again. |
yanavlasov
approved these changes
Jan 6, 2021
mpuncel
added a commit
to mpuncel/envoy
that referenced
this pull request
Jan 8, 2021
* master: (48 commits) Resolve 14506, avoid libidn2 for our curl dependency (envoyproxy#14601) fix new/free mismatch in Mainthread utility (envoyproxy#14596) opencensus: deprecate Zipkin configuration. (envoyproxy#14576) upstream: clean up code location (envoyproxy#14580) configuration impl: add cast for ios compilation (envoyproxy#14590) buffer impl: add cast for android compilation (envoyproxy#14589) ratelimit: add dynamic metadata to ratelimit response (envoyproxy#14508) tcp_proxy: wait for CONNECT response before start streaming data (envoyproxy#14317) stream info: cleanup address handling (envoyproxy#14432) [deps] update upb to latest commit (envoyproxy#14582) Add utility to check whether the execution is in main thread. (envoyproxy#14457) listener: undeprecate bind_to_port (envoyproxy#14480) Fix data race in overload integration test (envoyproxy#14586) deps: update PGV (envoyproxy#14571) dependencies: update cve_scan.py for some libcurl 7.74.0 false positives. (envoyproxy#14572) Network::Connection: Add L4 crash dumping support (envoyproxy#14509) ssl: remember stat names for configured ciphers. (envoyproxy#14534) formatter: add custom date formatting to downstream cert start and end dates (envoyproxy#14502) feat(lua): allow setting response body when the upstream response body is empty (envoyproxy#14486) Generalize the gRPC access logger base classes (envoyproxy#14469) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message: Generalize the gRPC access logger base classes
Additional Description: Some of the general logging logic was in the classes implementing the gRPC access logger interface (GrpcAccessLoggerImpl and GrpcAccessLoggerCacheImpl). Putting all the (soon to be) shared logic in the base class should make it very easy to create new loggers (see "source/extensions/access_loggers/grpc/grpc_access_log_impl.h/cc" for example).
While moving these things around and chancing some dependencies, I also went ahead and fixed some unused/indirect dependencies.
Risk Level: Low (refactoring)
Testing: Unit tests moved the implementation tests to the base class tests (now that the logic lies there) and simplified the implementing classes' tests.
Docs Changes: None
Release Notes: None
Platform Specific Features: None
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
@omriz @edrukh @yanavlasov @htuch