Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(plugins)(datadog): add a datadog tag for the route name to each metric #13494

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+104 −3
Open

feat(plugins)(datadog): add a datadog tag for the route name to each metric #13494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9afb491
feat(plugins)(datadog): add a datadog tag for the route name to each …
tysoekong Aug 13, 2024
22f9497
feat(plugins)(datadog): add a datadog tag for the route name to each …
tysoekong Sep 27, 2024
dd625a6
feat(plugins)(datadog): add a datadog tag for the route name to each …
tysoekong Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/kong/datadog-plugin-route-name-tag.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
message: |
Added the route name to a built-in tag in the Datadog plugin,
to each of the output metrics.
type: feature
scope: Plugin
10 changes: 7 additions & 3 deletions kong/plugins/datadog/handler.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ local get_consumer_id = {
}


local function compose_tags(service_name, status, consumer_id, tags, conf)
local function compose_tags(service_name, status, consumer_id, tags, conf, route_name)
local result = {
(conf.service_name_tag or "name") .. ":" .. service_name,
(conf.status_tag or "status") .. ":" .. status
(conf.status_tag or "status") .. ":" .. status,
route_name and route_name ~= "" and ((conf.route_name_tag or "route") .. ":" .. route_name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the checks route_name and route_name ~= "" can be omitted right? We know it'll either have a value or be "", and in case it's "" we probably want to just use it like that, or else this evaluation will return false, am I right?

}

if consumer_id ~= nil then
Expand Down Expand Up @@ -86,7 +87,10 @@ local function send_entries_to_datadog(conf, messages)
message.service and gsub(message.service.name ~= null and
message.service.name or message.service.host, "%.", "_") or "",
message.response and message.response.status or "-",
consumer_id, metric_config.tags, conf)
consumer_id, metric_config.tags,
conf,
message.route and (message.route.name and gsub(message.route.name ~= null and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this passing false to gsub when message.route.name is null?
When it gets this complex I'd rather be verbose and write down the logic above, assigning the final value to some local variable, and then just pass it to compose_tags, wdyt?

message.route.name, "%.", "_") or message.route.id) or "")

logger:send_statsd(stat_name, stat_value,
logger.stat_types[metric_config.stat_type],
Expand Down
3 changes: 3 additions & 0 deletions kong/plugins/datadog/schema.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ return {
{
service_name_tag = { description = "String to be attached as the name of the service.", type = "string",
default = "name" }, },
{
route_name_tag = { description = "String to be attached as the name of the route.", type = "string",
default = "route" }, },
{
status_tag = { description = "String to be attached as the tag of the HTTP status.", type = "string",
default = "status" }, },
Expand Down
89 changes: 89 additions & 0 deletions spec/03-plugins/08-datadog/01-log_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,18 @@ describe("Plugin: datadog (log)", function()
paths = { "/serviceless" },
no_service = true,
}

local route10 = bp.routes:insert {
name = "kong-sample_route",
hosts = { "datadog10.test" },
service = bp.services:insert { name = "dd10" }
}

local route11 = bp.routes:insert {
name = "kong-sample_route.11",
hosts = { "datadog11.test" },
service = bp.services:insert { name = "dd11" }
}

bp.plugins:insert {
name = "key-auth",
Expand Down Expand Up @@ -263,6 +275,35 @@ describe("Plugin: datadog (log)", function()
}
}

bp.plugins:insert {
name = "key-auth",
route = { id = route10.id },
}

bp.plugins:insert {
name = "datadog",
route = { id = route10.id },
config = {
host = "127.0.0.1",
port = 9999,
},
}

bp.plugins:insert {
name = "key-auth",
route = { id = route11.id },
}

bp.plugins:insert {
name = "datadog",
route = { id = route11.id },
config = {
host = "127.0.0.1",
port = 9999,
route_name_tag = "kong_route_name_tag",
},
}

assert(helpers.start_kong({
database = strategy,
nginx_conf = "spec/fixtures/custom_nginx.template",
Expand Down Expand Up @@ -528,6 +569,54 @@ describe("Plugin: datadog (log)", function()
assert.equal(DEFAULT_METRICS_COUNT, #gauges)
end)

it("logs metrics with route name over UDP", function()
local thread = helpers.udp_server(9999, DEFAULT_METRICS_COUNT)

local res = assert(proxy_client:send {
method = "GET",
path = "/status/200?apikey=kong",
headers = {
["Host"] = "datadog10.test"
}
})
assert.res_status(200, res)

local ok, gauges = thread:join()

assert.True(ok)
assert.equal(DEFAULT_METRICS_COUNT, #gauges)
assert.contains("kong.request.count:1|c|#name:dd10,status:200,route:kong-sample_route,consumer:bar,app:kong" , gauges)
assert.contains("kong.latency:%d*|ms|#name:dd10,status:200,route:kong-sample_route,consumer:bar,app:kong", gauges, true)
assert.contains("kong.request.size:%d*|ms|#name:dd10,status:200,route:kong-sample_route,consumer:bar,app:kong", gauges, true)
assert.contains("kong.response.size:%d*|ms|#name:dd10,status:200,route:kong-sample_route,consumer:bar,app:kong", gauges, true)
assert.contains("kong.upstream_latency:%d*|ms|#name:dd10,status:200,route:kong-sample_route,consumer:bar,app:kong", gauges, true)
assert.contains("kong.kong_latency:%d*|ms|#name:dd10,status:200,route:kong-sample_route,consumer:bar,app:kong", gauges, true)
end)

it("logs metrics with different route name tag over UDP", function()
local thread = helpers.udp_server(9999, DEFAULT_METRICS_COUNT)

local res = assert(proxy_client:send {
method = "GET",
path = "/status/200?apikey=kong",
headers = {
["Host"] = "datadog11.test"
}
})
assert.res_status(200, res)

local ok, gauges = thread:join()

assert.True(ok)
assert.equal(DEFAULT_METRICS_COUNT, #gauges)
assert.contains("kong.request.count:1|c|#name:dd11,status:200,kong_route_name_tag:kong-sample_route_11,consumer:bar,app:kong" , gauges)
assert.contains("kong.latency:%d+|ms|#name:dd11,status:200,kong_route_name_tag:kong-sample_route_11,consumer:bar,app:kong", gauges, true)
assert.contains("kong.request.size:%d+|ms|#name:dd11,status:200,kong_route_name_tag:kong-sample_route_11,consumer:bar,app:kong", gauges, true)
assert.contains("kong.response.size:%d+|ms|#name:dd11,status:200,kong_route_name_tag:kong-sample_route_11,consumer:bar,app:kong", gauges, true)
assert.contains("kong.upstream_latency:%d+|ms|#name:dd11,status:200,kong_route_name_tag:kong-sample_route_11,consumer:bar,app:kong", gauges, true)
assert.contains("kong.kong_latency:%d*|ms|#name:dd11,status:200,kong_route_name_tag:kong-sample_route_11,consumer:bar,app:kong", gauges, true)
end)

it("datadog plugin is triggered for serviceless routes", function()
local thread = helpers.udp_server(9999, DEFAULT_METRICS_COUNT)
local res = assert(proxy_client:send {
Expand Down
Loading