Skip to content

feat: sampling feature for logger#7

Merged
heitorlessa merged 4 commits intoaws-powertools:developfrom
danilohgds:sampling_feature
Feb 20, 2020
Merged

feat: sampling feature for logger#7
heitorlessa merged 4 commits intoaws-powertools:developfrom
danilohgds:sampling_feature

Conversation

@danilohgds
Copy link
Copy Markdown
Contributor

Issue #, if available:
This relates to issue Enhancement Suggestion - Log Sampling

Description of changes:
This pull request introduces a new variable/ constructor parameter to enable debug log sampling of a given percentage, similar to what DAZN powertools has. POWERTOOLS_LOGGER_SAMPLE_RATE accepts in 0-1 range, to log in debug mode at the given rate.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Copy Markdown
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking quite good!! Some minor adjustments and one ask to improve functional testing.

>>>
>>> def handler(event, context):
logger.info("Hello")
:param service:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

62-64 doesn't seem right - Can you remove it?

service name
LOG_LEVEL: str
logging level (e.g. INFO, DEBUG)
POWERTOOLS_LOGGER_SAMPLE_RATE: str
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant float in type annotation here - POWERTOOLS_LOGGER_SAMPLE_RATE: float

LOG_LEVEL: str
logging level (e.g. INFO, DEBUG)
POWERTOOLS_LOGGER_SAMPLE_RATE: str
samping rate ranging from 0 to 1, float precision
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop ", float precision" and use "1 being 100% sampling" instead. This along with float type annotation should clear out any ambiguity.

if sampling_rate and random.random() <= float(sampling_rate):
log_level = logging.DEBUG
except ValueError:
logger.debug(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raise the exception instead so they know they need to take corrective actions.

raise ValueError(f"Expected a float value ranging 0 to 1, but received {sampling_rate} instead. Please review POWERTOOLS_LOGGER_SAMPLE_RATE environment variable.")

-------
decorate : Callable
Decorated lambda handler
:param log_event:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop these two - I believe these are coming from your IDE automatically ;)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, pycharm being cheeky :)

log_level = os.getenv("LOG_LEVEL") or level
logger = logging.getLogger(name=service)

# sampling a small percentage of requests with debug level, using a float value 0.1 = 10%~
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this in the right place? ;)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed, good catch.

…g LOG_LEVEL.

comments/docstrings from IDE removed.
except ValueError:
raise ValueError(
f"Expected a float value ranging 0 to 1, but received {sampling_rate} instead. Please review "
f"POWERTOOLS_LOGGER_SAMPLE_RATE environment variable."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can merge this line with the above instead of having two lines ;)

f"Expected a float value ranging 0 to 1, but received {sampling_rate} instead. Please review POWERTOOLS_LOGGER_SAMPLE_RATE environment variable."

minor reformat removal ( pycharm pep8 complains with 120 characters on line )
@heitorlessa heitorlessa merged commit ccd9f03 into aws-powertools:develop Feb 20, 2020
@heitorlessa heitorlessa added the feature New feature or functionality label Jun 3, 2020
@heitorlessa heitorlessa changed the title Sampling feature feat: sampling feature for logger Jun 3, 2020
heitorlessa referenced this pull request in heitorlessa/aws-lambda-powertools-python Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants