[docker_daemon] Adding Marathon App Id tag capture for Docker Daemon #2870
Conversation
These changes allow including marathon app_id in the reported tags for containers
seeder
changed the title
There was a problem hiding this comment.
Thanks a lot for contributing @seeder !
The logic and the code are both great, I left a few minor comments to make the code fit more nicely with the check, but nothing major to change.
I have one more request though (sorry about that 😅 ): could you add this parameter to docker_daemon.yaml.example with a short comment to explain what it does? A short extract from your PR description would do the trick.
Thanks again!
| @@ -174,6 +174,7 @@ def init(self): | |||
| self._disable_net_metrics = False | |||
|
|
|||
| # Set tagging options | |||
| self.include_app_id = instance.get("include_marathon_app_id", False) | |||
There was a problem hiding this comment.
Nitpicking but can you rename this attribute to include_marathon_app_id? App is quite generic, it could be an issue when we add more tags in the future.
| @@ -357,6 +358,19 @@ def _get_tags(self, entity=None, tag_type=None): | |||
| if entity is not None: | |||
| pod_name = None | |||
|
|
|||
| if self.include_app_id and (tag_type is CONTAINER or tag_type is PERFORMANCE): | |||
| self.log.debug("We need to capture Marathon Id") | |||
There was a problem hiding this comment.
let's remove this one, debug logs are already spammy enough 😄
| if cont_id is not None: | ||
| cont_inspected = self.docker_client.inspect_container(cont_id) | ||
| for env_var in cont_inspected['Config']['Env']: | ||
| self.log.debug("We have env_var : %s" % env_var) |
| self.log.debug("We have env_var : %s" % env_var) | ||
| if 'MARATHON_APP_ID=' in env_var: | ||
| app_id = env_var.replace('MARATHON_APP_ID=','') | ||
| self.log.debug("We have found the id as %s" % app_id) |
There was a problem hiding this comment.
This one is cool.
If the tag is missing in Datadog and the logs don't show this line we can display env variables and see what's wrong.
| if 'MARATHON_APP_ID=' in env_var: | ||
| app_id = env_var.replace('MARATHON_APP_ID=','') | ||
| self.log.debug("We have found the id as %s" % app_id) | ||
| tags.append("app_id:%s" % app_id) |
There was a problem hiding this comment.
Same as for the attribute, app_id is quite generic, it will be difficult to make a sense of it in the check as we support many orchestrators besides Marathon. Could you rename it to marathon_app_id please?
What does this PR do?
On Mesos/Marathon managed docker daemons, allows tagging container data with app_id tag used by your Marathon integration, allowing to correlated the data in Dashboards
Motivation
As DevOps integrator, using datadog we would like to have Marathon and docker data correlation.
Sharing the integration with you, as we believe it maybe of use to others.
Testing Guidelines
An overview on testing
is available in our contribution guidelines.
Additional Notes
Feature gets enabled when instance config in docker_daemon.yaml has the
include_marathon_app_idset to trueIt may not be perfectly within Datadog coding guidelines and feel free to change as required.
Provided as is.