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

Bug: log sampling is not working in v2 #2668

Closed
theburningmonk opened this issue Jun 18, 2024 · 5 comments · Fixed by #2673
Closed

Bug: log sampling is not working in v2 #2668

theburningmonk opened this issue Jun 18, 2024 · 5 comments · Fixed by #2673
Assignees
Labels
bug Something isn't working completed This item is complete and has been merged/shipped logger This item relates to the Logger Utility

Comments

@theburningmonk
Copy link

Expected Behavior

When POWERTOOLS_LOG_SAMPLE_RATE is set to 0.1 and every invocation includes a call to logger.refreshSampleRateCalculation() to recalculate the sampling rate, the debug logs should be recorded for only 10% of invocations.

Current Behavior

The sampling decision is made at cold start, and then all invocations afterwards will either log all debug logs or none at all. The refreshSampleRateCalculation() method is not giving the correct behaviour.

Code snippet

This line is included at the start of a handler function:

logger.refreshSampleRateCalculation()

The POWERTOOLS_LOG_LEVEL env var is set to ERROR.
The POWERTOOLS_LOGGER_SAMPLE_RATE env var is set to 0.1

Steps to Reproduce

Create a function with:

  • POWERTOOLS_LOGGER_SAMPLE_RATE env var set to 0.1
  • POWERTOOLS_LOG_LEVEL env var set to ERROR
  • include a call to logger.refreshSampleRateCalculation() at the start of the handler
  • include a few debug logs
    Invoke the function multiple times to see sampling decision is taken once, at cold start, but then never changed on subsequent invocations on the same worker

Possible Solution

No response

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

20.x

Packaging format used

npm

Execution logs

No response

@theburningmonk theburningmonk added bug Something isn't working triage This item has not been triaged by a maintainer, please wait labels Jun 18, 2024
Copy link

boring-cyborg bot commented Jun 18, 2024

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #typescript channel on our Powertools for AWS Lambda Discord: Invite link

@dreamorosi dreamorosi self-assigned this Jun 18, 2024
@dreamorosi dreamorosi added logger This item relates to the Logger Utility discussing The issue needs to be discussed, elaborated, or refined and removed triage This item has not been triaged by a maintainer, please wait labels Jun 18, 2024
@dreamorosi dreamorosi moved this from Triage to Working on it in Powertools for AWS Lambda (TypeScript) Jun 18, 2024
@dreamorosi
Copy link
Contributor

Hi @theburningmonk, thank you for opening the issue.

As I mentioned in the thread we exchanged I think the report is valid and we have a bug.

I am currently investigating and I will share more info once I have a definitive answer.

@dreamorosi dreamorosi linked a pull request Jun 19, 2024 that will close this issue
@dreamorosi dreamorosi moved this from Working on it to Pending review in Powertools for AWS Lambda (TypeScript) Jun 19, 2024
@dreamorosi dreamorosi added confirmed The scope is clear, ready for implementation and removed discussing The issue needs to be discussed, elaborated, or refined labels Jun 19, 2024
@dreamorosi
Copy link
Contributor

Hi Yan, just wanted to let you know that I identified the bug and that I have already opened a PR with a proposed fix.

The root cause of the bug was the one that I hinted at yesterday during our exchange on Twitter. Basically we were not restoring the log level to the original one after having sampled a specific invocation.

Once the PR is merged it will be included in the next release.

In the meanwhile, if you want to continue upgrading before the release, a workaround like this would work:

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

const logger = new Logger({ sampleRateValue: 0.1, logLevel: 'ERROR' });

export const handler = async () => {
+ logger.setLogLevel('ERROR'); // TODO: remove after next release
  logger.refreshSampleRateCalculation();
  
  logger.debug('This is a debug message - should not appear every time');

  return {
    statusCode: 200,
    body: JSON.stringify("Hello, World!"),
  };
};

@github-project-automation github-project-automation bot moved this from Pending review to Coming soon in Powertools for AWS Lambda (TypeScript) Jun 19, 2024
Copy link
Contributor

⚠️ 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.

@github-actions github-actions bot added pending-release This item has been merged and will be released soon and removed confirmed The scope is clear, ready for implementation labels Jun 19, 2024
Copy link
Contributor

This is now released under v2.3.0 version!

@github-actions github-actions bot added completed This item is complete and has been merged/shipped and removed pending-release This item has been merged and will be released soon labels Jun 27, 2024
@dreamorosi dreamorosi moved this from Coming soon to Shipped in Powertools for AWS Lambda (TypeScript) Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working completed This item is complete and has been merged/shipped logger This item relates to the Logger Utility
Projects
Development

Successfully merging a pull request may close this issue.

2 participants