You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/07/20 07:30:22 UTC

[GitHub] [apisix] dmsolr opened a new pull request #4633: feat: add error log skywalking reporter

dmsolr opened a new pull request #4633:
URL: https://github.com/apache/apisix/pull/4633


   ### What this PR does / why we need it:
   <!--- Why is this change required? What problem does it solve? -->
   <!--- If it fixes an open issue, please link to the issue here. -->
   
   ### Pre-submission checklist:
   
   * [ ] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
   
   his plugin is `error-log-logger`-like, but it sends log data to Apache SkyWalking over http. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r677912388



##########
File path: t/plugin/error-log-logger-skywalking.t
##########
@@ -0,0 +1,232 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+worker_connections(128);
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 4: put plugin metadata and log an error level message
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log",
+                        "service_instance_name": "instance"
+                    },
+                    "batch_max_size": 15,
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*this is an error message for test.*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"instance\".*/
+--- wait: 5
+
+
+
+=== TEST 5: log an warn level message

Review comment:
       What information did I miss? What's the point of these tests.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r681385025



##########
File path: t/plugin/error-log-logger-skywalking.t
##########
@@ -0,0 +1,232 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+worker_connections(128);
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 4: put plugin metadata and log an error level message
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log",
+                        "service_instance_name": "instance"
+                    },
+                    "batch_max_size": 15,
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*this is an error message for test.*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"instance\".*/
+--- wait: 5
+
+
+
+=== TEST 5: log an warn level message

Review comment:
       This case is testing whether a warning message could be wrapper as the skywalking log format




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] starsz commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r675270290



##########
File path: apisix/plugins/log-generator.lua
##########
@@ -0,0 +1,63 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local ngx = ngx
+local core = require("apisix.core")
+
+local schema = {
+    type = "object",
+    properties = {
+
+    },
+    required = {},
+}
+
+
+local plugin_name = "log-generator"
+
+local _M = {
+    version = 0.1,
+    priority = 0,        -- TODO: add a type field, may be a good idea
+    name = plugin_name,
+    schema = schema
+}
+
+
+function _M.init()
+    local handler
+    handler = function (premature)
+        if premature then
+            return
+        end
+        core.log.warn("log something for your testing. 1")
+        core.log.warn("log something for your testing. 2")

Review comment:
       Just want to know what's this ?
   Do we need them?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r677931605



##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -146,7 +248,19 @@ local function process()
             core.log.warn("set log filter failed for ", err)
             return
         end
-
+        if not config.tcp then

Review comment:
       Also need to filter out config.skywalking

##########
File path: docs/en/latest/plugins/error-log-logger.md
##########
@@ -96,8 +103,26 @@ Step: update the attributes of the plugin
 ```shell
 curl http://127.0.0.1:9080/apisix/admin/plugin_metadata/error-log-logger -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
 {
-  "host": "127.0.0.1",
-  "port": 1999,
+  "tcp": {
+    "host": "127.0.0.1",
+    "port": 1999
+  },
+  "inactive_timeout": 1
+}'
+```
+
+## How to set the SkyWalking OAP server address
+
+Step: update the attributes of the plugin
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/plugin_metadata/error-log-logger -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+  "type": "SKYWALKING",
+  "skywalking": {
+    "host": "127.0.0.1",

Review comment:
       skywalking doesn't use host?

##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +37,52 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,
-        port = {type = "integer", minimum = 0},
-        tls = {type = "boolean", default = false},
-        tls_server_name = {type = "string"},
-        timeout = {type = "integer", minimum = 1, default = 3},
-        keepalive = {type = "integer", minimum = 1, default = 30},
+        type = {type = "string", default = "TCP", enum = {"TCP", "SKYWALKING"}},
+        tcp = {
+            type = "object",
+            properties = {
+                host = schema_def.host_def,
+                port = {type = "integer", minimum = 0},
+                tls = {type = "boolean", default = false},
+                tls_server_name = {type = "string"},
+            },
+            required = {"host", "port"}
+        },
+        skywalking = {
+            type = "object",
+            properties = {
+                endpoint_addr = {schema_def.uri, default = "http://127.0.0.1:12900/v3/logs"},
+                service_name = {type = "string", default = "APISIX"},
+                service_instance_name = {type="string", default = "APISIX Service Instance"},
+            },
+            required = {"endpoint_addr"}

Review comment:
       Once you provide a default value, the field is actually optional, as users don't need to specify their values.

##########
File path: t/plugin/error-log-logger-skywalking.t
##########
@@ -0,0 +1,232 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+worker_connections(128);
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 4: put plugin metadata and log an error level message
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log",
+                        "service_instance_name": "instance"
+                    },
+                    "batch_max_size": 15,
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")

Review comment:
       Need a test for multiple error logs

##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +37,52 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,
-        port = {type = "integer", minimum = 0},
-        tls = {type = "boolean", default = false},
-        tls_server_name = {type = "string"},
-        timeout = {type = "integer", minimum = 1, default = 3},
-        keepalive = {type = "integer", minimum = 1, default = 30},
+        type = {type = "string", default = "TCP", enum = {"TCP", "SKYWALKING"}},

Review comment:
       The type field is unused now?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r676798070



##########
File path: t/plugin/error-log-skywalking-logger.t
##########
@@ -0,0 +1,357 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: not enable the plugin
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+error-log-logger
+--- wait: 2
+
+
+
+=== TEST 4: enable the plugin, but not init the metadata
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 5: set a wrong metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "port": 1999,
+                    "inactive_timeout": 1
+                }]]
+                )
+
+            -- ensure the request is rejected even this plugin doesn't
+            -- have check_schema method
+            ngx.status = code
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- error_code: 201
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 6: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 7: put plugin metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: log an error level message
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"APISIX Service Instance\".*/
+--- wait: 15

Review comment:
       It is for testing on my local. fixed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r676246766



##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -134,6 +219,14 @@ local function update_filter(value)
 end
 
 
+local function send(data)
+    if config.type == "TCP" then

Review comment:
       Better to use config.tcp to decide the destination, so that we don't need to check the `config.tcp` is nil or not.

##########
File path: t/plugin/error-log-skywalking-logger.t
##########
@@ -0,0 +1,357 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: not enable the plugin

Review comment:
       TEST 3~5,9,10 are redundant as t/plugin/error-log-logger.t already checked

##########
File path: t/plugin/error-log-skywalking-logger.t
##########
@@ -0,0 +1,357 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: not enable the plugin
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+error-log-logger
+--- wait: 2
+
+
+
+=== TEST 4: enable the plugin, but not init the metadata
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 5: set a wrong metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "port": 1999,
+                    "inactive_timeout": 1
+                }]]
+                )
+
+            -- ensure the request is rejected even this plugin doesn't
+            -- have check_schema method
+            ngx.status = code
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- error_code: 201
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 6: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 7: put plugin metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: log an error level message
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"APISIX Service Instance\".*/
+--- wait: 15

Review comment:
       Do we need to wait so long as the error report is different from the skywalking tracer report?

##########
File path: t/plugin/error-log-skywalking-logger.t
##########
@@ -0,0 +1,357 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: not enable the plugin
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+error-log-logger
+--- wait: 2
+
+
+
+=== TEST 4: enable the plugin, but not init the metadata
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 5: set a wrong metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "port": 1999,
+                    "inactive_timeout": 1
+                }]]
+                )
+
+            -- ensure the request is rejected even this plugin doesn't
+            -- have check_schema method
+            ngx.status = code
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- error_code: 201
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 6: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 7: put plugin metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: log an error level message
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"APISIX Service Instance\".*/

Review comment:
       Need to check the error log in the text.

