-
Notifications
You must be signed in to change notification settings - Fork 853
Closed
Labels
Version: 3xenhancementM-T: A feature request for new functionalityM-T: A feature request for new functionalitysocket-mode
Milestone
Description
We have four Socket Mode client implementations (built-in, websocket-client, aiohttp, websockets). The granularity of the debug logging among them is not so consistent sometimes. It's not necessarily 100% the same but we can revisit some for even more consistency, particularly when trace_enabled is True.
Refer to the convo with @eddyg in the community workspace for more details: https://community.slack.com/archives/CHY642221/p1632434539059400?thread_ts=1632316107.057900&cid=CHY642221
eddyg Sep 22nd at 10:08 PM
With all the recent changes to python-slack-sdk around aiohttp Socket Mode, I’ve been running an app with debug logging enabled. I’ve attached some excerpted log lines. I can’t tell if these are “informational and expected” or “ideally this shouldn’t happen”. Any insight?
seratch:slack: 2 months ago
@eddyg The logs were added in the recent release for easier investigation of unexpected behaviors but indeed they may be a bit noisy… In the next release, I will make changes to enable these logs only when trace_enabled is True. Thanks for sharing this!
eddyg 2 months ago
I’m OK with extra logging at the “debug” level, it just wasn’t clear if those particular messages are “expected” and “normal”, or if they indicated a problem.
seratch:slack: 2 months ago
I understand it. The log messages can be improved not to bring such a confusion.
eddyg 2 months ago
If they indicated an issue, I just wanted to provide feedback to help improve things. :smile:
eddyg 2 months ago
The other thing I just noticed is that this commit changed when this debug message gets logged… it now requires trace_enabled to be set. Not sure if that was intentional or not; that debug log message was sometimes useful because it included the contents of the message received from Slack.
seratch:slack: 2 months ago
Oh, I intentionally moved the log to trace-mode to reduce noise. However, I just found that the logging policies across the four implementations seem to be inconsistent. I will revisit this and make the logging behavior too.
Category (place an x in each of the [ ])
- slack_sdk.web.WebClient (sync/async) (Web API client)
- slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
- slack_sdk.models (UI component builders)
- slack_sdk.oauth (OAuth Flow Utilities)
- slack_sdk.socket_mode (Socket Mode client)
- slack_sdk.audit_logs (Audit Logs API client)
- slack_sdk.scim (SCIM API client)
- slack_sdk.rtm (RTM client)
- slack_sdk.signature (Request Signature Verifier)
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.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Version: 3xenhancementM-T: A feature request for new functionalityM-T: A feature request for new functionalitysocket-mode
