Skip to content

WIP: Introduce experimental JSON logging #16

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions localstack/aws/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def __init__(self, service_manager: ServiceManager = None) -> None:
# the main request handler chain
self.request_handlers.extend(
[
handlers.log_request,
handlers.push_request_context,
handlers.add_internal_request_params,
handlers.handle_runtime_shutdown,
Expand All @@ -45,6 +46,7 @@ def __init__(self, service_manager: ServiceManager = None) -> None:
handlers.add_region_from_header,
handlers.add_account_id,
handlers.parse_service_request,
# TODO: add logger that initializes a request "trace"
metric_collector.record_parsed_request,
handlers.serve_custom_service_request_handlers,
load_service, # once we have the service request we can make sure we load the service
Expand Down
1 change: 1 addition & 0 deletions localstack/aws/handlers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,4 @@
set_close_connection_header = legacy.set_close_connection_header
push_request_context = legacy.push_request_context
pop_request_context = legacy.pop_request_context
log_request = logging.RequestLogger()
Copy link

Choose a reason for hiding this comment

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

style: Consider adding a comment explaining the purpose of this new logger

45 changes: 39 additions & 6 deletions localstack/aws/handlers/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from localstack.http import Response
from localstack.http.request import restore_payload
from localstack.logging.format import AwsTraceLoggingFormatter, TraceLoggingFormatter
from localstack.logging.setup import create_default_handler

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -72,11 +71,12 @@ def internal_http_logger(self):

# make sure loggers are loaded after logging config is loaded
def _prepare_logger(self, logger: logging.Logger, formatter: Type):
if logger.isEnabledFor(logging.DEBUG):
logger.propagate = False
handler = create_default_handler(logger.level)
handler.setFormatter(formatter())
logger.addHandler(handler)
# TODO: uncommenting this will block .http and .aws logs from being propagated
# if logger.isEnabledFor(logging.DEBUG):
# logger.propagate = False
# handler = create_default_handler(logger.level)
# handler.setFormatter(formatter())
# logger.addHandler(handler)
return logger
Comment on lines 73 to 80
Copy link

Choose a reason for hiding this comment

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

style: Commented-out code block prevents propagation of .http and .aws logs. Consider removing or implementing a configuration option to control this behavior.


def _log(self, context: RequestContext, response: Response):
Expand All @@ -95,6 +95,10 @@ def _log(self, context: RequestContext, response: Response):
response.status_code,
context.service_exception.code,
extra={
"request_id": context.request_id,
"service": context.service.service_name,
"operation": context.operation.name,
"status_code": response.status_code,
# context
"account_id": context.account_id,
"region": context.region,
Expand All @@ -117,6 +121,10 @@ def _log(self, context: RequestContext, response: Response):
context.operation.name,
response.status_code,
extra={
"request_id": context.request_id,
"service": context.service.service_name,
"operation": context.operation.name,
"status_code": response.status_code,
# context
"account_id": context.account_id,
"region": context.region,
Expand All @@ -142,6 +150,9 @@ def _log(self, context: RequestContext, response: Response):
context.request.path,
response.status_code,
extra={
"request_id": context.request_id or "NOREQUESTID",
"http_method": context.request.method,
"status_code": response.status_code,
# request
"input_type": "Request",
"input": restore_payload(context.request),
Expand All @@ -152,3 +163,25 @@ def _log(self, context: RequestContext, response: Response):
"response_headers": dict(response.headers),
},
)


class RequestLogger:
def __call__(self, _: HandlerChain, context: RequestContext, response: Response):
if context.request.path == "/health" or context.request.path == "/_localstack/health":
# special case so the health check doesn't spam the logs
return
Comment on lines +170 to +172
Copy link

Choose a reason for hiding this comment

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

style: Duplicate health check logic. Consider extracting this into a separate method to avoid repetition.


logger = logging.getLogger("localstack.request.raw_http")
logger.info(
"%s %s",
context.request.method,
context.request.path,
extra={
"request_id": context.request_id or "NOREQUESTID",
"http_method": context.request.method,
# request
"input_type": "Request",
"input": restore_payload(context.request), # TODO: remove?
"request_headers": dict(context.request.headers),
},
)
43 changes: 43 additions & 0 deletions localstack/logging/setup.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import json
import logging
import sys
import warnings

from localstack_snapshot.util.encoding import CustomJsonEncoder

from localstack import config, constants

from .format import AddFormattedAttributes, DefaultFormatter
Expand Down Expand Up @@ -87,6 +90,32 @@ def create_default_handler(log_level: int):
return log_handler


class JsonFormatter(logging.Formatter):
Copy link

Choose a reason for hiding this comment

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

style: JsonFormatter implementation is naive and may not handle all data types correctly. Consider using a more robust JSON serialization method.

def format(self, record: logging.LogRecord) -> str:
# TODO: extras are currently flat at root level and not nested
data = {}
skip_kw = ["msg", "message", "args", "exc_info"]
for k, v in record.__dict__.items():
if k in skip_kw:
continue
if v is not None:
data[k] = v
data["message"] = record.getMessage()

if record.exc_info:
data["error_type"] = record.exc_info[0].__name__
data["error_msg"] = str(record.exc_info[1])
if record.exc_text:
data["exc_text"] = record.exc_text

# import re
# reg = re.compile(r"./localstack/services/([^/])")
# if "localstack/services/" in record.pathname:
# service = record.pathname.split("")
# data["service"] =
Comment on lines +111 to +115
Copy link

Choose a reason for hiding this comment

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

style: Commented-out code for service extraction. Either implement or remove this block.

return json.dumps(data, cls=CustomJsonEncoder)


def setup_logging(log_level=logging.INFO) -> None:
"""
Configures the python logging environment for LocalStack.
Expand All @@ -109,6 +138,20 @@ def setup_logging(log_level=logging.INFO) -> None:
for logger, level in default_log_levels.items():
logging.getLogger(logger).setLevel(level)

# Configure JSON logs
# TODO: make configurable/opt-in
logging.basicConfig(level=logging.DEBUG)
file_handler = logging.FileHandler("/tmp/localstack.log", mode="w", encoding="utf-8")
Comment on lines +142 to +144
Copy link

Choose a reason for hiding this comment

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

style: JSON logging is not configurable and uses a hardcoded file path. Make this configurable as mentioned in the TODO.


file_handler.setFormatter(JsonFormatter())
logging.root.addHandler(file_handler)
logging.root.setLevel(logging.DEBUG) # FIXME
Copy link

Choose a reason for hiding this comment

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

logic: Setting root logger to DEBUG level may be too verbose. Implement proper log level configuration.


# silence noisy libs by default
logging.getLogger("werkzeug").setLevel(logging.CRITICAL)
logging.getLogger("stevedore").setLevel(logging.CRITICAL)
logging.getLogger("botocore").setLevel(logging.CRITICAL)


def setup_hypercorn_logger(hypercorn_config) -> None:
"""
Expand Down
1 change: 1 addition & 0 deletions localstack/services/lambda_/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,7 @@ def create_function(
f"Value {request.get('Runtime')} at 'runtime' failed to satisfy constraint: Member must satisfy enum value set: {VALID_RUNTIMES} or be a valid ARN",
Type="User",
)

request_function_name = request["FunctionName"]
# Validate FunctionName:
# a) Function name: just function name (max 64 chars)
Expand Down