You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by sp...@apache.org on 2021/11/11 02:10:40 UTC

[apisix] branch master updated: feat: introducing prefer_name attribute in datadog plugin (#5463)

This is an automated email from the ASF dual-hosted git repository.

spacewander pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git


The following commit(s) were added to refs/heads/master by this push:
     new 7b60f42  feat: introducing prefer_name attribute in datadog plugin (#5463)
7b60f42 is described below

commit 7b60f42785dda9fcd22bf67af3dfac4aca0f9ac8
Author: Bisakh <bi...@gmail.com>
AuthorDate: Thu Nov 11 07:40:30 2021 +0530

    feat: introducing prefer_name attribute in datadog plugin (#5463)
---
 apisix/plugins/datadog.lua        |  25 +++++--
 docs/en/latest/plugins/datadog.md |  20 +++---
 t/plugin/datadog.t                | 138 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 167 insertions(+), 16 deletions(-)

diff --git a/apisix/plugins/datadog.lua b/apisix/plugins/datadog.lua
index 7fe7d3f..0a6a134 100644
--- a/apisix/plugins/datadog.lua
+++ b/apisix/plugins/datadog.lua
@@ -18,6 +18,7 @@ local core = require("apisix.core")
 local plugin = require("apisix.plugin")
 local batch_processor = require("apisix.utils.batch-processor")
 local fetch_log = require("apisix.utils.log-util").get_full_log
+local service_fetch = require("apisix.http.service").get
 local ngx = ngx
 local udp = ngx.socket.udp
 local format = string.format
@@ -43,6 +44,7 @@ local schema = {
         inactive_timeout = {type = "integer", minimum = 1, default = 5},
         batch_max_size = {type = "integer", minimum = 1, default = 5000},
         max_retry_count = {type = "integer", minimum = 1, default = 1},
+        prefer_name = {type = "boolean", default = true}
     }
 }
 
@@ -83,15 +85,12 @@ local function generate_tag(entry, const_tags)
         tags = {}
     end
 
-    -- priority on route name, if not found using the route id.
-    if entry.route_name ~= "" then
-        core.table.insert(tags, "route_name:" .. entry.route_name)
-    elseif entry.route_id and entry.route_id ~= "" then
+    if entry.route_id and entry.route_id ~= "" then
         core.table.insert(tags, "route_name:" .. entry.route_id)
     end
 
     if entry.service_id and entry.service_id ~= "" then
-        core.table.insert(tags, "service_id:" .. entry.service_id)
+        core.table.insert(tags, "service_name:" .. entry.service_id)
     end
 
     if entry.consumer and entry.consumer ~= "" then
@@ -142,9 +141,23 @@ function _M.log(conf, ctx)
     local entry = fetch_log(ngx, {})
     entry.upstream_latency = ctx.var.upstream_response_time * 1000
     entry.balancer_ip = ctx.balancer_ip or ""
-    entry.route_name = ctx.route_name or ""
     entry.scheme = ctx.upstream_scheme or ""
 
+    -- if prefer_name is set, fetch the service/route name. If the name is nil, fall back to id.
+    if conf.prefer_name then
+        if entry.service_id and entry.service_id ~= "" then
+            local svc = service_fetch(entry.service_id)
+
+            if svc and svc.value.name ~= "" then
+                entry.service_id =  svc.value.name
+            end
+        end
+
+        if ctx.route_name and ctx.route_name ~= "" then
+            entry.route_id = ctx.route_name
+        end
+    end
+
     local log_buffer = buffers[conf]
     if log_buffer then
         log_buffer:push(entry)
diff --git a/docs/en/latest/plugins/datadog.md b/docs/en/latest/plugins/datadog.md
index 508ae1c..7842018 100644
--- a/docs/en/latest/plugins/datadog.md
+++ b/docs/en/latest/plugins/datadog.md
@@ -45,12 +45,13 @@ For more info on Batch-Processor in Apache APISIX please refer.
 
 ## Attributes
 
-| Name             | Type   | Requirement  | Default      | Valid | Description                                                                                |
-| -----------      | ------ | -----------  | -------      | ----- | ------------------------------------------------------------                               |
-| batch_max_size   | integer | optional    | 5000         | [1,...] | Max buffer size of each batch                                                            |
-| inactive_timeout | integer | optional    | 5            | [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    | 1            | [1,...] | Maximum number of retries if one entry fails to reach dogstatsd server                   |
+| Name             | Type   | Requirement  | Default      | Valid       | Description                                                                                |
+| -----------      | ------ | -----------  | -------      | -----       | ------------------------------------------------------------                               |
+| prefer_name      | boolean | optional    | true         | true/false  | If set to `false`, would use route/service id instead of name(default) with metric tags.   |
+| batch_max_size   | integer | optional    | 5000         | [1,...]     | Max buffer size of each batch                                                              |
+| inactive_timeout | integer | optional    | 5            | [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    | 1            | [1,...]     | Maximum number of retries if one entry fails to reach dogstatsd server                     |
 
 ## Metadata
 
@@ -80,13 +81,12 @@ The metrics will be sent to the DogStatsD agent with the following tags:
 
 > If there is no suitable value for any particular tag, the tag will simply be omitted.
 
-- **route_name**: Name specified in the route schema definition. If not present, it will fall back to the route id value.
-  - Note: If multiple routes have the same name duplicated, we suggest you to visualize graphs on the Datadog dashboard over multiple tags that could compositely pinpoint a particular route/service. If it's still insufficient for your needs, feel free to drop a feature request at [apisix/issues](https://github.com/apache/apisix/issues).
-- **service_id**: If a route has been created with the abstraction of service, the particular service id will be used.
+- **route_name**: Name specified in the route schema definition. If not present or plugin attribute `prefer_name` is set to `false`, it will fall back to the route id value.
+- **service_name**: If a route has been created with the abstraction of service, the particular service name/id (based on plugin `prefer_name` attribute) will be used.
 - **consumer**: If the route has a linked consumer, the consumer Username will be added as a tag.
 - **balancer_ip**: IP of the Upstream balancer that has processed the current request.
 - **response_status**: HTTP response status code.
-- **scheme**: Scheme that has been used to make the request. e.g. HTTP, gRPC, gRPCs etc.
+- **scheme**: Scheme that has been used to make the request, such as HTTP, gRPC, gRPCs etc.
 
 ## How To Enable
 
diff --git a/t/plugin/datadog.t b/t/plugin/datadog.t
index b307ee7..74dafed 100644
--- a/t/plugin/datadog.t
+++ b/t/plugin/datadog.t
@@ -395,3 +395,141 @@ message received: apisix\.apisix\.latency:[\d.]+\|h\|#source:apisix,new_tag:must
 message received: apisix\.ingress\.size:[\d]+\|ms\|#source:apisix,new_tag:must,route_name:1,balancer_ip:[\d.]+,response_status:200,scheme:http
 message received: apisix\.egress\.size:[\d]+\|ms\|#source:apisix,new_tag:must,route_name:1,balancer_ip:[\d.]+,response_status:200,scheme:http
 /
+
+
+
+=== TEST 8: testing behaviour with service id
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/services/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "name": "service-1",
+                        "plugins": {
+                            "datadog": {
+                                "batch_max_size" : 1
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1982": 1
+                            },
+                            "type": "roundrobin"
+                        }
+                }]]
+            )
+
+            if code >= 300 then
+                ngx.status = code
+                ngx.say("fail")
+                return
+            end
+
+            ngx.say(body)
+
+            -- create a route with service level abstraction
+            local code, body = t('/apisix/admin/routes/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                       "name": "route-1",
+                       "uri": "/opentracing",
+                       "service_id": "1"
+
+                 }]]
+            )
+
+            if code >= 300 then
+                ngx.status = code
+                ngx.say("fail")
+                return
+            end
+
+            ngx.say(body)
+
+            -- making a request to the route
+            local code, _, body = t("/opentracing", "GET")
+             if code >= 300 then
+                ngx.status = code
+                ngx.say("fail")
+                return
+            end
+
+            ngx.print(body)
+        }
+    }
+--- response_body
+passed
+passed
+opentracing
+--- wait: 0.5
+--- grep_error_log eval
+qr/message received: apisix(.+?(?=, ))/
+--- grep_error_log_out eval
+qr/message received: apisix\.request\.counter:1\|c\|#source:apisix,new_tag:must,route_name:route-1,service_name:service-1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.request\.latency:[\d.]+\|h\|#source:apisix,new_tag:must,route_name:route-1,service_name:service-1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.upstream\.latency:[\d.]+\|h\|#source:apisix,new_tag:must,route_name:route-1,service_name:service-1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.apisix\.latency:[\d.]+\|h\|#source:apisix,new_tag:must,route_name:route-1,service_name:service-1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.ingress\.size:[\d]+\|ms\|#source:apisix,new_tag:must,route_name:route-1,service_name:service-1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.egress\.size:[\d]+\|ms\|#source:apisix,new_tag:must,route_name:route-1,service_name:service-1,balancer_ip:[\d.]+,response_status:200,scheme:http
+/
+
+
+
+=== TEST 9: testing behaviour with prefer_name is false and service name is nil
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/services/1',
+                 ngx.HTTP_PUT,
+                 [[{
+                        "plugins": {
+                            "datadog": {
+                                "batch_max_size" : 1,
+                                "prefer_name": false
+                            }
+                        },
+                        "upstream": {
+                            "nodes": {
+                                "127.0.0.1:1982": 1
+                            },
+                            "type": "roundrobin"
+                        }
+                }]]
+            )
+
+            if code >= 300 then
+                ngx.status = code
+                ngx.say("fail")
+                return
+            end
+
+            ngx.say(body)
+
+            -- making a request to the route
+            local code, _, body = t("/opentracing", "GET")
+             if code >= 300 then
+                ngx.status = code
+                ngx.say("fail")
+                return
+            end
+
+            ngx.print(body)
+        }
+    }
+--- response_body
+passed
+opentracing
+--- wait: 0.5
+--- grep_error_log eval
+qr/message received: apisix(.+?(?=, ))/
+--- grep_error_log_out eval
+qr/message received: apisix\.request\.counter:1\|c\|#source:apisix,new_tag:must,route_name:1,service_name:1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.request\.latency:[\d.]+\|h\|#source:apisix,new_tag:must,route_name:1,service_name:1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.upstream\.latency:[\d.]+\|h\|#source:apisix,new_tag:must,route_name:1,service_name:1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.apisix\.latency:[\d.]+\|h\|#source:apisix,new_tag:must,route_name:1,service_name:1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.ingress\.size:[\d]+\|ms\|#source:apisix,new_tag:must,route_name:1,service_name:1,balancer_ip:[\d.]+,response_status:200,scheme:http
+message received: apisix\.egress\.size:[\d]+\|ms\|#source:apisix,new_tag:must,route_name:1,service_name:1,balancer_ip:[\d.]+,response_status:200,scheme:http
+/