-
Notifications
You must be signed in to change notification settings - Fork 39
Calling MetricsLogger.flush() causes RuntimeWarnings #52
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
Comments
This is related to #21 where we will likely make flush() synchronous. Today, it is an async call and should be awaited. We can update the documentation in the meantime to make this more clear. |
what's the status on this issue? this is super noisy in lambda CWL output |
@ryandeivert status is unchanged. The method is still asynchronous, and you need to be awaiting it. In my code, I'm doing this:
|
yes I understand - but for larger lambda codebases, where the bulk of the logic does not occur within the scope of a single function, this isn't a super ideal way to use this library (as a decorator). effectively, I'd have to wrap any of my functions that log metrics with this, instead of just creating a logger object to be used directly. unless I'm misunderstanding the API, in which I'd love to hear about alternatives. edit: can you also clarify why certain certain properties are injected into the output, with no ability to override these (see here) I'm currently using the below to work around this: from aws_embedded_metrics import MetricsLogger as _MetricsLogger
from aws_embedded_metrics.environment.lambda_environment import LambdaEnvironment
class MetricsLogger(_MetricsLogger):
def __init__(self):
super().__init__(None, None)
self.environment = LambdaEnvironment()
def flush(self) -> None:
"""Override the default async MetricsLogger.flush method, flushing to stdout immediately"""
sink = self.environment.get_sink()
sink.accept(self.context)
self.context = self.context.create_copy_with_context()
def with_dimensions(self, *dimensions):
return self.set_dimensions(*dimensions)
def main():
new_logger = MetricsLogger()
new_logger.put_metric('metric_name', 10).with_dimensions({'dim01': 'value01', 'dim02': 'value02'})
new_logger.flush() |
@SamStephens Why would anyone need to do that? |
@ryandeivert thanks for your workaround, it's saved my bacon with Flask, Gunicorn and Gevent where for reasons I don't fully understand I cannot use Flask's async support. However, also I don't understand why you need your workaround with Lambda. If you're actually calling other functions, surely your main functions really looks like
If so, I don't actually see how this is different to
|
@ryandeivert it's worth noting this isn't just noise, the warning means that it's possible for the Lambda function to be shutdown before the flush actually completes, because you're not awaiting it to complete. This means there's a chance of metrics being lost. |
Yeah this is pretty annoying. I basically want to emit a metric in our lambda that can possibly process multiple failing hostnames: from aws_embedded_metrics.logger.metrics_logger import MetricsLogger
async def emit_hostname_failure(logger: MetricsLogger, hostname: str) -> None:
logger.set_namespace("MyNamespace")
logger.put_metric("HostnameFailure", 1, "Count")
# Hostname is a high cardinality value. Do NOT use put_dimensions, but instead use set_property
# where results will be queried through CloudWatch Insights.
logger.set_property("Hostname", hostname)
# A property can only be tied to a single metric, so call flush
# for each device data point.
await logger.flush() # ERROR! Result of async function call is not used; use "await" or assign result to variable Unfortunately I now need to either figure out how to make the lambda work async, or override my own class like @ryandeivert did. Either way this is a headache. Looks like wrapping the call in import asyncio
from aws_embedded_metrics.logger.metrics_logger_factory import create_metrics_logger
def handler(event, context):
logger = create_metrics_logger()
...
asyncio.run(emit_hostname_failure(logger, "hostname") |
We have seen the following issue:
|
I think #113 being released may mean this issue can be closed? |
Calling
MetricsLogger().flush()
as documented in the README causes RuntimeWarnings:Here is a minimal example:
Output when calling:
The text was updated successfully, but these errors were encountered: