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

Respect the configuration of logger parameter across App/AsyncApp loggers #617

Closed
brian-nguyen-bolt opened this issue Mar 18, 2022 · 2 comments · Fixed by #618
Closed

Respect the configuration of logger parameter across App/AsyncApp loggers #617

brian-nguyen-bolt opened this issue Mar 18, 2022 · 2 comments · Fixed by #618
Assignees
Milestone

Comments

@brian-nguyen-bolt
Copy link

brian-nguyen-bolt commented Mar 18, 2022

When I pass in a custom logger into App(), I am expecting that logger from def home_tab_opened listener is the same logger as new_logger.

slack_app = App (
    token=<SLACK_BOT_TOKEN>,
    logger=new_logger
)

@slack_app.event("app_home_opened")
def home_tab_opened(client: WebClient, event: dict, logger: Logger):

Reproducible in:

import logging
from slack_sdk.web.client import WebClient
from slack_bolt.context.ack import Ack
from slack_bolt import App
from slack_bolt.adapter.socket_mode import SocketModeHandler

info_logger = logging.getLogger("bolt")
info_logger.setLevel(level=logging.INFO)

slack_app = App(
    token=<SLACK_BOT_TOKEN>,
    logger=info_logger
)

# Slack Config for SocketMode
info_logger.info("Bolt App is in Socket Mode")
handler = SocketModeHandler(slack_app, <SLACK_APP_TOKEN>)
handler.connect()

@slack_app.event(Constants.APP_HOME_OPENED_EVENT)
def home_tab_opened(client: WebClient, event: dict, logger: Logger):
    """
    This will load the App Home Page
    """
    print(f"Logger from listener {logger.level}") >>> This will print 30 - I was expecting this logger to be 20 since we set the logger when initialing App
    print(f"Expected logger {info_logger.level}") >>> This will print 20
    try:
        client.views_publish(
            user_id=event["user"],
            view=app_home_template
        )
    except Exception as e:
        logger.error(f"Error publishing home tab: {e}")

The slack_bolt version

root@b11dc6f4dcb5:/opt/project# pip freeze | grep slack
slack-bolt==1.11.6
slack-sdk==3.15.2

Python runtime version

root@b11dc6f4dcb5:/opt/project# python --version
Python 3.9.10

OS info

sw_vers did not work as a built the container from Docker using this image
FROM python:3.9

root@b11dc6f4dcb5:/opt/project# uname -v
#1 SMP Mon Nov 8 10:21:19 UTC 2021

Steps to reproduce:

(Share the commands to run, source code, and project settings (e.g., setup.py))

  1. Setup an app in Slack that listens for app_home_open event
  2. Add tokens and start up the app locally
  3. Go to Slack and go to the app home page to trigger the event with a debug breakpoint where the loggers are

Expected result:

I expect the Logger passed in App() is the same logger the framework passes use to the listener function

Print would show

Logger from listener 20 >> logging level info
Expected logger 20 >> logging level info

Actual result:

Logger from listener 30 >> logging level warning
Expected logger 20 >> logging level info

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

@seratch seratch changed the title Do we expect the logger passed in App() to be the same logger from action listener? Respect the configuration of logger parameter across App/AsyncApp loggers Mar 18, 2022
@seratch seratch added this to the 1.13.0 milestone Mar 18, 2022
@seratch seratch self-assigned this Mar 18, 2022
seratch added a commit to seratch/bolt-python that referenced this issue Mar 18, 2022
@seratch
Copy link
Member

seratch commented Mar 18, 2022

Hi @brian-nguyen-bolt, thanks for the great feedback!

The logger in the App/AsyncApp constructor is supposed to be used only for logging inside the bolt-python framework. For simple use cases, we have been suggesting to configure logging level at root level and it works for bolt-python apps. However, indeed, this is not intuitive and less flexible.

I just submitted a pull request #618 fixing this issue, which resolves the issue reported here. The change will be released shortly.

@brian-nguyen-bolt
Copy link
Author

Thank you so much @seratch. I am very impressed how fast someone looked into this. Thank you very much 🙏 🙇‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants