question: should the lua filter log whether or not responses will be hooked when missing function envoy_on_...?

[cetanu]

I get these log messages showing up

"envoy_on_request() function not found. Lua filter will not hook requests."

and I wanted to bring forward the question as to whether they should be an INFO level log message, or debug?
They seem somewhat noisy - my thinking is that most people who configure a lua filter are not surprised if they aren't hooking into either the request or response, in the cases where they only want to handle one. So what is the purpose behind this message?

Should it perhaps be a warning if BOTH functions are missing, but otherwise debug?

Secondarily, if someone was to add function envoy_on_response(_) end into all their lua filters in order to suppress this log message, does that incur any overheads when envoy is running? Does it then cause the lua filter to hook into the request/response, do nothing, and then continue?

Let me know your thoughts.

https://github.com/envoyproxy/envoy/blob/main/source/extensions/filters/http/lua/lua_filter.cc#L223-L230

[ggreenway]

I think the idea behind the log message is for cases where you didn't realize you didn't hook the function, for example if you had a typo in the name of the function.

Possibly a better fix might be to provide configuration for whether the two functions are defined or not, and throw an error if it is supposed to be present and is not. But it would probably be reasonable to change those messages to debug-level unless they're both missing, as you suggest.

[cetanu]

I agree with the better fix 👍🏼 putting this in configuration with sane defaults make sense.

Does this structure and naming seem sensible:

message Lua {
  LogMissingHooks log_missing_hooks = 0;

  enum LogMissingHooks {
    LOG_NONE = 0;
    LOG_MISSING_REQUEST_HOOK = 1;
    LOG_MISSING_RESPONSE_HOOK = 2;
    LOG_BOTH_HOOKS = 3;
    LOG_ANY_MISSING = 4;
  }
}

If so, I will try to find some time to open a PR for this

[ggreenway]

I think just having two boolean settings would be simpler.

But this is probably overkill for this. I'd just change it to a debug-level message for both of these, unless neither is defined and then log them at info/warn level.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.