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

[core] Allow DD_AGENT_HOST and DD_TRACE_AGENT_PORT env variables #708

Merged
merged 4 commits into from
Nov 14, 2018

Conversation

brettlangdon
Copy link
Member

To help support desired containerization onboarding improvements we need to support DD_AGENT_HOST and DD_TRACE_AGENT_PORT environment variables for configuration.

These changes are the minimal viable changes needed to support these variables. It isn't the most clean, using something like get_env() would be better, but we decided to get this out the door since @Kyle-Verhoog has a task to work on standardizing our DD_* variables.

@brettlangdon brettlangdon added this to the 0.17.0 milestone Nov 12, 2018
DEFAULT_HOSTNAME = environ.get('DATADOG_TRACE_AGENT_HOSTNAME', 'localhost')
DEFAULT_PORT = 8126
DEFAULT_HOSTNAME = environ.get('DD_AGENT_HOST', environ.get('DATADOG_TRACE_AGENT_HOSTNAME', 'localhost'))
DEFAULT_PORT = int(environ.get('DD_TRACE_AGENT_PORT', 8126))

Choose a reason for hiding this comment

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

Do we have a test for this code path? Such as initializing a new tracer after setting the env vars without using ddtrace-run?

Kyle-Verhoog
Kyle-Verhoog previously approved these changes Nov 13, 2018
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

To prevent regressions we can add the test @palazzem suggested. Looks good otherwise.

Copy link

@palazzem palazzem left a comment

Choose a reason for hiding this comment

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

Let's add the missing test without ddtrace-run and then we're ready to merge.

@brettlangdon
Copy link
Member Author

@palazzem I think if I understood what you were looking for correctly, I updated the tests to also run the same hostname check runner, but without using ddtrace-run. I also updated those tests to use a non-default port... just to be sure.

@palazzem palazzem merged commit 73e67f2 into 0.17-dev Nov 14, 2018
@palazzem palazzem deleted the brettlangdon/dd.agent.host branch November 14, 2018 15:35
Kyle-Verhoog pushed a commit that referenced this pull request Nov 23, 2018
* [core] Allow DD_AGENT_HOST and DD_TRACE_AGENT_PORT env variables
* [core] test different tracer port and running without ddtrace-run
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants