add env vars support for dogstatsd host, port and entity id#363
add env vars support for dogstatsd host, port and entity id#363ahmed-mez merged 5 commits intoDataDog:masterfrom
Conversation
datadog/__init__.py
Outdated
| statsd.host = None | ||
| statsd.port = None | ||
| else: | ||
| # Check host and port env vars, override arguments if env vars exist |
There was a problem hiding this comment.
Please give arguments precedence as it says in the PR description.
| log.warning("Port number provided in DD_DOGSTATSD_PORT env var is not an integer: \ | ||
| %s, using %s as port number", DD_DOGSTATSD_PORT, port) | ||
| pass | ||
|
|
There was a problem hiding this comment.
We may have to set the default arguments to None to detect actual args. @yannmh wdyt?
There was a problem hiding this comment.
I don't have a strong opinion on this. If we decide to keep the default args, I'd name the default argument, so it becomes
# Check host and port env vars
DD_AGENT_HOST = os.environ.get('DD_AGENT_HOST', '')
if DD_AGENT_HOST and host == DEFAULT_HOST:
host = DD_AGENT_HOSTOther notes:
- is it a "conventional" to name Python variables that store env variable with UPPERCASE?
os.environ.get('DD_AGENT_HOST', '')->os.environ.get('DD_AGENT_HOST')
There was a problem hiding this comment.
thanks for the review :)
I think I'm going to keep default args and name them as you suggested
regarding the other notes:
- it is not conventional to name Python variables that store env var with UPPERCASE, so I'm going to change it lowercase 👍
- 👍
datadog/__init__.py
Outdated
| statsd_port = int(DD_DOGSTATSD_PORT) | ||
| except ValueError: | ||
| pass | ||
|
|
There was a problem hiding this comment.
I don't think you need these. The "already instantiated" statsd client, would have had those values set in the constructor below.
datadog/dogstatsd/base.py
Outdated
| if constant_tags is None: | ||
| constant_tags = [] | ||
| self.constant_tags = constant_tags + env_tags | ||
| ENTITY_ID_TAG_NAME = "dd.internal.entity_id" |
There was a problem hiding this comment.
this constant should be moved outside of the constructor
datadog/dogstatsd/base.py
Outdated
| except ValueError: | ||
| log.warning("Port number provided in DD_DOGSTATSD_PORT env var is not an integer: \ | ||
| %s, using %s as port number", DD_DOGSTATSD_PORT, port) | ||
| pass |
tests/unit/dogstatsd/test_statsd.py
Outdated
| os.environ['DD_DOGSTATSD_PORT'] = '4321' | ||
| initialize() | ||
| t.assert_equal(statsd.host, "myenvvarhost") | ||
| t.assert_equal(statsd.port, 4321) |
| statsd.socket = FakeSocket() | ||
| statsd.gauge('gt', 123.4) | ||
| t.assert_equal('gt:123.4|g|#country:canada,red,country:china,age:45,blue,dd.internal.entity_id:04652bb7-19b7-11e9-9cc6-42010a9c016d', statsd.socket.recv()) | ||
|
|
@yannmh thanks, can you please see the changes I've made? let me know if you have any other comment :) |
|
Looks good to me. |
3 participants
Add environment variables support for Client configuration
Support of 3 environment variables for Client configuration:
"DD_AGENT_HOST"used to provide the Agent hostname."DD_DOGSTATSD_PORT"used to provide the DogStatsD port."DD_ENTITY_ID"used to provide an identifier to the Client.If Client constructor's arguments are not set, prioritize
"DD_AGENT_HOST"and"DD_DOGSTATSD_PORT"over default values.Add
"DD_ENTITY_ID"to tags with the tag namedd.internal.entity_id.