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

Use whitelisted docker env variables as metric tags #2236

Closed
wants to merge 1 commit into from

Conversation

bergerx
Copy link

@bergerx bergerx commented Jan 6, 2017

We want to group container metrics of marathon applications in our DC/OS environment.
Mesos/Marathon exposes some marathon specific metadata as environment variables (mainly MESOS_TASK_ID, MARATHON_APP_ID, MARATHON_APP_VERSION) and this allows us to create metric dashboards for specific marathon apps.

I actually copied the idea from cAdvisor, but having the same feature in telegraf could save us from maintaining a new component (cAdvisor) on all nodes. Also current cAdvisor feature not yet working with influxdb storage: google/cadvisor#1427

I'm new in go, so please comment if something seems wrong. (I also signed the CLA)

I'm not sure about how to implement tests for this particular feature. I thought it would be similar to docker label as metrics tags, but that as well doesn't seem like have tests. Adding test doesn't seem trivial.

@sparrc sparrc added this to the 1.3.0 milestone Jan 10, 2017
@bergerx
Copy link
Author

bergerx commented Jan 30, 2017

Hey @sparrc is this planned to be merged in before 1.3.0 is released? Or are there some other blocking issues for this to be merged.

We are actually thinking about depending on this feature. Maintaining a fork until this can make it's way to upstream could be an option, but we prefer not to maintain a fork for this in long term.

@sparrc
Copy link
Contributor

sparrc commented Jan 30, 2017

yep, it's planned for 1.3, just haven't had the time to give it a full review yet

@sparrc
Copy link
Contributor

sparrc commented Jan 30, 2017

err, sorry but this actually looks like a dupe of #2160, so I'm going to close it for the time being until we can fix #2071.

Apologies for that, but I'd prefer to fix our usage of a deprecated docker library before adding more features.

@sparrc sparrc closed this Jan 30, 2017
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.

2 participants