DataDog / kong-plugin-ddtrace

Datadog APM Plugin for Kong Gateway
Apache License 2.0
15 stars 7 forks source link

Allow `agent_endpoint` to be referenceable #7

Closed zarakay closed 11 months ago

zarakay commented 11 months ago

Closes #6

When running in a containerized environment, the url to the DataDog Agent may change depending on where the gateway instance is running, which means its hard to predict what the ip address of the agent will be until the container is allocated a host. This becomes more complicated when multiple containers are involved as they may be on different hosts. Traditionally, you run the DataDog agent is a Daemon set with a copy on each instance which hosts a container. What we need is a way to tell the plugin to use the agent which is on the host its running on.

Kong providers a vault mechanism which allows you to configure plugins to reference external configuration engines.

In our deployment of Kong on ECS, we provider a custom entrypoint script that figures out the container instance the container is running on and sets an environment variable. We can then configure the plugin to reference this environment variable dynamically. This PR simply marks the field as referenceable similar to the Kong native DataDog plugin.

Not sure if we want any tests / documentation updates here.

cgilmour commented 11 months ago

Thanks for submitting this. Yes, it would need some words in the documentation about it, as well as updating the versions that are considered compatible. This feature seems to be first available as beta in v2.8.0.

Would it be useful for other fields to be referenceable, or is it mainly this one?

As far as tests go, it'd be nice to have but not critical, because it'd be essentially a test covering kong and its general plugin functionality, rather than this plugin itself. The consequence being that if it failed the test, it wouldn't be a bug that I can fix within this plugin. Having said that, are you able to test this to confirm it works as expected in your environment, or would that require this being merged and/or published in a release?

zarakay commented 11 months ago

Hey @cgilmour, Thanks for having a look.

cgilmour commented 11 months ago

This might work as an alternative, so it works across more versions.

-- make a field referenceable if kong version >= 2.8.0
local function allow_referenceable(field)
    if kong.version_num >= 2008000 then
        field.referenceable = true
    end
    return field
end
...
                { environment = { type = "string", default = "none" } },
                { agent_endpoint = allow_referenceable(typedefs.url({ default = "http://localhost:8126/v0.4/traces" })) },
                { static_tags = { type = "array", elements = static_tag,
...
zarakay commented 11 months ago

@cgilmour

I made the fixes as requested. Thanks for the code snippet, super helpful. I added some light documentation of the new capability, and I double checked that this change worked with my test harness like I tested this previously. Let me know if you want anything else changed.

cgilmour commented 11 months ago

Thanks @zarakay, this will be in the next release (probably next week)

zarakay commented 11 months ago

Amazing! Thanks for making this experience so easy <3