test: register a custom variable in a custom Lua file and source it with extra_lua_path

[kayx23]

Description

• Updated doc for Custom Variable.
• Added example and test to register a custom variable by sourcing a custom Lua file.

Checklist

• I have explained the need for this PR and the problem it solves
• I have explained the changes or the new features added to this PR
• I have added tests corresponding to this change
• I have updated the documentation to reflect this change
• I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

[Revolyssup]

To show that the variable was really registered, should we try to access and log this in another phase and then grep and assert in the tests?

[kayx23]

Do you mean something like this?

local old_http_access_phase = apisix.http_access_phase
apisix.http_access_phase = function(...)
    ngx.log(ngx.EMERG, ctx.a6_route_labels)
    old_http_access_phase(...)
end

My understanding is that the custom variable a6_route_labels would only has value if a route is configured with the prop labels.

The register_var("a6_route_labels") example is kept consistent with this test case:

apisix/t/plugin/http-logger-log-format.t

Lines 431 to 441 in 89c1858

	=== TEST 15: use custom variable in the logger
	--- extra_init_by_lua
	local core = require "apisix.core"
	core.ctx.register_var("a6_route_labels", function(ctx)
	local route = ctx.matched_route and ctx.matched_route.value
	if route and route.labels then
	return route.labels
	end
	return nil
	end)

Please advise.

[Revolyssup]

Yes part of the suggestion was to PUT that route (just like in the test you mentioned) and then send request to that route, after which to make assertion. But I don't have a strong opinion here. If others think that showing how it's registered is good enough then I think its fine. cc: @monkeyDluffy6017 @leslie-tsang What do you think?

[leslie-tsang]

We've already add test cases for this feature, and it doesn't seem necessary to do it again in the demo.
BTW, "my hook works in http" doesn't mean it actually works, it's just a log.

[kayx23]

Yeah I understand this doesn't cover the entire use case. The initial motivation of this was users having difficulties understanding how to register a variable with a custom Lua file that gets sourced (i.e. lua_module_hook). Once it's sourced and APISIX starts successfully (aka here it doesn't error out), the rest is easy. I agree with you that the feature of sourcing external files is covered in test, however.

[shreemaan-abhishek]

Since we already have a test:

apisix/t/plugin/http-logger-log-format.t

Lines 431 to 441 in 89c1858

	=== TEST 15: use custom variable in the logger
	--- extra_init_by_lua
	local core = require "apisix.core"
	core.ctx.register_var("a6_route_labels", function(ctx)
	local route = ctx.matched_route and ctx.matched_route.value
	if route and route.labels then
	return route.labels
	end
	return nil
	end)

That shows the given example function for registering vars is correct. This PR LGTM.

[Revolyssup]

Since we already have a test:

apisix/t/plugin/http-logger-log-format.t

Lines 431 to 441 in 89c1858

	=== TEST 15: use custom variable in the logger
	--- extra_init_by_lua
	local core = require "apisix.core"
	core.ctx.register_var("a6_route_labels", function(ctx)
	local route = ctx.matched_route and ctx.matched_route.value
	if route and route.labels then
	return route.labels
	end
	return nil
	end)

That shows the given example function for registering vars is correct. This PR LGTM.

So we should reference this test in the doc

[leslie-tsang]

Suggested change

	return route.labels
	return route.labels

Please standardize indentation style. 4 space will be please

[leslie-tsang]

Suggested change

	ngx.log(ngx.EMERG, "my hook works in stream")
	ngx.log(ngx.EMERG, "my hook works in stream")

please use core.log instead.

[leslie-tsang]

We've already add test cases for this feature, and it doesn't seem necessary to do it again in the demo.
BTW, "my hook works in http" doesn't mean it actually works, it's just a log.

[kayx23]

it doesn't seem necessary to do it again in the demo

@leslie-tsang this provides a working example for this issue (and doc): #9878

I don't have a strong opinion here either. It it's considered fully covered, feel free to close the PR.

[kayx23]

I agree this is more of an example than test coverage for the feature. Maybe we'd just update the docs alone to provide more instructions. #10086 (comment)