lua: timestamp in millis #14522

mindyor · 2020-12-28T04:36:40Z

Commit Message: add Lua function to get timestamp in milliseconds and nanoseconds
Additional Description: Description: adds Lua function to get timestamp in higher resolution. Resolutions are: milliseconds since epoch. Resolution defaults to milliseconds since epoch.
Risk Level: low
Testing: added unit tests for default case, resolution levels, and invalid input
Docs Changes: added documentation for function in lua_filter.rst
Release Notes: added release note in version_history.rst
Platform Specific Features: n/a
[Optional Runtime guard:]
[Optional Fixes #Issue] #10282
[Optional Deprecated:]

((an update of #10329))

repokitteh-read-only · 2020-12-28T04:36:43Z

Hi @mindyor, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #14522 was opened by mindyor.

see: more, trace.

dio · 2020-12-28T22:07:56Z

Thanks, @mindyor! Lua API looks good. Could you fix CI?

repokitteh-read-only · 2021-01-15T23:03:10Z

🙀 Error while processing event:

evaluation error
error: context deadline exceeded

🐱

Caused by: #14522 was edited by mattklein123.

see: more, trace.

dio · 2021-02-03T08:42:01Z

@mindyor, do you want me to help you to finish this PR? :).

dio · 2021-02-03T23:54:30Z

@mindyor when you have time, please have a look at https://github.com/envoyproxy/envoy/compare/main...dio:lua-timestamp-millis-updated?expand=1. I added two commits (one of them is to "rebase" it with the current main branch "main", the second one is to make it compiles). I think we need to add tests for running registerGlobal with some initializers (as use-case tests) in test/extensions/filters/common/lua/lua_test.cc. Hope this helps! Thanks!

dio

Please add tests when calling registerGlobal (most likely you need to add it in test/extensions/filters/common/lua/lua_test.cc).

dio · 2021-02-05T09:59:46Z

source/extensions/filters/common/lua/lua.cc

-  tls_slot_->runOnAllThreads([global](OptRef<LuaThreadLocal> tls) {
+uint64_t ThreadLocalState::registerGlobal(const std::string& global,
+                                          const InitializerList& initializers) {
+  tls_slot_->runOnAllThreads([global, &initializers](OptRef<LuaThreadLocal> tls) {


Sorry, my bad, seems like we need to copy the initializers vector.

tls_slot_->runOnAllThreads([global, initializers](OptRef<LuaThreadLocal> tls) {

Thanks! Changed.

dio · 2021-02-05T10:00:17Z

source/extensions/filters/common/lua/lua.cc

@@ -22,7 +22,7 @@ void Coroutine::start(int function_ref, int num_args, const std::function<void()

  state_ = State::Yielded;
  lua_rawgeti(coroutine_state_.get(), LUA_REGISTRYINDEX, function_ref);
-  ASSERT(lua_isfunction(coroutine_state_.get(), -1));
+  ASSERT(lua_isfunction(coroutine_state_.get(), -1) == 1);


Can you comment why this needs to be exactly 1? :)

Hmm it may not be. Taking this out

dio · 2021-02-05T20:21:17Z

This cd870da (in #14882) I think will remedy clang-tidy's complaints.

dio · 2021-02-06T09:04:34Z

/retest

repokitteh-read-only · 2021-02-06T09:04:38Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #14522 (comment) was created by @dio.

see: more, trace.

dio

Looks good. A request to expand the docs a bit more. Thanks!

dio · 2021-02-06T09:05:53Z

docs/root/configuration/http/http_filters/lua_filter.rst

+  timestamp = handle:timestamp(format)
+
+High resolution timestamp function. *format* is an optional string parameter to indicate the format of the timestamp.
+*milliseconds_since_epoch* and *nanoseconds_since_epoch* are supported.


I think we need to document the exported enums here?

dio · 2021-02-06T09:06:13Z

docs/root/version_history/current.rst

@@ -33,6 +33,7 @@ Minor Behavior Changes
  instead of `//source/common/chromium_url`. The new path canonicalizer is enabled by default. To
  revert to the legacy path canonicalizer, enable the runtime flag
  `envoy.reloadable_features.remove_forked_chromium_url`.
+* lua: added function `timestamp` to provide high resolution timestamps.


Could you expand this, so we are aware of the enums.

repokitteh-read-only · 2021-03-22T17:20:26Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
API shepherd assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #14522 was synchronize by mindyor.

see: more, trace.

Signed-off-by: mindyor <[email protected]>

repokitteh-read-only · 2021-03-22T18:25:47Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #14522 was synchronize by mindyor.

see: more, trace.

moderation · 2021-03-22T21:42:34Z

Looks like you need to merge main. Quite a few conflicts

Signed-off-by: mindyor <[email protected]>

…y lua int limits. update documentation Signed-off-by: mindyor <[email protected]>

Signed-off-by: mindyor <[email protected]>

mindyor · 2021-03-26T23:04:35Z

/retest

repokitteh-read-only · 2021-03-26T23:04:41Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #14522 (comment) was created by @mindyor.

see: more, trace.

dio

Thanks, @mindyor! Looks good!

mattklein123

Thanks!

dio self-assigned this Dec 28, 2020

Base automatically changed from master to main January 15, 2021 23:02

dio suggested changes Feb 4, 2021

View reviewed changes

dio reviewed Feb 5, 2021

View reviewed changes

dio suggested changes Feb 6, 2021

View reviewed changes

mindyor force-pushed the lua-timestamp-millis branch from 64cf446 to 127c801 Compare March 17, 2021 23:57

mindyor requested review from alyssawilk, antoniovicente, asraa, cpakulski, ggreenway, htuch, jmarantz, junr03, lambdai, lizan, mattklein123, PiotrSikora, Shikugawa, snowp and yanavlasov as code owners March 17, 2021 23:57

mindyor force-pushed the lua-timestamp-millis branch 2 times, most recently from 127c801 to df5e616 Compare March 22, 2021 18:12

mindyor added 2 commits March 22, 2021 11:25

changes moved from envoy PR 10329

7aeadc5

Signed-off-by: mindyor <[email protected]>

use enumToInt comparison

b9e4ae7

Signed-off-by: mindyor <[email protected]>

mindyor force-pushed the lua-timestamp-millis branch from df5e616 to 8ba3f45 Compare March 22, 2021 18:25

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Mar 22, 2021

mindyor added 10 commits March 22, 2021 23:53

fix lua_test

a385a77

Signed-off-by: mindyor <[email protected]>

fix format

ad5190a

Signed-off-by: mindyor <[email protected]>

alphabetize current.rst

312706b

Signed-off-by: mindyor <[email protected]>

re-up current.rst file

49bf7f9

Signed-off-by: mindyor <[email protected]>

fix wrappers_test

ea4afec

Signed-off-by: mindyor <[email protected]>

formatting

835732b

Signed-off-by: mindyor <[email protected]>

fix lua integration test segfault

0d9c8e9

Signed-off-by: mindyor <[email protected]>

remove extraneous assertion

b4a5223

Signed-off-by: mindyor <[email protected]>

fix clang-tidy

d3f7fe7

Signed-off-by: mindyor <[email protected]>

remove nanosecond resolution due to number size not being supported b…

488bcb7

…y lua int limits. update documentation Signed-off-by: mindyor <[email protected]>

mindyor force-pushed the lua-timestamp-millis branch from 8ba3f45 to 488bcb7 Compare March 23, 2021 07:00

fix formatting

d545b6e

Signed-off-by: mindyor <[email protected]>

htuch removed the api label Mar 23, 2021

mindyor and others added 3 commits March 23, 2021 10:13

fix current.rst

6f4a2f1

Signed-off-by: mindyor <[email protected]>

fix current.rst

ec28a46

Signed-off-by: mindyor <[email protected]>

Merge branch 'main' into lua-timestamp-millis

d7bf086

dio approved these changes Mar 26, 2021

View reviewed changes

mattklein123 approved these changes Mar 28, 2021

View reviewed changes

mattklein123 merged commit 486cd88 into envoyproxy:main Mar 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lua: timestamp in millis #14522

lua: timestamp in millis #14522

mindyor commented Dec 28, 2020 •

edited

Loading

repokitteh-read-only bot commented Dec 28, 2020

dio commented Dec 28, 2020

repokitteh-read-only bot commented Jan 15, 2021

dio commented Feb 3, 2021 •

edited

Loading

dio commented Feb 3, 2021

dio left a comment

dio Feb 5, 2021

mindyor Feb 5, 2021

dio Feb 5, 2021

mindyor Feb 5, 2021

dio commented Feb 5, 2021

dio commented Feb 6, 2021

repokitteh-read-only bot commented Feb 6, 2021

dio left a comment

dio Feb 6, 2021

dio Feb 6, 2021

repokitteh-read-only bot commented Mar 22, 2021

repokitteh-read-only bot commented Mar 22, 2021

moderation commented Mar 22, 2021

mindyor commented Mar 26, 2021

repokitteh-read-only bot commented Mar 26, 2021

dio left a comment

mattklein123 left a comment

lua: timestamp in millis #14522

lua: timestamp in millis #14522

Conversation

mindyor commented Dec 28, 2020 • edited Loading

repokitteh-read-only bot commented Dec 28, 2020

dio commented Dec 28, 2020

repokitteh-read-only bot commented Jan 15, 2021

dio commented Feb 3, 2021 • edited Loading

dio commented Feb 3, 2021

dio left a comment

Choose a reason for hiding this comment

dio Feb 5, 2021

Choose a reason for hiding this comment

mindyor Feb 5, 2021

Choose a reason for hiding this comment

dio Feb 5, 2021

Choose a reason for hiding this comment

mindyor Feb 5, 2021

Choose a reason for hiding this comment

dio commented Feb 5, 2021

dio commented Feb 6, 2021

repokitteh-read-only bot commented Feb 6, 2021

dio left a comment

Choose a reason for hiding this comment

dio Feb 6, 2021

Choose a reason for hiding this comment

dio Feb 6, 2021

Choose a reason for hiding this comment

repokitteh-read-only bot commented Mar 22, 2021

repokitteh-read-only bot commented Mar 22, 2021

moderation commented Mar 22, 2021

mindyor commented Mar 26, 2021

repokitteh-read-only bot commented Mar 26, 2021

dio left a comment

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment

mindyor commented Dec 28, 2020 •

edited

Loading

dio commented Feb 3, 2021 •

edited

Loading