##########
File path: t/plugin/error-log-skywalking-logger.t
##########
@@ -0,0 +1,357 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: not enable the plugin
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+error-log-logger
+--- wait: 2
+
+
+
+=== TEST 4: enable the plugin, but not init the metadata
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 5: set a wrong metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "port": 1999,
+                    "inactive_timeout": 1
+                }]]
+                )
+
+            -- ensure the request is rejected even this plugin doesn't
+            -- have check_schema method
+            ngx.status = code
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- error_code: 201
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 6: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 7: put plugin metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: log an error level message
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"APISIX Service Instance\".*/

Review comment:
       Also, need a case that multiple error logs are captured and sent.

##########
File path: t/plugin/error-log-skywalking-logger.t
##########
@@ -0,0 +1,357 @@
+#

Review comment:
       Should use `t/plugin/error-log-logger-skywalking.t`, the prefix is the plugin name.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r675265925



##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +37,46 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,
-        port = {type = "integer", minimum = 0},
-        tls = {type = "boolean", default = false},
-        tls_server_name = {type = "string"},
-        timeout = {type = "integer", minimum = 1, default = 3},
-        keepalive = {type = "integer", minimum = 1, default = 30},
+        type = {type = "string", default = "TCP", enum = {"TCP", "SKYWALKING"}},
+        tcp = {
+            type = "object",
+            properties = {
+                host = schema_def.host_def,
+                port = {type = "integer", minimum = 0},
+                tls = {type = "boolean", default = false},
+                tls_server_name = {type = "string"},
+            },
+            required = {"host", "port"}
+        },
+        skywalking = {
+            type = "object",
+            properties = {
+                endpoint_addr = schema_def.uri,
+                service_name = {type = "string", default = "APISIX"},
+                service_instance_name = {type="string", default = "APISIX Service Instance"},
+            },
+            required = {"endpoint_addr"}
+        },
+        host = {schema_def.host_def, description = "Deprecated, use `tcp.host` instead."},
+        port = {type = "integer", minimum = 0, description = "Deprecated, use `tcp.port` instead."},
+        tls = {type = "boolean", default = false,
+                description = "Deprecated, use `tcp.tls` instead."},
+        tls_server_name = {type = "string",
+                description = "Deprecated, use `tcp.tls_server_name` instead."},
         name = {type = "string", default = plugin_name},
         level = {type = "string", default = "WARN", enum = {"STDERR", "EMERG", "ALERT", "CRIT",
                 "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"}},
+        timeout = {type = "integer", minimum = 1, default = 3},
+        keepalive = {type = "integer", minimum = 1, default = 30},
         batch_max_size = {type = "integer", minimum = 0, default = 1000},
         max_retry_count = {type = "integer", minimum = 0, default = 0},
         retry_delay = {type = "integer", minimum = 0, default = 1},
         buffer_duration = {type = "integer", minimum = 1, default = 60},
         inactive_timeout = {type = "integer", minimum = 1, default = 3},
     },
-    required = {"host", "port"}

Review comment:
       Now the outer requirement constraint should be either `tcp` or `skywalking`.

##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -94,33 +126,86 @@ local function send_to_server(data)
 
     sock:settimeout(config.timeout * 1000)
 
-    local ok, err = sock:connect(config.host, config.port)
+    local tcp_config = config.tcp
+    local ok, err = sock:connect(tcp_config.host, tcp_config.port)
     if not ok then
-        return false, "failed to connect the TCP server: host[" .. config.host
-            .. "] port[" .. tostring(config.port) .. "] err: " .. err
+        return false, "failed to connect the TCP server: host[" .. tcp_config.host
+            .. "] port[" .. tostring(tcp_config.port) .. "] err: " .. err
     end
 
-    if config.tls then
-        ok, err = sock:sslhandshake(false, config.tls_server_name, false)
+    if tcp_config.tls then
+        ok, err = sock:sslhandshake(false, tcp_config.tls_server_name, false)
         if not ok then
             sock:close()
             return false, "failed to perform TLS handshake to TCP server: host["
-                .. config.host .. "] port[" .. tostring(config.port) .. "] err: " .. err
+                .. tcp_config.host .. "] port[" .. tostring(tcp_config.port) .. "] err: " .. err
         end
     end
 
     local bytes, err = sock:send(data)
     if not bytes then
         sock:close()
-        return false, "failed to send data to TCP server: host[" .. config.host
-            .. "] port[" .. tostring(config.port) .. "] err: " .. err
+        return false, "failed to send data to TCP server: host[" .. tcp_config.host
+            .. "] port[" .. tostring(tcp_config.port) .. "] err: " .. err
     end
 
     sock:setkeepalive(config.keepalive * 1000)
     return true
 end
 
 
+local function send_to_skywalking(log_message)
+    local err_msg
+    local res = true
+    core.log.info("sending a batch logs to ", config.skywalking.endpoint_addr)
+
+    local httpc = http.new()
+    httpc:set_timeout(config.timeout * 1000)
+
+    local entries = {}
+    for i = 1, #log_message, 2 do
+        local content = {
+            service = config.skywalking.service_name,
+            serviceInstance = config.skywalking.service_instance_name,
+            endpoint = "",
+            body = {
+                text = {
+                    text = log_message[i]
+                }
+           }
+        }
+        table.insert(entries, content)
+    end
+
+    local httpc_res, httpc_err = httpc:request_uri(
+        config.skywalking.endpoint_addr,
+        {
+            method = "POST",
+            body = core.json.encode(entries),
+            keepalive_timeout = config.keepalive * 1000,
+            headers = {
+                ["Content-Type"] = "application/json",
+            }
+        }
+    )
+
+    if not httpc_res then
+        return false, "error while sending data to skywalking["
+            .. config.skywalking.endpoint_addr .. "] " .. httpc_err
+    end
+
+    -- some error occurred in the server
+    if httpc_res.status >= 400 then
+        res =  false
+        err_msg = "server returned status code[" .. httpc_res.status

Review comment:
       Use `string.format` to make the message more readable?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#issuecomment-886034767


   @dmsolr Please also replenish the documents.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander merged pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4633:
URL: https://github.com/apache/apisix/pull/4633


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r673043567



