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

Jump to bottom

chore(slack): remove old client from webhook handling code #74104

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+103 −442
Open

chore(slack): remove old client from webhook handling code #74104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
478913a
remove old client from webhook handling code
cathteng Jul 10, 2024
ca49fbb
add some metrics
cathteng Jul 10, 2024
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: 0 additions & 2 deletions src/sentry/features/temporary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,6 @@ def register_temporary_features(manager: FeatureManager):
# Feature flags for migrating to the Slack SDK WebClient
# Use new Slack SDK Client in get_channel_id_with_timeout
manager.add("organizations:slack-sdk-get-channel-id", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE)
# Use new Slack SDK Client in SlackActionEndpoint
manager.add("organizations:slack-sdk-webhook-handling", OrganizationFeature, FeatureHandlerStrategy.OPTIONS)
# Use new Slack SDK Client in SlackActionEndpoint's `view.open`
manager.add("organizations:slack-sdk-action-view-open", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE)
# Use new Slack SDK Client for SlackNotifyBasicMixin
Expand Down
6 changes: 6 additions & 0 deletions src/sentry/integrations/slack/metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
SLACK_WEBHOOK_DM_ENDPOINT_FAILURE_DATADOG_METRIC = "sentry.integrations.slack.dm_endpoint.failure"
SLACK_EVENT_ENDPOINT_SUCCESS_DATADOG_METRIC = "sentry.integrations.slack.event_endpoint.success"
SLACK_EVENT_ENDPOINT_FAILURE_DATADOG_METRIC = "sentry.integrations.slack.event_endpoint.failure"
SLACK_WEBHOOK_GROUP_ACTIONS_SUCCESS_DATADOG_METRIC = (
"sentry.integrations.slack.group_actions.success"
)
SLACK_WEBHOOK_GROUP_ACTIONS_FAILURE_DATADOG_METRIC = (
"sentry.integrations.slack.group_actions.failure"
)

# Slack Commands
SLACK_COMMANDS_ENDPOINT_SUCCESS_DATADOG_METRIC = (
Expand Down
80 changes: 45 additions & 35 deletions src/sentry/integrations/slack/webhooks/action.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
from sentry.integrations.slack.client import SlackClient
from sentry.integrations.slack.message_builder import SlackBody
from sentry.integrations.slack.message_builder.issues import SlackIssuesMessageBuilder
from sentry.integrations.slack.metrics import (
SLACK_WEBHOOK_GROUP_ACTIONS_FAILURE_DATADOG_METRIC,
SLACK_WEBHOOK_GROUP_ACTIONS_SUCCESS_DATADOG_METRIC,
)
from sentry.integrations.slack.requests.action import SlackActionRequest
from sentry.integrations.slack.requests.base import SlackRequestError
from sentry.integrations.slack.sdk_client import SlackSdkClient
Expand All @@ -42,6 +46,7 @@
from sentry.notifications.utils.actions import BlockKitMessageAction, MessageAction
from sentry.shared_integrations.exceptions import ApiError
from sentry.users.services.user import RpcUser
from sentry.utils import metrics

from ..utils import logger

Expand Down Expand Up @@ -655,42 +660,37 @@ def _handle_group_actions(
issue_details=True,
skip_fallback=True,
).build()
body = self.construct_reply(
blocks, is_message=slack_request.callback_data["is_message"]
)
# use the original response_url to update the link attachment
if not features.has(
"organizations:slack-sdk-webhook-handling", group.project.organization
):
slack_client = SlackClient(integration_id=slack_request.integration.id)
try:
private_metadata = orjson.loads(
slack_request.data["view"]["private_metadata"],
)
slack_client.post(private_metadata["orig_response_url"], data=body, json=True)
except ApiError as e:
logger.error("slack.action.response-error", extra={"error": str(e)})

else:
json_blocks = orjson.dumps(blocks.get("blocks")).decode()
view = View(**slack_request.data["view"])
try:
private_metadata = orjson.loads(view.private_metadata)
webhook_client = WebhookClient(private_metadata["orig_response_url"])
webhook_client.send(
blocks=json_blocks, delete_original=False, replace_original=True
)
logger.info(
"slack.webhook.view_submission.success",
extra={
"integration_id": slack_request.integration.id,
"blocks": json_blocks,
},
)
except SlackApiError as e:
logger.error(
"slack.webhook.view_submission.response-error", extra={"error": str(e)}
)
# use the original response_url to update the link attachment
json_blocks = orjson.dumps(blocks.get("blocks")).decode()
view = View(**slack_request.data["view"])
try:
private_metadata = orjson.loads(view.private_metadata)
webhook_client = WebhookClient(private_metadata["orig_response_url"])
webhook_client.send(
blocks=json_blocks, delete_original=False, replace_original=True
)
logger.info(
"slack.webhook.view_submission.success",
extra={
"integration_id": slack_request.integration.id,
"blocks": json_blocks,
},
)
Comment on lines +673 to +679
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the success log now that we're moving to enabled? Plus we have the metrics for it to monitor

metrics.incr(
SLACK_WEBHOOK_GROUP_ACTIONS_SUCCESS_DATADOG_METRIC,
sample_rate=1.0,
tags={"type": "submit_modal"},
)
except SlackApiError as e:
metrics.incr(
SLACK_WEBHOOK_GROUP_ACTIONS_FAILURE_DATADOG_METRIC,
sample_rate=1.0,
tags={"type": "submit_modal"},
)
logger.error(
"slack.webhook.view_submission.response-error", extra={"error": str(e)}
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to log more information to help with debugging?

)

return self.respond()

Expand Down Expand Up @@ -763,7 +763,17 @@ def _handle_group_actions(
"slack.webhook.update_status.success",
extra={"integration_id": slack_request.integration.id, "blocks": json_blocks},
)
metrics.incr(
SLACK_WEBHOOK_GROUP_ACTIONS_SUCCESS_DATADOG_METRIC,
sample_rate=1.0,
tags={"type": "update_message"},
)
except SlackApiError as e:
metrics.incr(
SLACK_WEBHOOK_GROUP_ACTIONS_FAILURE_DATADOG_METRIC,
sample_rate=1.0,
tags={"type": "update_message"},
)
logger.error("slack.webhook.update_status.response-error", extra={"error": str(e)})

return self.respond(response)
Expand Down
Loading
Loading