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

Add container name tag in auto discovery #3421

Merged
merged 2 commits into from Jul 10, 2017
Merged

Add container name tag in auto discovery #3421

merged 2 commits into from Jul 10, 2017

Conversation

hkaj
Copy link
Member

@hkaj hkaj commented Jul 10, 2017

Note: Please remember to review the Datadog Contribution Guidelines
if you have not yet done so.

What does this PR do?

Rework #3365 to move back to a different template variable name.

Motivation

This is something we actually don't want included by default in %%tags%%. It should be used with care as it could flood the tag auto complete menu with a useless tag if used carelessly (typically: in a highly volatile environment where containers churn often and have different names each time).

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Additional Notes

Sorry for the back and forth @subuk, your first version was actually the good one!

@@ -337,8 +338,13 @@ def get_tags(self, state, c_id):

return tags

def _get_container_name(self, state, c_id, tpl_var):
Copy link
Member

Choose a reason for hiding this comment

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

tpl_var isn't used at all, is it normal ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah it's passed to all tag extractors by https://github.com/DataDog/dd-agent/blob/5.14.1/utils/service_discovery/sd_docker_backend.py#L489 but only used by some.

@hkaj hkaj merged commit a5dfb6d into master Jul 10, 2017
@hkaj hkaj deleted the subuk-master branch July 10, 2017 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants