-
Notifications
You must be signed in to change notification settings - Fork 44
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
feat: Support to exclude default metrics and other options #68
base: master
Are you sure you want to change the base?
Conversation
@kobik This is another PR for changes I've made for my company's use case, I'm fixing tests and making sure the build passes, but if you could start taking a look and let me know your feedback please. Thanks |
@yorch can you please rebase from master so we'll have the circleci running on your branch? |
d758f74
to
b068881
Compare
@kobik this is ready for review please |
Will do |
Hi @yorch , sorry for the delay it's been busy time + holidays here. I started reviewing you PR and I have a question regarding |
@kobik The use case for the prefix is that depending on your infrastructure, you may want to avoid having multiple different NodeJS services exposing the same metrics. Like in our case, we have a big variety of NodeJS services, so we only want similar services to share metric names. |
ok, so i have a little different idea in mind. how about adding and on the next breaking version we can change the default behavior to make it more consistent, making this way the developer will be aware of the metrics he's adding. i.e {
excludeDefaultMetricLabels: true,
httpRequestMetrics: ["sum", "count", "histogram"],
httpResponseMetrics: ["sum", "count", "histogram"]
} let me know what you think. |
Are there plans to merge this PR? |
never got a response from @yorch regarding my latest comment. i don't want to introduce a breaking change if we can avoid that. |
httpMetricsPrefix
option to optionally add a prefix to HTTP metrics.excludeDefaultMetricLabels
option to exclude all or certain metrics that are added by default.useCountersForRequestSizeMetric
option to expose two counters for Request Size (_sum
and_count
) instead of Histogram.useCountersForResponseSizeMetric
option to expose two counters for Response Size (_sum
and_count
) instead of Histogram.