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.

Sign up for GitHub

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

RHCLOUD-4283 - Enabling stdout logs #650

Merged
merged 25 commits into from
Jul 7, 2020
Merged

RHCLOUD-4283 - Enabling stdout logs #650

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f150695
rebasing
tahmidefaz May 1, 2020
4d83a84
using logconfig as default log_config_file
tahmidefaz May 4, 2020
f4d5bc2
Merge branch 'master' into stdout-logging
tahmidefaz May 4, 2020
afca122
Merge remote-tracking branch 'origin' into stdout-logging
tahmidefaz May 22, 2020
c983e0c
resolving conflicts
tahmidefaz May 22, 2020
e693f88
applying suggestions
tahmidefaz May 27, 2020
8e61d98
fixing cloudwatch logs
tahmidefaz May 28, 2020
188767e
Merge branch 'master' into stdout-logging
shdunning Jun 4, 2020
5a794b3
cloudwatch working without global vars :tada:
tahmidefaz Jun 15, 2020
9292e45
Merge branch 'master' into stdout-logging
tahmidefaz Jun 15, 2020
8a9a754
Merge branch 'master' into stdout-logging
Glutexo Jun 24, 2020
dfc53ba
Merge branch 'master' into stdout-logging
Glutexo Jun 29, 2020
3f29d5f
Stdout logging (#673)
tahmidefaz Jul 1, 2020
adc7f91
remove cw handler if creds not present
tahmidefaz Jul 2, 2020
6a0bd5b
freaking lint :man_facepalming:
tahmidefaz Jul 2, 2020
4dee8cb
reverting
tahmidefaz Jul 2, 2020
c7bda5e
reverting
tahmidefaz Jul 2, 2020
36895ea
fix
tahmidefaz Jul 2, 2020
060c8a5
Merge branch 'master' into stdout-logging
tahmidefaz Jul 2, 2020
68dd40a
deleting logconfig.ini
tahmidefaz Jul 2, 2020
a824ae5
Merge branch 'stdout-logging' of github.com:RedHatInsights/insights-h…
tahmidefaz Jul 2, 2020
213a10e
addressing suggestions
tahmidefaz Jul 2, 2020
270d3fe
Remove redundant logging
Glutexo Jul 3, 2020
9628b91
Remove config file variable from Makefile
Glutexo Jul 3, 2020
8ad446c
update README
jharting Jul 7, 2020
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
3 changes: 1 addition & 2 deletions app/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
from app.validators import verify_uuid_format # noqa: 401
from tasks import init_tasks

logger = get_logger(__name__)

IDENTITY_HEADER = "x-rh-identity"
REQUEST_ID_HEADER = "x-rh-insights-request-id"
UNKNOWN_REQUEST_ID_VALUE = "-1"
Expand All @@ -41,6 +39,7 @@ def create_app(config_name, start_tasks=False, start_payload_tracker=False):
# This feels like a hack but it is needed. The logging configuration
# needs to be setup before the flask app is initialized.
configure_logging(config_name)
logger = get_logger(__name__) # we need to call the logger after configuring it, not before
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the comment actually true? Even if we get the logger before the configuration, its configuration still gets updated. It makes perfect sense to get the logger after the configuration, but technically it’s not necessary.

Suggested change
logger = get_logger(__name__) # we need to call the logger after configuring it, not before
logger = get_logger(__name__)

To reproduce, swap these two lines and see the logging still works the same. Compare to how it doesn’t work if you try to log something before the configuration.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe the reason I moved this line around was that due to the way logging was setup earlier, some of the logs weren't being produced. However, after the logging refactoring, that seems to be fixed. I will move it back to where it was earlier. 👍


app_config = Config(RuntimeEnvironment.server)
app_config.log_configuration(config_name)
Expand Down
33 changes: 22 additions & 11 deletions app/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@

threadctx = local()

session_boto3 = None

tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved

def configure_logging(config_name):
env_var_name = "INVENTORY_LOGGING_CONFIG_FILE"
log_config_file = os.getenv(env_var_name)
log_config_file = os.getenv(env_var_name, "logconfig.ini")
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
if log_config_file is not None:
# The logging module throws an odd error (KeyError) if the
# config file is not found. Hopefully, this makes it more clear.
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -43,7 +45,6 @@ def _configure_watchtower_logging_handler():
aws_region_name = os.getenv("AWS_REGION_NAME", None)
log_group = os.getenv("AWS_LOG_GROUP", "platform")
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
stream_name = os.getenv("AWS_LOG_STREAM", _get_hostname()) # default to hostname
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
create_log_group = str(os.getenv("AWS_CREATE_LOG_GROUP")).lower() == "true"

if all([aws_access_key_id, aws_secret_access_key, aws_region_name, stream_name]):
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
print(f"Configuring watchtower logging (log_group={log_group}, stream_name={stream_name})")
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -53,19 +54,24 @@ def _configure_watchtower_logging_handler():
region_name=aws_region_name,
)

root = logging.getLogger()
handler = watchtower.CloudWatchLogHandler(
boto3_session=boto3_session,
log_group=log_group,
stream_name=stream_name,
create_log_group=create_log_group,
)
handler.setFormatter(logstash_formatter.LogstashFormatterV1())
root.addHandler(handler)
global session_boto3
session_boto3 = boto3_session
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
global session_boto3
session_boto3 = boto3_session
handler = _get_cloudwatch_handler(aws_access_key_id, aws_secret_access_key, aws_region_name, aws_log_group, aws_stream_name, create_log_group)
logger = logging.addHandler(handler)

Copy link
Member Author

Choose a reason for hiding this comment

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

see my comment below

else:
print("Unable to configure watchtower logging. Please verify watchtower logging configuration!")


def _get_cloudwatch_handler():
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
global session_boto3
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
group = os.getenv("AWS_LOG_GROUP", "platform")
stream = os.getenv("AWS_LOG_STREAM", _get_hostname())
create_log_group = str(os.getenv("AWS_CREATE_LOG_GROUP")).lower() == "true"
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
handler = watchtower.CloudWatchLogHandler(
boto3_session=session_boto3, log_group=group, stream_name=stream, create_log_group=create_log_group
)
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved
handler.setFormatter(logstash_formatter.LogstashFormatterV1())
return handler

tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved

def _get_hostname():
return os.uname()[1]

Expand Down Expand Up @@ -122,5 +128,10 @@ def get_logger(name):
log_level = os.getenv("INVENTORY_LOG_LEVEL", "INFO").upper()
logger = logging.getLogger(LOGGER_PREFIX + name)
logger.addFilter(ContextualFilter())

global session_boto3
if session_boto3 is not None:
logger.addHandler(_get_cloudwatch_handler())
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved

logger.setLevel(log_level)
return logger
tahmidefaz marked this conversation as resolved.
Show resolved Hide resolved