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

Different identifier for increment and timing #25

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

xarlio
Copy link

@xarlio xarlio commented Jan 26, 2018

We have some problems to visualize the metrics as increment and timing functions are called with the same identifier. Adding '.counter' and '.timer' suffixes will differentiate them

@xarlio xarlio changed the title Different identifier for increment and timing [BREAKING CHANGE] Different identifier for increment and timing Jan 26, 2018
@coveralls
Copy link

coveralls commented Jan 26, 2018

Coverage Status

Coverage increased (+0.3%) to 96.429% when pulling cadf647 on xarlio:patch-1 into 031723a on mac-:master.

@mac-
Copy link
Owner

mac- commented Jan 26, 2018

Thanks for the contribution! A couple of questions:

  1. What tool are you using to visualize the metrics? From my experience, Graphite and Grafana are able to differentiate these metrics without changing the metric name.
  2. I don't have a problem with allowing a way to differentiate these by modifying the metric names, however, instead of hard-coding the .timer and .counter suffixes, can you make it configurable via options so that we don't force everyone into this convention?

@xarlio
Copy link
Author

xarlio commented Jan 30, 2018

We are using Datadog, and AFAIK there is no way to differentiate the two metrics. I will make it optional. Any suggestion for the options names and its defaults?

@mac-
Copy link
Owner

mac- commented Jan 31, 2018

you could do something as simple as a boolean like includeMetricTypeInName as an option, and if it's true, add it like you are doing now.

@xarlio xarlio changed the title [BREAKING CHANGE] Different identifier for increment and timing Different identifier for increment and timing Feb 2, 2018
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.

None yet

3 participants