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.

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

proposal: allow configuration of Trace Agent URL with DD_TRACE_AGENT_URL #1023

Closed
PonzaMatteo opened this issue Sep 30, 2021 · 4 comments · Fixed by #1199
Closed

proposal: allow configuration of Trace Agent URL with DD_TRACE_AGENT_URL #1023

PonzaMatteo opened this issue Sep 30, 2021 · 4 comments · Fixed by #1199
Labels
ack proposal/accepted Accepted proposals proposal more in depth change that requires full team approval

Comments

@PonzaMatteo
Copy link

Problem Statement

Other SDKs (like the js and python one) allow to configure the trace agent URL using the env variable DD_TRACE_AGENT_URL while the Go SDK only allows using the combination of DD_AGENT_HOST + DD_TRACE_AGENT_PORT, this results in the following issues for our needs:

  • the hardcoded protocol is HTTP, but we would need to utilize HTTPS
  • it's not possible to omit the port in the agent URL since if left empty it defaults to 8126

Proposed Solution

Allow using DD_TRACE_AGENT_URL to override the combination of the other two variables.

@mcculls
Copy link

mcculls commented Oct 7, 2021

Note: another benefit of DD_TRACE_AGENT_URL is that you can use it to configure a UDS connection to the agent (ie. unix://<path> like in the other tracers)

@hush-hush
Copy link
Member

This would also solve the issue of customer not being able to configure both a DogStatsD client and the tracer on the same host when using UDS:

ref: DataDog/datadog-go#201

@knusbaum
Copy link
Contributor

I think DD_TRACE_AGENT_URL would be a more useful and consistent option than the current host + port combination for all the reasons mentioned here.

I think in addition to introducing DD_TRACE_AGENT_URL and having it override the other existing environment variables, we should deprecate the old ones and discourage their use.

@adawalli
Copy link

The fact that this is hard-coded goes against the defaults that are now used in the helm chart for agent deploys: https://github.com/DataDog/helm-charts/blob/main/charts/datadog/values.yaml#L355

8126 is not even exposed by default anymore - the Unix socket is used!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack proposal/accepted Accepted proposals proposal more in depth change that requires full team approval
Projects
None yet
5 participants