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

Override DATADOG_PREFIX only if it is not defined #113

Conversation

@rmax
Copy link
Contributor

rmax commented Nov 13, 2019

What did you implement:

Changed the behavior that overrides DATADOG_PREFIX if it is undefined or
empty.

How did you implement it:

Override only if it is undefined.

How can we verify it:

The tests illustrate the new behavior.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / projects / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: YES

@rmax

This comment has been minimized.

Copy link
Contributor Author

rmax commented Nov 13, 2019

This change is backward incompatible if:

  • The user sets DATADOG_PREFIX=''
  • Let the library store the metrics as function_name.custom.metric
  • Uses that metrics in for graphs

After this change, metric name would change from function_name.custom.metric to custom.metric causing a break in the graphs.

@@ -9,7 +9,7 @@ const FUNCTION_NAME = process.env.AWS_LAMBDA_FUNCTION_NAME
const FUNCTION_VERSION = process.env.AWS_LAMBDA_FUNCTION_VERSION
const ENV = process.env.ENVIRONMENT || process.env.STAGE

if (!process.env.DATADOG_PREFIX) {
if (typeof process.env.DATADOG_PREFIX === 'undefined') {

This comment has been minimized.

Copy link
@theburningmonk

theburningmonk Nov 14, 2019

Collaborator

do we need typeof here? would process.env.DATADOG_PREFIX === undefined suffice?

This comment has been minimized.

Copy link
@rmax

rmax Nov 14, 2019

Author Contributor

Personally would prefer typeof to avoid comparison with different types but in this case it's enough ===.

@@ -16,7 +16,7 @@ const ENV = process.env.ENVIRONMENT || process.env.STAGE
*/
const FILTERING_MODE = Object.freeze({ 'BLACKLIST': 'BLACKLIST', 'WHITELIST': 'WHITELIST' })

if (!process.env.DATADOG_PREFIX) {
if (typeof process.env.DATADOG_PREFIX === 'undefined') {

This comment has been minimized.

Copy link
@theburningmonk

theburningmonk Nov 14, 2019

Collaborator

do we need typeof here? would process.env.DATADOG_PREFIX === undefined suffice?

@rmax rmax force-pushed the rmax-contrib:feature/override-datadog-prefix-only-if-undefined branch from d18c742 to f7b2bae Nov 14, 2019
@theburningmonk theburningmonk self-requested a review Nov 14, 2019
@theburningmonk theburningmonk merged commit a6de4dd into getndazn:master Nov 14, 2019
4 checks passed
4 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: test-node10 Your tests passed on CircleCI!
Details
ci/circleci: test-node12 Your tests passed on CircleCI!
Details
ci/circleci: test-node8 Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.