-
Notifications
You must be signed in to change notification settings - Fork 25
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(metrics): add metrics logging and configuration support #1559
base: master
Are you sure you want to change the base?
Conversation
72084b7
to
a8911c0
Compare
a1f424b
to
f656e3a
Compare
(env: NodeJS.ProcessEnv) => env.METRICS_ENABLED === "yes", | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the configuration module that is imported on line 43?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have checked official documentation and some other articles. ConditionalModule.registerWhen
method do not support access to the configService.
The cleanest way is to use ConditionalModule.registerWhen
and fetch env directly, however, there's hack to achieve same result with DyncmiModule with increased complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
32c4913
to
92098f3
Compare
92098f3
to
a99a96d
Compare
Description
Motivation
Fixes
Changes:
Tests included
Documentation
official documentation info