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

Feature request: allow to pass custom logger for warning and debug logs #2126

Closed
1 of 2 tasks
dreamorosi opened this issue Feb 21, 2024 · 2 comments · Fixed by #3057
Closed
1 of 2 tasks

Feature request: allow to pass custom logger for warning and debug logs #2126

dreamorosi opened this issue Feb 21, 2024 · 2 comments · Fixed by #3057
Assignees
Labels
confirmed The scope is clear, ready for implementation enhancement PRs that introduce minor changes, usually to existing features feature-request This item refers to a feature request for an existing or new utility metrics This item relates to the Metrics Utility

Comments

@dreamorosi
Copy link
Contributor

dreamorosi commented Feb 21, 2024

Use case

The Metrics utility emits some warning logs to notify customers that certain expected conditions are not being met. This is the case for when a namespace is not specified or when the publishStoredMetrics() method is called on an empty buffer.

Currently customers have no way of suppressing these warnings and some customers have reported wanting to do so (#2036). I think this is a fair ask and whatever implementation we settle for in this issue will also be reused for other utilities that emit either warnings or debug logs (Idempotency, Tracer, and Parameters).

Solution/User Experience

We could define a new type/interface in the commons package and expose it:

interface UtilityLogger {
  trace?: (...content: any[]) => void;
  debug: (...content: any[]) => void;
  info: (...content: any[]) => void;
  warn: (...content: any[]) => void;
  error: (...content: any[]) => void;
}

From there, customers can use it as a reference to create their own logger and pass it to the Metrics utility.

In the example below I'm making the warn method a no-op to disable the warnings entirely, but customers can write their own custom implementation to decide whether the logs are emitted or not.

import { Metrics } from '@aws-lambda-powertools/metrics';
import type { UtilityLogger } from '@aws-lambda-powertools/commons/types';

const myLogger: UtilityLogger = {
  debug: console.debug,
  info: console.info,
  warn: {}, // no-op - but customers can add their own logic
  error: console.error,
};

const metrics = new Metrics({
  namespace: 'serverlessAirline',
  serviceName: 'orders',
  logger: myLogger,
});

Customers should also be able to pass an instance of Powertools Logger if they wish to do so:

import { Metrics } from '@aws-lambda-powertools/metrics';
import { Logger } from '@aws-lambda-powertools/logger';

const logger = new Logger({
  serviceName: 'orders',
  logLevel: 'ERROR',
});

const metrics = new Metrics({
  namespace: 'serverlessAirline',
  serviceName: 'orders',
  logger,
});

My main concern with this is avoiding confusion and conveying clearly that this is only a logger that will be used for debug and warning logs but not to emit the EMF metrics themselves.

The Metrics utility maintain its own Console object that logs the metrics using console.log (notice that the log() method is not part of the suggested interface). This is needed for the Metrics utility to work with the Advanced Logging Configuration feature.

Alternative solutions

If my memory serves me right the AWS Lambda Node.js managed runtime treats warning emitted via process.emitWarning(warning[, options]) as errors rendering this method unviable.

As part of this issue however we should still test this option just in case I'm wrong.

Other alternatives that I'm not inclined to consider would go along the lines of adding a doNotWarnOnEmptyMetrics option to suppress these warnings.

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

@dreamorosi dreamorosi added metrics This item relates to the Metrics Utility feature-request This item refers to a feature request for an existing or new utility enhancement PRs that introduce minor changes, usually to existing features discussing The issue needs to be discussed, elaborated, or refined labels Feb 21, 2024
@dreamorosi
Copy link
Contributor Author

@heitorlessa & @am29d would love your opinion on this and especially your point of view on the concern I share at the end of the "Solution/User Experience" section.

Also please let me know if anything is not clear, happy to clarify & expound on any detail. Thanks!

@dreamorosi dreamorosi added confirmed The scope is clear, ready for implementation and removed discussing The issue needs to be discussed, elaborated, or refined labels Sep 12, 2024
@dreamorosi dreamorosi self-assigned this Sep 12, 2024
@dreamorosi dreamorosi linked a pull request Sep 21, 2024 that will close this issue
Copy link
Contributor

github-actions bot commented Nov 7, 2024

⚠️ COMMENT VISIBILITY WARNING ⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed The scope is clear, ready for implementation enhancement PRs that introduce minor changes, usually to existing features feature-request This item refers to a feature request for an existing or new utility metrics This item relates to the Metrics Utility
Projects
Status: Coming soon
Development

Successfully merging a pull request may close this issue.

1 participant