Skip to content

sanitize the name of the metric sent to timing() and increment()...#7

Merged
adborden merged 1 commit intogoodeggs:masterfrom
cainus:master
Nov 18, 2014
Merged

sanitize the name of the metric sent to timing() and increment()...#7
adborden merged 1 commit intogoodeggs:masterfrom
cainus:master

Conversation

@cainus
Copy link
Contributor

@cainus cainus commented May 11, 2014

... with the rules used in librato's official statsd backend:
https://github.com/librato/statsd-librato-backend/blob/dffece631fcdc4c94b2bff1f7526486aa5bfbab9/lib/librato.js#L146
. This should help avoid a number of scenarios where the client sends a
bad request (due to a bad metric name) and the service is forced to 400.

Also: hard-code the coffeescript version that is expected, because only
a very specific version will not cause diffs across the codebase when
the js files are built.

… the

rules used in librato's official statsd backend:
https://github.com/librato/statsd-librato-backend/blob/dffece631fcdc4c94b2bff1f7526486aa5bfbab9/lib/librato.js#L146
.  This should help avoid a number of scenarios where the client sends a
bad request (due to a bad metric name) and the service is forced to 400.

Also: hard-code the coffeescript version that is expected, because only
a very specific version will not cause diffs across the codebase when
the js files are built.
@cainus
Copy link
Contributor Author

cainus commented May 11, 2014

I believe this should fix #6 .

Also, I created a Makefile locally that might be nicer than using npm (or maybe not if you like npm that much):

build:
    ./node_modules/.bin/coffee -c -o lib src

test: build
    ./node_modules/.bin/mocha

( just make test to run tests, or make build to just compile ).

@cainus cainus closed this May 11, 2014
@bobzoller
Copy link
Contributor

this looks good... did you close it on accident or do you have something different coming?

@bobzoller bobzoller reopened this May 12, 2014
@cainus
Copy link
Contributor Author

cainus commented May 13, 2014

Yeah I must've closed it by accident. Duh :) I've been using this patch in production, and it works pretty well, so I think it's good to merge unless you have objections.

adborden pushed a commit that referenced this pull request Nov 18, 2014
sanitize the name of the metric sent to timing() and increment()...
@adborden adborden merged commit cf49bae into goodeggs:master Nov 18, 2014
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.

3 participants