DataDog / kong-plugin-ddtrace

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

Respect DD_AGENT_HOST environment variable when setting the default agent endpoint. #15

Closed twelvelabs closed 6 months ago

twelvelabs commented 10 months ago

I am using the Datadog Admission Controller in a k8s cluster. The controller injects the DD_AGENT_HOST env variable into pods at startup.

I tried using the following in my declarative config:

agent_endpoint: 'http://{vault://env/dd-agent-host}:8126/v0.4/traces'

... but it appears that the secrets management logic does not support nesting vault: placeholders in another string - it only replaces them when the string begins and ends with the placeholder.

I'm currently using envsubst to swap out $DD_AGENT_HOST in the kong.yaml file in the Docker entrypoint, but it would great if the plugin could just use that value in the default if present.

dgoffredo commented 10 months ago

Hi Skip,

I looked into what you said:

... but it appears that the secrets management logic does not support nesting vault: placeholders in another string - it only replaces them when the string begins and ends with the placeholder.

That's probably this code, in Kong itself: https://github.com/Kong/kong/blob/e0d53a8993f215a02351a2dfe3884ca3ed884f76/kong/pdk/vault.lua#L63-L79.

I wonder why they made that choice. Perhaps it's meant to be "either a reference, or a string literal," as opposed to being an interpolated language. :shrug:

[...] it would great if the plugin could just use that value in the default if present.

What do you mean by "if present"? Do you mean parse references differently than how Kong does, or do you mean inspect the environment? What sort of behavior do you have in mind?

twelvelabs commented 10 months ago

No, I wouldn't expect the plugin to parse the config string differently. More the later - ideally the plugin would inspect the environment when setting the default. IIRC, that's the default behavior in some of the other ddtrace implementations I've used.

dgoffredo commented 10 months ago

Inspecting the environment from within an nginx worker processes can be a problem, because nginx does not forward most environment variables to its children. They must be explicitly named with env in nginx.conf.

I'll bring this up with my teammates.

An alternative would be to have an option:

agent_host: '{vault://env/dd-agent-host}'

if we don't want to open up the environ can of worms.

dmehala commented 6 months ago

Hello @twelvelabs,

Appreciate your report on this issue. I'd like to update you that we've successfully merged the solution, and it will be part of the forthcoming release.

Thank you for bringing this to our attention. If you have any additional questions or concerns, feel free to reach out.