##########
File path: apisix/plugins/error-log-skywalking-logger.lua
##########
@@ -0,0 +1,260 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local errlog = require("ngx.errlog")
+local batch_processor = require("apisix.utils.batch-processor")
+local plugin = require("apisix.plugin")
+local timers = require("apisix.timers")
+local http = require("resty.http")
+local url = require("net.url")
+local plugin_name = "error-log-skywalking-logger"
+local table = core.table
+local schema_def = core.schema
+local ngx = ngx
+local string = string
+local tostring = tostring
+local ipairs  = ipairs
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 32
+})
+
+
+local metadata_schema = {
+    type = "object",
+    properties = {
+        endpoint = schema_def.uri,
+        service_name = {type = "string", default = "APISIX"},
+        service_instance_name = {type="string", default = "APISIX Service Instance"},
+        timeout = {type = "integer", minimum = 1, default = 3},
+        keepalive = {type = "integer", minimum = 1, default = 30},
+        level = {type = "string", default = "WARN", enum = {"STDERR", "EMERG", "ALERT", "CRIT",
+                "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"}},
+        batch_max_size = {type = "integer", minimum = 0, default = 1000},
+        max_retry_count = {type = "integer", minimum = 0, default = 0},
+        retry_delay = {type = "integer", minimum = 0, default = 1},
+        buffer_duration = {type = "integer", minimum = 1, default = 60},
+        inactive_timeout = {type = "integer", minimum = 1, default = 3},
+    },
+    required = {"endpoint"}
+}
+
+
+local schema = {
+    type = "object",
+}
+
+
+local log_level = {

Review comment:
       Agree, and this extension might be reused by many other plugins in the future.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r672881807



##########
File path: apisix/plugins/error-log-skywalking-logger.lua
##########
@@ -0,0 +1,260 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local errlog = require("ngx.errlog")
+local batch_processor = require("apisix.utils.batch-processor")
+local plugin = require("apisix.plugin")
+local timers = require("apisix.timers")
+local http = require("resty.http")
+local url = require("net.url")
+local plugin_name = "error-log-skywalking-logger"
+local table = core.table
+local schema_def = core.schema
+local ngx = ngx
+local string = string
+local tostring = tostring
+local ipairs  = ipairs
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 32
+})
+
+
+local metadata_schema = {
+    type = "object",
+    properties = {
+        endpoint = schema_def.uri,
+        service_name = {type = "string", default = "APISIX"},
+        service_instance_name = {type="string", default = "APISIX Service Instance"},
+        timeout = {type = "integer", minimum = 1, default = 3},
+        keepalive = {type = "integer", minimum = 1, default = 30},
+        level = {type = "string", default = "WARN", enum = {"STDERR", "EMERG", "ALERT", "CRIT",
+                "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"}},
+        batch_max_size = {type = "integer", minimum = 0, default = 1000},
+        max_retry_count = {type = "integer", minimum = 0, default = 0},
+        retry_delay = {type = "integer", minimum = 0, default = 1},
+        buffer_duration = {type = "integer", minimum = 1, default = 60},
+        inactive_timeout = {type = "integer", minimum = 1, default = 3},
+    },
+    required = {"endpoint"}
+}
+
+
+local schema = {
+    type = "object",
+}
+
+
+local log_level = {

Review comment:
       What about extending the error-log plugin to send skywalking format error log? Therefore we can avoid copying lots of code.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r672881807



##########
File path: apisix/plugins/error-log-skywalking-logger.lua
##########
@@ -0,0 +1,260 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local errlog = require("ngx.errlog")
+local batch_processor = require("apisix.utils.batch-processor")
+local plugin = require("apisix.plugin")
+local timers = require("apisix.timers")
+local http = require("resty.http")
+local url = require("net.url")
+local plugin_name = "error-log-skywalking-logger"
+local table = core.table
+local schema_def = core.schema
+local ngx = ngx
+local string = string
+local tostring = tostring
+local ipairs  = ipairs
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 32
+})
+
+
+local metadata_schema = {
+    type = "object",
+    properties = {
+        endpoint = schema_def.uri,
+        service_name = {type = "string", default = "APISIX"},
+        service_instance_name = {type="string", default = "APISIX Service Instance"},
+        timeout = {type = "integer", minimum = 1, default = 3},
+        keepalive = {type = "integer", minimum = 1, default = 30},
+        level = {type = "string", default = "WARN", enum = {"STDERR", "EMERG", "ALERT", "CRIT",
+                "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"}},
+        batch_max_size = {type = "integer", minimum = 0, default = 1000},
+        max_retry_count = {type = "integer", minimum = 0, default = 0},
+        retry_delay = {type = "integer", minimum = 0, default = 1},
+        buffer_duration = {type = "integer", minimum = 1, default = 60},
+        inactive_timeout = {type = "integer", minimum = 1, default = 3},
+    },
+    required = {"endpoint"}
+}
+
+
+local schema = {
+    type = "object",
+}
+
+
+local log_level = {

Review comment:
       What about extending the error-log plugin to send skywalking format error log? Therefore we can avoid copying lots of code.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r674452676



##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +38,40 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,

Review comment:
       We need to keep the old fields and mark them as deprecated (at least in the doc), like:
   https://github.com/apache/apisix/blob/71bc27cc41cf52ba1a41816311412527ae278045/apisix/plugins/authz-keycloak.lua#L34

##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -94,33 +121,109 @@ local function send_to_server(data)
 
     sock:settimeout(config.timeout * 1000)
 
-    local ok, err = sock:connect(config.host, config.port)
+    local tcp_config = config.tcp
+    local ok, err = sock:connect(tcp_config.host, tcp_config.port)
     if not ok then
-        return false, "failed to connect the TCP server: host[" .. config.host
-            .. "] port[" .. tostring(config.port) .. "] err: " .. err
+        return false, "failed to connect the TCP server: host[" .. tcp_config.host
+            .. "] port[" .. tostring(tcp_config.port) .. "] err: " .. err
     end
 
-    if config.tls then
-        ok, err = sock:sslhandshake(false, config.tls_server_name, false)
+    if tcp_config.tls then
+        ok, err = sock:sslhandshake(false, tcp_config.tls_server_name, false)
         if not ok then
             sock:close()
             return false, "failed to perform TLS handshake to TCP server: host["
-                .. config.host .. "] port[" .. tostring(config.port) .. "] err: " .. err
+                .. tcp_config.host .. "] port[" .. tostring(tcp_config.port) .. "] err: " .. err
         end
     end
 
     local bytes, err = sock:send(data)
     if not bytes then
         sock:close()
-        return false, "failed to send data to TCP server: host[" .. config.host
-            .. "] port[" .. tostring(config.port) .. "] err: " .. err
+        return false, "failed to send data to TCP server: host[" .. tcp_config.host
+            .. "] port[" .. tostring(tcp_config.port) .. "] err: " .. err
     end
 
-    sock:setkeepalive(config.keepalive * 1000)
+    sock:setkeepalive(tcp_config.keepalive * 1000)
     return true
 end
 
 
+local function send_to_skywalking(log_message)
+    local err_msg
+    local res = true
+    local url_decoded = url.parse(config.skywalking.endpoint_addr)
+    local host = url_decoded.host
+    local port = url_decoded.port
+
+    core.log.info("sending a batch logs to ", config.skywalking.endpoint_addr)
+
+    if ((not port) and url_decoded.scheme == "https") then
+        port = 443
+    elseif not port then
+        port = 80
+    end
+
+    local httpc = http.new()
+    httpc:set_timeout(config.timeout * 1000)
+    local ok, err = httpc:connect(host, port)
+
+    if not ok then
+        return false, "failed to connect to host[" .. host .. "] port["
+            .. tostring(port) .. "] " .. err
+    end
+
+    if url_decoded.scheme == "https" then
+        ok, err = httpc:ssl_handshake(true, host, false)
+        if not ok then
+            return nil, "failed to perform SSL with host[" .. host .. "] "
+                .. "port[" .. tostring(port) .. "] " .. err
+        end
+    end
+
+    local entries = {}
+    for i = 1, #log_message, 2 do
+        local content = {
+            service = config.skywalking.service_name,
+            serviceInstance = config.skywalking.service_instance_name,
+            endpoint = "",
+            body = {
+                text = {
+                    text = log_message[i]
+                }
+           }
+        }
+        table.insert(entries, content)
+    end
+
+    local httpc_res, httpc_err = httpc:request({

Review comment:
       Why not use `httpc:request_uri` directly?

##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +38,40 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,
-        port = {type = "integer", minimum = 0},
-        tls = {type = "boolean", default = false},
-        tls_server_name = {type = "string"},
-        timeout = {type = "integer", minimum = 1, default = 3},
-        keepalive = {type = "integer", minimum = 1, default = 30},
+        type = {type = "string", default = "TCP", enum = {"TCP", "SKYWALKING"}},
+        tcp = {
+            type = "object",
+            properties = {
+                host = schema_def.host_def,
+                port = {type = "integer", minimum = 0},
+                tls = {type = "boolean", default = false},
+                tls_server_name = {type = "string"},
+                keepalive = {type = "integer", minimum = 1, default = 30},
+            },
+	    required = {"host", "port"}

Review comment:
       Please don't mix tab with space

##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -204,4 +316,4 @@ function _M.destroy()
 end
 
 
-return _M
+return _M

Review comment:
       Need newline at the end of file




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r676797377



##########
File path: apisix/plugins/log-generator.lua
##########
@@ -0,0 +1,63 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local ngx = ngx
+local core = require("apisix.core")
+
+local schema = {
+    type = "object",
+    properties = {
+
+    },
+    required = {},
+}
+
+
+local plugin_name = "log-generator"
+
+local _M = {
+    version = 0.1,
+    priority = 0,        -- TODO: add a type field, may be a good idea
+    name = plugin_name,
+    schema = schema
+}
+
+
+function _M.init()
+    local handler
+    handler = function (premature)
+        if premature then
+            return
+        end
+        core.log.warn("log something for your testing. 1")
+        core.log.warn("log something for your testing. 2")

Review comment:
       It is a mistake.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r681385684



##########
File path: docs/en/latest/plugins/error-log-logger.md
##########
@@ -30,31 +30,38 @@ title: error-log-logger
 
 ## Name
 
-`error-log-logger` is a plugin which pushes the log data of APISIX's error.log to TCP servers.
+`error-log-logger` is a plugin which pushes the log data of APISIX's `error.log` to TCP servers or [Apache SkyWalking](https://skywalking.apache.org/).
 
-This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers.
+This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers, and SkyWalking over HTTP.
 
-This plugin provides the ability as a batch to push the log data to your external TCP servers. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
+This plugin provides the ability as a batch to push the log data to your external TCP servers or monitoring tools. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
 
 For more info on Batch-Processor in Apache APISIX please refer.
 [Batch-Processor](../batch-processor.md)
 
 ## Attributes
 
-| Name             | Type    | Requirement | Default | Valid   | Description                                                                              |
-| ---------------- | ------- | ----------- | ------- | ------- | ---------------------------------------------------------------------------------------- |
-| host             | string  | required    |         |         | IP address or the Hostname of the TCP server.                                            |
-| port             | integer | required    |         | [0,...] | Target upstream port.                                                                    |
-| timeout          | integer | optional    | 3       | [1,...] | Timeout for the upstream to connect and send, unit: second.                                                   |
-| keepalive        | integer | optional    | 30      | [1,...] | Time for keeping the cosocket alive, unit: second.                                                   |
-| level            | string  | optional    | WARN    |         | The filter's log level, default warn, choose the level in ["STDERR", "EMERG", "ALERT", "CRIT", "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"], the value ERR equals ERROR.         |
-| tls              | boolean | optional    | false   |         | Control whether to perform SSL verification                                              |
-| tls_server_name  | string  | optional    |         |         | The server name for the new TLS extension  SNI                                                            |
-| batch_max_size   | integer | optional    | 1000    | [1,...] | Max size of each batch                                                                   |
-| inactive_timeout | integer | optional    | 3       | [1,...] | Maximum age in seconds when the buffer will be flushed if inactive                       |
-| buffer_duration  | integer | optional    | 60      | [1,...] | Maximum age in seconds of the oldest entry in a batch before the batch must be processed |
-| max_retry_count  | integer | optional    | 0       | [0,...] | Maximum number of retries before removing from the processing pipe line                  |
-| retry_delay      | integer | optional    | 1       | [0,...] | Number of seconds the process execution should be delayed if the execution fails         |
+| Name                             | Type    | Requirement | Default                        | Valid   | Description                                                                                          |
+| -------------------------------- | ------- | ----------- | ------------------------------ | ------- | ---------------------------------------------------------------------------------------------------- |
+| tcp.host                         | string  | required    |                                |         | IP address or the Hostname of the TCP server.                                                        |
+| tcp.port                         | integer | required    |                                | [0,...] | Target upstream port.                                                                                |
+| tcp.tls                          | boolean | optional    | false                          |         | Control whether to perform SSL verification.                                                         |
+| tcp.tls_server_name              | string  | optional    |                                |         | The server name for the new TLS extension  SNI.                                                      |
+| skywalking.endpoint_addr         | string  | required    | http://127.0.0.1:12900/v3/logs |         | the http endpoint of Skywalking.                                                                     |

Review comment:
       Got it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tzssangglass commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r677176804



##########
File path: t/plugin/error-log-logger-skywalking.t
##########
@@ -0,0 +1,201 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/

Review comment:
       ```suggestion
   qr/failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\]/
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r672881807



##########
File path: apisix/plugins/error-log-skywalking-logger.lua
##########
@@ -0,0 +1,260 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local errlog = require("ngx.errlog")
+local batch_processor = require("apisix.utils.batch-processor")
+local plugin = require("apisix.plugin")
+local timers = require("apisix.timers")
+local http = require("resty.http")
+local url = require("net.url")
+local plugin_name = "error-log-skywalking-logger"
+local table = core.table
+local schema_def = core.schema
+local ngx = ngx
+local string = string
+local tostring = tostring
+local ipairs  = ipairs
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 32
+})
+
+
+local metadata_schema = {
+    type = "object",
+    properties = {
+        endpoint = schema_def.uri,
+        service_name = {type = "string", default = "APISIX"},
+        service_instance_name = {type="string", default = "APISIX Service Instance"},
+        timeout = {type = "integer", minimum = 1, default = 3},
+        keepalive = {type = "integer", minimum = 1, default = 30},
+        level = {type = "string", default = "WARN", enum = {"STDERR", "EMERG", "ALERT", "CRIT",
+                "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"}},
+        batch_max_size = {type = "integer", minimum = 0, default = 1000},
+        max_retry_count = {type = "integer", minimum = 0, default = 0},
+        retry_delay = {type = "integer", minimum = 0, default = 1},
+        buffer_duration = {type = "integer", minimum = 1, default = 60},
+        inactive_timeout = {type = "integer", minimum = 1, default = 3},
+    },
+    required = {"endpoint"}
+}
+
+
+local schema = {
+    type = "object",
+}
+
+
+local log_level = {

Review comment:
       What about extending the error-log plugin to send skywalking format error log? Therefore we can avoid copying lots of code.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r681362436



##########
File path: t/plugin/error-log-logger.t
##########
@@ -281,7 +287,115 @@ passed
 
 
 
-=== TEST 9: want to reload the plugin by route
+=== TEST 9: log a warn level message (schema compatibility testing)

Review comment:
       Better to add test at the end of file

##########
File path: docs/en/latest/plugins/error-log-logger.md
##########
@@ -30,31 +30,38 @@ title: error-log-logger
 
 ## Name
 
-`error-log-logger` is a plugin which pushes the log data of APISIX's error.log to TCP servers.
+`error-log-logger` is a plugin which pushes the log data of APISIX's `error.log` to TCP servers or [Apache SkyWalking](https://skywalking.apache.org/).
 
-This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers.
+This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers, and SkyWalking over HTTP.
 
-This plugin provides the ability as a batch to push the log data to your external TCP servers. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
+This plugin provides the ability as a batch to push the log data to your external TCP servers or monitoring tools. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
 
 For more info on Batch-Processor in Apache APISIX please refer.
 [Batch-Processor](../batch-processor.md)
 
 ## Attributes
 
-| Name             | Type    | Requirement | Default | Valid   | Description                                                                              |
-| ---------------- | ------- | ----------- | ------- | ------- | ---------------------------------------------------------------------------------------- |
-| host             | string  | required    |         |         | IP address or the Hostname of the TCP server.                                            |
-| port             | integer | required    |         | [0,...] | Target upstream port.                                                                    |
-| timeout          | integer | optional    | 3       | [1,...] | Timeout for the upstream to connect and send, unit: second.                                                   |
-| keepalive        | integer | optional    | 30      | [1,...] | Time for keeping the cosocket alive, unit: second.                                                   |
-| level            | string  | optional    | WARN    |         | The filter's log level, default warn, choose the level in ["STDERR", "EMERG", "ALERT", "CRIT", "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"], the value ERR equals ERROR.         |
-| tls              | boolean | optional    | false   |         | Control whether to perform SSL verification                                              |
-| tls_server_name  | string  | optional    |         |         | The server name for the new TLS extension  SNI                                                            |
-| batch_max_size   | integer | optional    | 1000    | [1,...] | Max size of each batch                                                                   |
-| inactive_timeout | integer | optional    | 3       | [1,...] | Maximum age in seconds when the buffer will be flushed if inactive                       |
-| buffer_duration  | integer | optional    | 60      | [1,...] | Maximum age in seconds of the oldest entry in a batch before the batch must be processed |
-| max_retry_count  | integer | optional    | 0       | [0,...] | Maximum number of retries before removing from the processing pipe line                  |
-| retry_delay      | integer | optional    | 1       | [0,...] | Number of seconds the process execution should be delayed if the execution fails         |
+| Name                             | Type    | Requirement | Default                        | Valid   | Description                                                                                          |
+| -------------------------------- | ------- | ----------- | ------------------------------ | ------- | ---------------------------------------------------------------------------------------------------- |
+| tcp.host                         | string  | required    |                                |         | IP address or the Hostname of the TCP server.                                                        |
+| tcp.port                         | integer | required    |                                | [0,...] | Target upstream port.                                                                                |
+| tcp.tls                          | boolean | optional    | false                          |         | Control whether to perform SSL verification.                                                         |
+| tcp.tls_server_name              | string  | optional    |                                |         | The server name for the new TLS extension  SNI.                                                      |
+| skywalking.endpoint_addr         | string  | required    | http://127.0.0.1:12900/v3/logs |         | the http endpoint of Skywalking.                                                                     |

Review comment:
       Once you provide a default value it is no longer required.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r677039740



##########
File path: t/plugin/error-log-logger-skywalking.t
##########
@@ -0,0 +1,201 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 4: put plugin metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+[error]
+
+
+
+=== TEST 5: log an error level message
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*this is an error message for test.*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"APISIX Service Instance\".*/
+--- wait: 5
+
+
+
+=== TEST 6: delete metadata for the plugin, recover to the default
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_DELETE)
+
+            if code >= 300 then
+                ngx.status = code
+            end
+
+            ngx.say(body)
+        }
+    }
+--- request
+GET /tg
+--- response_body
+passed
+--- no_error_log
+[error]

Review comment:
       Please configure your editor to add a newline there

##########
File path: docs/en/latest/plugins/error-log-logger.md
##########
@@ -30,31 +30,39 @@ title: error-log-logger
 
 ## Name
 
-`error-log-logger` is a plugin which pushes the log data of APISIX's error.log to TCP servers.
+`error-log-logger` is a plugin which pushes the log data of APISIX's `error.log` to TCP servers or [Apache SkyWalking](https://skywalking.apache.org/).
 
-This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers.
+This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers, and SkyWalking over HTTP.
 
-This plugin provides the ability as a batch to push the log data to your external TCP servers. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
+This plugin provides the ability as a batch to push the log data to your external TCP servers or monitoring tools. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
 
 For more info on Batch-Processor in Apache APISIX please refer.
 [Batch-Processor](../batch-processor.md)
 
 ## Attributes
 
-| Name             | Type    | Requirement | Default | Valid   | Description                                                                              |
-| ---------------- | ------- | ----------- | ------- | ------- | ---------------------------------------------------------------------------------------- |
-| host             | string  | required    |         |         | IP address or the Hostname of the TCP server.                                            |
-| port             | integer | required    |         | [0,...] | Target upstream port.                                                                    |
-| timeout          | integer | optional    | 3       | [1,...] | Timeout for the upstream to connect and send, unit: second.                                                   |
-| keepalive        | integer | optional    | 30      | [1,...] | Time for keeping the cosocket alive, unit: second.                                                   |
-| level            | string  | optional    | WARN    |         | The filter's log level, default warn, choose the level in ["STDERR", "EMERG", "ALERT", "CRIT", "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"], the value ERR equals ERROR.         |
-| tls              | boolean | optional    | false   |         | Control whether to perform SSL verification                                              |
-| tls_server_name  | string  | optional    |         |         | The server name for the new TLS extension  SNI                                                            |
-| batch_max_size   | integer | optional    | 1000    | [1,...] | Max size of each batch                                                                   |
-| inactive_timeout | integer | optional    | 3       | [1,...] | Maximum age in seconds when the buffer will be flushed if inactive                       |
-| buffer_duration  | integer | optional    | 60      | [1,...] | Maximum age in seconds of the oldest entry in a batch before the batch must be processed |
-| max_retry_count  | integer | optional    | 0       | [0,...] | Maximum number of retries before removing from the processing pipe line                  |
-| retry_delay      | integer | optional    | 1       | [0,...] | Number of seconds the process execution should be delayed if the execution fails         |
+| Name                             | Type    | Requirement | Default                        | Valid   | Description                                                                               |
+| -------------------------------- | ------- | ----------- | ------------------------------ | ------- | ----------------------------------------------------------------------------------------- |
+| tcp.host                         | string  | required    |                                |         | IP address or the Hostname of the TCP server.                                             |
+| tcp.port                         | integer | required    |                                | [0,...] | Target upstream port.                                                                     |
+| tcp.tls                          | boolean | optional    | false                          |         | Control whether to perform SSL verification.                                              |
+| tcp.tls_server_name              | string  | optional    |                                |         | The server name for the new TLS extension  SNI.                                           |
+| skywalking.endpoint_addr         | string  | required    | http://127.0.0.1:12900/v3/logs |         | the http endpoint of Skywalking.                                                          |
+| skywalking.service_name          | string  | optional    | APISIX                         |         | service name for skywalking reporter                                                      |
+| skywalking.service_instance_name | String  | optional    | APISIX Instance Name           |         | Service instance name for skywalking reporter, set it to `$hostname` to get local hostname directly.|
+| timeout                          | integer | optional    | 3                              | [1,...] | (`Deprecated`) Timeout for the upstream to connect and send, unit: second.                |
+| host                             | string  | optional    |                                |         | IP address or the Hostname of the TCP server.                                             |

Review comment:
       The host is deprecated but timeout isn't? Also, better to point out what field can be used instead of the deprecated one

##########
File path: t/plugin/error-log-logger.t
##########
@@ -157,8 +159,10 @@ plugins:
             local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
                 ngx.HTTP_PUT,
                 [[{
-                    "host": "127.0.0.1",
-                    "port": 9999,
+                    "tcp": {

Review comment:
       Need to keep some old test to check for the compatibility.

##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -134,6 +229,14 @@ local function update_filter(value)
 end
 
 
+local function send(data)
+    if config.tcp then

Review comment:
       Err... I think we should check skywalking first to avoid breaking the old configuration.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r681647888



##########
File path: docs/en/latest/plugins/error-log-logger.md
##########
@@ -30,31 +30,38 @@ title: error-log-logger
 
 ## Name
 
-`error-log-logger` is a plugin which pushes the log data of APISIX's error.log to TCP servers.
+`error-log-logger` is a plugin which pushes the log data of APISIX's `error.log` to TCP servers or [Apache SkyWalking](https://skywalking.apache.org/).
 
-This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers.
+This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers, and SkyWalking over HTTP.
 
-This plugin provides the ability as a batch to push the log data to your external TCP servers. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
+This plugin provides the ability as a batch to push the log data to your external TCP servers or monitoring tools. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
 
 For more info on Batch-Processor in Apache APISIX please refer.
 [Batch-Processor](../batch-processor.md)
 
 ## Attributes
 
-| Name             | Type    | Requirement | Default | Valid   | Description                                                                              |
-| ---------------- | ------- | ----------- | ------- | ------- | ---------------------------------------------------------------------------------------- |
-| host             | string  | required    |         |         | IP address or the Hostname of the TCP server.                                            |
-| port             | integer | required    |         | [0,...] | Target upstream port.                                                                    |
-| timeout          | integer | optional    | 3       | [1,...] | Timeout for the upstream to connect and send, unit: second.                                                   |
-| keepalive        | integer | optional    | 30      | [1,...] | Time for keeping the cosocket alive, unit: second.                                                   |
-| level            | string  | optional    | WARN    |         | The filter's log level, default warn, choose the level in ["STDERR", "EMERG", "ALERT", "CRIT", "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"], the value ERR equals ERROR.         |
-| tls              | boolean | optional    | false   |         | Control whether to perform SSL verification                                              |
-| tls_server_name  | string  | optional    |         |         | The server name for the new TLS extension  SNI                                                            |
-| batch_max_size   | integer | optional    | 1000    | [1,...] | Max size of each batch                                                                   |
-| inactive_timeout | integer | optional    | 3       | [1,...] | Maximum age in seconds when the buffer will be flushed if inactive                       |
-| buffer_duration  | integer | optional    | 60      | [1,...] | Maximum age in seconds of the oldest entry in a batch before the batch must be processed |
-| max_retry_count  | integer | optional    | 0       | [0,...] | Maximum number of retries before removing from the processing pipe line                  |
-| retry_delay      | integer | optional    | 1       | [0,...] | Number of seconds the process execution should be delayed if the execution fails         |
+| Name                             | Type    | Requirement | Default                        | Valid   | Description                                                                                          |
+| -------------------------------- | ------- | ----------- | ------------------------------ | ------- | ---------------------------------------------------------------------------------------------------- |
+| tcp.host                         | string  | required    |                                |         | IP address or the Hostname of the TCP server.                                                        |
+| tcp.port                         | integer | required    |                                | [0,...] | Target upstream port.                                                                                |
+| tcp.tls                          | boolean | optional    | false                          |         | Control whether to perform SSL verification.                                                         |
+| tcp.tls_server_name              | string  | optional    |                                |         | The server name for the new TLS extension  SNI.                                                      |
+| skywalking.endpoint_addr         | string  | optional   | http://127.0.0.1:12900/v3/logs |         | the http endpoint of Skywalking.                                                                     |
+| skywalking.service_name          | string  | optional    | APISIX                         |         | service name for skywalking reporter                                                                 |
+| skywalking.service_instance_name | String  | optional    | APISIX Instance Name           |         | Service instance name for skywalking reporter, set it to `$hostname` to get local hostname directly. |
+| host                             | string  | optional    |                                |         | (`Deprecated`, replaced with `tcp.host`) IP address or the Hostname of the TCP server.               |

Review comment:
       ```suggestion
   | host                             | string  | optional    |                                |         | (`Deprecated`, use `tcp.host` instead) IP address or the Hostname of the TCP server.               |
   ```

##########
File path: docs/en/latest/plugins/error-log-logger.md
##########
@@ -30,31 +30,38 @@ title: error-log-logger
 
 ## Name
 
-`error-log-logger` is a plugin which pushes the log data of APISIX's error.log to TCP servers.
+`error-log-logger` is a plugin which pushes the log data of APISIX's `error.log` to TCP servers or [Apache SkyWalking](https://skywalking.apache.org/).
 
-This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers.
+This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers, and SkyWalking over HTTP.
 
-This plugin provides the ability as a batch to push the log data to your external TCP servers. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
+This plugin provides the ability as a batch to push the log data to your external TCP servers or monitoring tools. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
 
 For more info on Batch-Processor in Apache APISIX please refer.
 [Batch-Processor](../batch-processor.md)
 
 ## Attributes
 
-| Name             | Type    | Requirement | Default | Valid   | Description                                                                              |
-| ---------------- | ------- | ----------- | ------- | ------- | ---------------------------------------------------------------------------------------- |
-| host             | string  | required    |         |         | IP address or the Hostname of the TCP server.                                            |
-| port             | integer | required    |         | [0,...] | Target upstream port.                                                                    |
-| timeout          | integer | optional    | 3       | [1,...] | Timeout for the upstream to connect and send, unit: second.                                                   |
-| keepalive        | integer | optional    | 30      | [1,...] | Time for keeping the cosocket alive, unit: second.                                                   |
-| level            | string  | optional    | WARN    |         | The filter's log level, default warn, choose the level in ["STDERR", "EMERG", "ALERT", "CRIT", "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"], the value ERR equals ERROR.         |
-| tls              | boolean | optional    | false   |         | Control whether to perform SSL verification                                              |
-| tls_server_name  | string  | optional    |         |         | The server name for the new TLS extension  SNI                                                            |
-| batch_max_size   | integer | optional    | 1000    | [1,...] | Max size of each batch                                                                   |
-| inactive_timeout | integer | optional    | 3       | [1,...] | Maximum age in seconds when the buffer will be flushed if inactive                       |
-| buffer_duration  | integer | optional    | 60      | [1,...] | Maximum age in seconds of the oldest entry in a batch before the batch must be processed |
-| max_retry_count  | integer | optional    | 0       | [0,...] | Maximum number of retries before removing from the processing pipe line                  |
-| retry_delay      | integer | optional    | 1       | [0,...] | Number of seconds the process execution should be delayed if the execution fails         |
+| Name                             | Type    | Requirement | Default                        | Valid   | Description                                                                                          |
+| -------------------------------- | ------- | ----------- | ------------------------------ | ------- | ---------------------------------------------------------------------------------------------------- |
+| tcp.host                         | string  | required    |                                |         | IP address or the Hostname of the TCP server.                                                        |
+| tcp.port                         | integer | required    |                                | [0,...] | Target upstream port.                                                                                |
+| tcp.tls                          | boolean | optional    | false                          |         | Control whether to perform SSL verification.                                                         |
+| tcp.tls_server_name              | string  | optional    |                                |         | The server name for the new TLS extension  SNI.                                                      |
+| skywalking.endpoint_addr         | string  | optional   | http://127.0.0.1:12900/v3/logs |         | the http endpoint of Skywalking.                                                                     |
+| skywalking.service_name          | string  | optional    | APISIX                         |         | service name for skywalking reporter                                                                 |
+| skywalking.service_instance_name | String  | optional    | APISIX Instance Name           |         | Service instance name for skywalking reporter, set it to `$hostname` to get local hostname directly. |
+| host                             | string  | optional    |                                |         | (`Deprecated`, replaced with `tcp.host`) IP address or the Hostname of the TCP server.               |
+| port                             | integer | optional    |                                | [0,...] | (`Deprecated`, replaced with `tcp.port`) Target upstream port.                                       |
+| tls                              | boolean | optional    | false                          |         | (`Deprecated`, replaced with `tcp.tls`) Control whether to perform SSL verification.                 |

Review comment:
       ```suggestion
   | tls                              | boolean | optional    | false                          |         | (`Deprecated`, use `tcp.tls` instead) Control whether to perform SSL verification.                 |
   ```

##########
File path: docs/en/latest/plugins/error-log-logger.md
##########
@@ -30,31 +30,38 @@ title: error-log-logger
 
 ## Name
 
-`error-log-logger` is a plugin which pushes the log data of APISIX's error.log to TCP servers.
+`error-log-logger` is a plugin which pushes the log data of APISIX's `error.log` to TCP servers or [Apache SkyWalking](https://skywalking.apache.org/).
 
-This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers.
+This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers, and SkyWalking over HTTP.
 
-This plugin provides the ability as a batch to push the log data to your external TCP servers. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
+This plugin provides the ability as a batch to push the log data to your external TCP servers or monitoring tools. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
 
 For more info on Batch-Processor in Apache APISIX please refer.
 [Batch-Processor](../batch-processor.md)
 
 ## Attributes
 
-| Name             | Type    | Requirement | Default | Valid   | Description                                                                              |
-| ---------------- | ------- | ----------- | ------- | ------- | ---------------------------------------------------------------------------------------- |
-| host             | string  | required    |         |         | IP address or the Hostname of the TCP server.                                            |
-| port             | integer | required    |         | [0,...] | Target upstream port.                                                                    |
-| timeout          | integer | optional    | 3       | [1,...] | Timeout for the upstream to connect and send, unit: second.                                                   |
-| keepalive        | integer | optional    | 30      | [1,...] | Time for keeping the cosocket alive, unit: second.                                                   |
-| level            | string  | optional    | WARN    |         | The filter's log level, default warn, choose the level in ["STDERR", "EMERG", "ALERT", "CRIT", "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"], the value ERR equals ERROR.         |
-| tls              | boolean | optional    | false   |         | Control whether to perform SSL verification                                              |
-| tls_server_name  | string  | optional    |         |         | The server name for the new TLS extension  SNI                                                            |
-| batch_max_size   | integer | optional    | 1000    | [1,...] | Max size of each batch                                                                   |
-| inactive_timeout | integer | optional    | 3       | [1,...] | Maximum age in seconds when the buffer will be flushed if inactive                       |
-| buffer_duration  | integer | optional    | 60      | [1,...] | Maximum age in seconds of the oldest entry in a batch before the batch must be processed |
-| max_retry_count  | integer | optional    | 0       | [0,...] | Maximum number of retries before removing from the processing pipe line                  |
-| retry_delay      | integer | optional    | 1       | [0,...] | Number of seconds the process execution should be delayed if the execution fails         |
+| Name                             | Type    | Requirement | Default                        | Valid   | Description                                                                                          |
+| -------------------------------- | ------- | ----------- | ------------------------------ | ------- | ---------------------------------------------------------------------------------------------------- |
+| tcp.host                         | string  | required    |                                |         | IP address or the Hostname of the TCP server.                                                        |
+| tcp.port                         | integer | required    |                                | [0,...] | Target upstream port.                                                                                |
+| tcp.tls                          | boolean | optional    | false                          |         | Control whether to perform SSL verification.                                                         |
+| tcp.tls_server_name              | string  | optional    |                                |         | The server name for the new TLS extension  SNI.                                                      |
+| skywalking.endpoint_addr         | string  | optional   | http://127.0.0.1:12900/v3/logs |         | the http endpoint of Skywalking.                                                                     |
+| skywalking.service_name          | string  | optional    | APISIX                         |         | service name for skywalking reporter                                                                 |
+| skywalking.service_instance_name | String  | optional    | APISIX Instance Name           |         | Service instance name for skywalking reporter, set it to `$hostname` to get local hostname directly. |
+| host                             | string  | optional    |                                |         | (`Deprecated`, replaced with `tcp.host`) IP address or the Hostname of the TCP server.               |
+| port                             | integer | optional    |                                | [0,...] | (`Deprecated`, replaced with `tcp.port`) Target upstream port.                                       |
+| tls                              | boolean | optional    | false                          |         | (`Deprecated`, replaced with `tcp.tls`) Control whether to perform SSL verification.                 |
+| tls_server_name                  | string  | optional    |                                |         | (`Deprecated`, replaced with `tcp.tls_server_name`) The server name for the new TLS extension SNI.   |

Review comment:
       ```suggestion
   | tls_server_name                  | string  | optional    |                                |         | (`Deprecated`, use `tcp.tls_server_name` instead) The server name for the new TLS extension SNI.   |
   ```

##########
File path: docs/en/latest/plugins/error-log-logger.md
##########
@@ -30,31 +30,38 @@ title: error-log-logger
 
 ## Name
 
-`error-log-logger` is a plugin which pushes the log data of APISIX's error.log to TCP servers.
+`error-log-logger` is a plugin which pushes the log data of APISIX's `error.log` to TCP servers or [Apache SkyWalking](https://skywalking.apache.org/).
 
-This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers.
+This plugin will provide the ability to send the log data which selected by the level to Monitoring tools and other TCP servers, and SkyWalking over HTTP.
 
-This plugin provides the ability as a batch to push the log data to your external TCP servers. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
+This plugin provides the ability as a batch to push the log data to your external TCP servers or monitoring tools. If not receive the log data, don't worry, it will automatically send the logs after the timer function expires in our Batch Processor.
 
 For more info on Batch-Processor in Apache APISIX please refer.
 [Batch-Processor](../batch-processor.md)
 
 ## Attributes
 
-| Name             | Type    | Requirement | Default | Valid   | Description                                                                              |
-| ---------------- | ------- | ----------- | ------- | ------- | ---------------------------------------------------------------------------------------- |
-| host             | string  | required    |         |         | IP address or the Hostname of the TCP server.                                            |
-| port             | integer | required    |         | [0,...] | Target upstream port.                                                                    |
-| timeout          | integer | optional    | 3       | [1,...] | Timeout for the upstream to connect and send, unit: second.                                                   |
-| keepalive        | integer | optional    | 30      | [1,...] | Time for keeping the cosocket alive, unit: second.                                                   |
-| level            | string  | optional    | WARN    |         | The filter's log level, default warn, choose the level in ["STDERR", "EMERG", "ALERT", "CRIT", "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"], the value ERR equals ERROR.         |
-| tls              | boolean | optional    | false   |         | Control whether to perform SSL verification                                              |
-| tls_server_name  | string  | optional    |         |         | The server name for the new TLS extension  SNI                                                            |
-| batch_max_size   | integer | optional    | 1000    | [1,...] | Max size of each batch                                                                   |
-| inactive_timeout | integer | optional    | 3       | [1,...] | Maximum age in seconds when the buffer will be flushed if inactive                       |
-| buffer_duration  | integer | optional    | 60      | [1,...] | Maximum age in seconds of the oldest entry in a batch before the batch must be processed |
-| max_retry_count  | integer | optional    | 0       | [0,...] | Maximum number of retries before removing from the processing pipe line                  |
-| retry_delay      | integer | optional    | 1       | [0,...] | Number of seconds the process execution should be delayed if the execution fails         |
+| Name                             | Type    | Requirement | Default                        | Valid   | Description                                                                                          |
+| -------------------------------- | ------- | ----------- | ------------------------------ | ------- | ---------------------------------------------------------------------------------------------------- |
+| tcp.host                         | string  | required    |                                |         | IP address or the Hostname of the TCP server.                                                        |
+| tcp.port                         | integer | required    |                                | [0,...] | Target upstream port.                                                                                |
+| tcp.tls                          | boolean | optional    | false                          |         | Control whether to perform SSL verification.                                                         |
+| tcp.tls_server_name              | string  | optional    |                                |         | The server name for the new TLS extension  SNI.                                                      |
+| skywalking.endpoint_addr         | string  | optional   | http://127.0.0.1:12900/v3/logs |         | the http endpoint of Skywalking.                                                                     |
+| skywalking.service_name          | string  | optional    | APISIX                         |         | service name for skywalking reporter                                                                 |
+| skywalking.service_instance_name | String  | optional    | APISIX Instance Name           |         | Service instance name for skywalking reporter, set it to `$hostname` to get local hostname directly. |
+| host                             | string  | optional    |                                |         | (`Deprecated`, replaced with `tcp.host`) IP address or the Hostname of the TCP server.               |
+| port                             | integer | optional    |                                | [0,...] | (`Deprecated`, replaced with `tcp.port`) Target upstream port.                                       |

Review comment:
       ```suggestion
   | port                             | integer | optional    |                                | [0,...] | (`Deprecated`, use  `tcp.port` instead) Target upstream port.                                       |
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r673043567



##########
File path: apisix/plugins/error-log-skywalking-logger.lua
##########
@@ -0,0 +1,260 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local core = require("apisix.core")
+local errlog = require("ngx.errlog")
+local batch_processor = require("apisix.utils.batch-processor")
+local plugin = require("apisix.plugin")
+local timers = require("apisix.timers")
+local http = require("resty.http")
+local url = require("net.url")
+local plugin_name = "error-log-skywalking-logger"
+local table = core.table
+local schema_def = core.schema
+local ngx = ngx
+local string = string
+local tostring = tostring
+local ipairs  = ipairs
+local lrucache = core.lrucache.new({
+    ttl = 300, count = 32
+})
+
+
+local metadata_schema = {
+    type = "object",
+    properties = {
+        endpoint = schema_def.uri,
+        service_name = {type = "string", default = "APISIX"},
+        service_instance_name = {type="string", default = "APISIX Service Instance"},
+        timeout = {type = "integer", minimum = 1, default = 3},
+        keepalive = {type = "integer", minimum = 1, default = 30},
+        level = {type = "string", default = "WARN", enum = {"STDERR", "EMERG", "ALERT", "CRIT",
+                "ERR", "ERROR", "WARN", "NOTICE", "INFO", "DEBUG"}},
+        batch_max_size = {type = "integer", minimum = 0, default = 1000},
+        max_retry_count = {type = "integer", minimum = 0, default = 0},
+        retry_delay = {type = "integer", minimum = 0, default = 1},
+        buffer_duration = {type = "integer", minimum = 1, default = 60},
+        inactive_timeout = {type = "integer", minimum = 1, default = 3},
+    },
+    required = {"endpoint"}
+}
+
+
+local schema = {
+    type = "object",
+}
+
+
+local log_level = {

Review comment:
       Agree, and this extension might be reused by many other plugins in the future.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r681383121



##########
File path: t/plugin/error-log-logger.t
##########
@@ -281,7 +287,115 @@ passed
 
 
 
-=== TEST 9: want to reload the plugin by route
+=== TEST 9: log a warn level message (schema compatibility testing)

Review comment:
       we have resetting environment tasks at the end.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r676797798



##########
File path: t/plugin/error-log-skywalking-logger.t
##########
@@ -0,0 +1,357 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('debug');
+repeat_each(1);
+no_long_string();
+no_root_location();
+run_tests;
+
+__DATA__
+
+=== TEST 1: test schema checker
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        endpoint_addr = "http://127.0.0.1"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: test schema checker, missing required field
+--- config
+    location /t {
+        content_by_lua_block {
+        local core = require("apisix.core")
+            local plugin = require("apisix.plugins.error-log-logger")
+            local ok, err = plugin.check_schema(
+                {
+                    type = "SKYWALKING",
+                    skywalking = {
+                        service = "APISIX"
+                    }
+                },
+                core.schema.TYPE_METADATA
+            )
+            if not ok then
+                ngx.say(err)
+            end
+
+            ngx.say("done")
+        }
+    }
+--- request
+GET /t
+--- response_body
+property "skywalking" validation failed: property "endpoint_addr" is required
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: not enable the plugin
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+error-log-logger
+--- wait: 2
+
+
+
+=== TEST 4: enable the plugin, but not init the metadata
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 5: set a wrong metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "port": 1999,
+                    "inactive_timeout": 1
+                }]]
+                )
+
+            -- ensure the request is rejected even this plugin doesn't
+            -- have check_schema method
+            ngx.status = code
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- error_code: 201
+--- response_body
+--- error_log eval
+qr/please set the correct plugin_metadata for error-log-logger/
+--- wait: 2
+
+
+
+=== TEST 6: test unreachable server
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1988/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+            ngx.sleep(2)
+            core.log.warn("this is a warning message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[lua\] batch-processor.lua:63: Batch Processor\[error-log-logger\] failed to process entries: error while sending data to skywalking\[http:\/\/127.0.0.1:1988\/log\] connection refused, context: ngx.timer/
+--- wait: 3
+
+
+
+=== TEST 7: put plugin metadata
+--- yaml_config
+apisix:
+    enable_admin: true
+    admin_key: null
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
+                ngx.HTTP_PUT,
+                [[{
+                    "type": "SKYWALKING",
+                    "skywalking": {
+                        "endpoint_addr": "http://127.0.0.1:1982/log"
+                    },
+                    "inactive_timeout": 1
+                }]]
+                )
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: log an error level message
+--- yaml_config
+plugins:
+  - error-log-logger
+--- config
+    location /tg {
+        content_by_lua_block {
+            local core = require("apisix.core")
+            ngx.sleep(2)
+            core.log.error("this is an error message for test.")
+        }
+    }
+--- request
+GET /tg
+--- response_body
+--- error_log eval
+qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"APISIX Service Instance\".*/

Review comment:
       Fixed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#issuecomment-884621382


   If these changes are ok in basically, I will update the related docs and tests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r674451932



##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +38,40 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,
-        port = {type = "integer", minimum = 0},
-        tls = {type = "boolean", default = false},
-        tls_server_name = {type = "string"},
-        timeout = {type = "integer", minimum = 1, default = 3},
-        keepalive = {type = "integer", minimum = 1, default = 30},
+        type = {type = "string", default = "TCP", enum = {"TCP", "SKYWALKING"}},

Review comment:
        This is a definitely broken change, we may need some further discussions.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r674463021



##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +38,40 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,

Review comment:
       got it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r674463474



##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +38,40 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,
-        port = {type = "integer", minimum = 0},
-        tls = {type = "boolean", default = false},
-        tls_server_name = {type = "string"},
-        timeout = {type = "integer", minimum = 1, default = 3},
-        keepalive = {type = "integer", minimum = 1, default = 30},
+        type = {type = "string", default = "TCP", enum = {"TCP", "SKYWALKING"}},

Review comment:
       I will revert the remove operation and mark them as depreceated.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] dmsolr commented on a change in pull request #4633: feat: add error log skywalking reporter

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4633:
URL: https://github.com/apache/apisix/pull/4633#discussion_r678002585



##########
File path: apisix/plugins/error-log-logger.lua
##########
@@ -36,23 +37,52 @@ local lrucache = core.lrucache.new({
 local metadata_schema = {
     type = "object",
     properties = {
-        host = schema_def.host_def,
-        port = {type = "integer", minimum = 0},
-        tls = {type = "boolean", default = false},
-        tls_server_name = {type = "string"},
-        timeout = {type = "integer", minimum = 1, default = 3},
-        keepalive = {type = "integer", minimum = 1, default = 30},
+        type = {type = "string", default = "TCP", enum = {"TCP", "SKYWALKING"}},

Review comment:
       Yes, I will remove it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org