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

[go_expvar] Fix path tag appended to all metrics matching path regex #472

Merged
merged 5 commits into from
Jun 13, 2017

Conversation

zippolyte
Copy link
Contributor

@zippolyte zippolyte commented Jun 12, 2017

What does this PR do?

Fix bug where each path matching the path regex of a metric would be added as a tag when an alias is set.
This would result in something like this :

 ('scheduler.queues.size',
  1497030010,
  1,
  {'hostname': 'mac-agent5',
   'tags': ['check_type:python',
            'expvar_url:http://localhost:5000/debug/vars',
            'path:scheduler.Queues.0.Size',
            'path:scheduler.Queues.1.Size'],
   'type': 'gauge'}),
 ('scheduler.queues.size',
  1497030010,
  2,
  {'hostname': 'mac-agent5',
   'tags': ['check_type:python',
            'expvar_url:http://localhost:5000/debug/vars',
            'path:scheduler.Queues.0.Size',
            'path:scheduler.Queues.1.Size'],
   'type': 'gauge'})

instead of this :

 ('scheduler.queues.size',
  1497030010,
  1,
  {'hostname': 'mac-agent5',
   'tags': ['check_type:python',
            'expvar_url:http://localhost:5000/debug/vars',
            'path:scheduler.Queues.0.Size]',
   'type': 'gauge'}),
 ('scheduler.queues.size',
  1497030010,
  2,
  {'hostname': 'mac-agent5',
   'tags': ['check_type:python',
            'expvar_url:http://localhost:5000/debug/vars',
            'path:scheduler.Queues.1.Size'],
   'type': 'gauge'})

for this yaml file :

- path: scheduler/Queues/\d+/Size
   type: gauge
   alias: scheduler.queues.size

Motivation

Discovered the bug while testing the scheduler in the new agent.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Versioning

  • Bumped the version check in manifest.json
  • Updated CHANGELOG.md

Additional Notes

Anything else we should know when reviewing?

@olivielpeau olivielpeau self-requested a review June 12, 2017 22:15
olivielpeau
olivielpeau previously approved these changes Jun 13, 2017
Copy link
Member

@olivielpeau olivielpeau left a comment

Choose a reason for hiding this comment

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

Code LGTM! 👌

One comment on the changelog. Could you update manifest.json with the version bump too?

@@ -3,6 +3,7 @@
1.0.2 / Unreleased
==================

* [BUGFIX] Fix path tag appended to all metrics matching path regex
Copy link
Member

Choose a reason for hiding this comment

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

could you put this line in a new 1.0.3 section? 1.0.2 is what was shipped with 5.14.0

@olivielpeau olivielpeau added this to the 5.15 milestone Jun 13, 2017
@zippolyte zippolyte merged commit bae30da into master Jun 13, 2017
@zippolyte zippolyte deleted the hippo/fix_tag_path_expvar branch June 13, 2017 17:46
@masci masci modified the milestones: 5.15, 5.14.1 Jun 14, 2017
gml3ff pushed a commit that referenced this pull request May 14, 2020
Flipping the attribute 'datadog''agent6' to true installs the agent6
instead of the agent5. Switching it back to false should reverse
the operation as long as the agent 5 version is pinned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants