-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
access log: added new START_TIME_LOCAL & EMIT_TIME_LOCAL command #35280
Conversation
Signed-off-by: doujiang24 <[email protected]>
PTAL cc @wbpcode Thanks. |
In order to review this PR, we check the code of I will inclined to fix that bug first. And I think it will simply this PR. /wait-any |
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.
Thanks for the contribution. And please also provide the comparation of the speed test results between the old implementation and the new one.
/wait |
Signed-off-by: doujiang24 <[email protected]>
Signed-off-by: doujiang24 <[email protected]>
@wbpcode Thanks and sorry for the late. Here are the benchmark results:
|
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.
Thanks for the update. It's near there. Some comments are added.
And you may could add a simple speed test in the utility_speed_test.cc for the AccessLogDateTimeFormatter::fromTime
with local time zone.
(I guess it won't bring much effect because there is only some additional bool check, but this is a performance sensitive feature anyway, a test would be better)
Signed-off-by: doujiang24 <[email protected]>
Signed-off-by: doujiang24 <[email protected]>
LGTM. Thanks. |
…oyproxy#35280) Commit Message: access log: added new START_TIME_LOCAL & EMIT_TIME_LOCAL command Additional Description: Same as START_TIME & EMIT_TIME, but use local time zone. close envoyproxy#35116 Risk Level: low. Testing: unit. Docs Changes: added. Release Notes: TODO. Platform Specific Features: n/a. --------- Signed-off-by: doujiang24 <[email protected]> Signed-off-by: Martin Duke <[email protected]>
…oyproxy#35280) Commit Message: access log: added new START_TIME_LOCAL & EMIT_TIME_LOCAL command Additional Description: Same as START_TIME & EMIT_TIME, but use local time zone. close envoyproxy#35116 Risk Level: low. Testing: unit. Docs Changes: added. Release Notes: TODO. Platform Specific Features: n/a. --------- Signed-off-by: doujiang24 <[email protected]> Signed-off-by: asingh-g <[email protected]>
Commit Message: access log: added new START_TIME_LOCAL & EMIT_TIME_LOCAL command
Additional Description:
Same as START_TIME & EMIT_TIME, but use local time zone.
close #35116
Risk Level: low.
Testing: unit.
Docs Changes: added.
Release Notes: TODO.
Platform Specific Features: n/a.