[yarn] Consolidate Tags #2473
[yarn] Consolidate Tags #2473
Conversation
|
Thanks a lot for your contribution! This looks pretty decent, it passes the checks. We will review and test this soon to make sure it all works! 😄 |
|
|
||
| tags = ['cluster_id:' + str(cluster_id), 'app_id:' + str(app_id)] | ||
| tags = ['app_name:%s' % str(app_name)] | ||
| tags.extend(addl_tags) | ||
|
|
||
| self._set_yarn_metrics_from_json(tags, app_json, YARN_APP_METRICS) |
There was a problem hiding this comment.
The check now tags these metrics by app_name instead of app_id, so there can be duplicate sets of tags for the same metric (when multiple apps have the same name). So we should use increment instead of gauge to send these metrics
|
@zachradtka Made 2 comments, once they're addressed we'll test the check on our clusters, and we may have more feedback then :) Thanks! |
| ''' | ||
| Return the cluster ID, otherwise raise an exception | ||
| ''' | ||
| info_json = self._rest_request_to_json(rm_address, INFO_URI) |
There was a problem hiding this comment.
the INFO_URI var should be removed from the check too
|
@olivielpeau I have updated the check and tests to remove the |
|
Looks good @zachradtka! Could you squash your commits please? I'll merge once that's done, thanks! |
zachradtka
force-pushed
the
minerkasch/yarn-consolidate-tags
branch
from
2227ae5 to
414272a
Compare
Added application tags in the configuration file, allowing set extra tags for app metrics like queue, user, etc. Also, added the tags functionality merged in #2473 on tests
This is mainly a bug fix to reduce the number of tags created by the YARN agent check.
The major fixes are:
cluster_nameinstead ofcluster_idapp_nameinstead ofapp_id