diff --git a/src/sentry/integrations/slack/client.py b/src/sentry/integrations/slack/client.py index 6488eb730b0685..97379b750d057d 100644 --- a/src/sentry/integrations/slack/client.py +++ b/src/sentry/integrations/slack/client.py @@ -84,6 +84,7 @@ def track_response_data( span: Span | None = None, error: Optional[str] = None, resp: Optional[Response] = None, + extra: Optional[Mapping[str, str]] = None, ) -> None: # if no span was passed, create a dummy to which to add data to avoid having to wrap every # span call in `if span` @@ -121,6 +122,7 @@ def track_response_data( ) extra = { + **(extra or {}), self.integration_type: self.name, "status_string": str(code), "error": str(error)[:256] if error else None, diff --git a/src/sentry/shared_integrations/client/base.py b/src/sentry/shared_integrations/client/base.py index ff67c9cd250e7b..677bcb3974bcb5 100644 --- a/src/sentry/shared_integrations/client/base.py +++ b/src/sentry/shared_integrations/client/base.py @@ -243,6 +243,11 @@ def _request( if span and existing_transaction: span.set_data("existing_transaction", existing_transaction) + extra = {"url": full_url} + # It shouldn't be possible for integration_type to be null. + if self.integration_type: + extra[self.integration_type] = self.name + try: with build_session() as session: finalized_request = self.finalize_request(_prepared_request) @@ -266,30 +271,27 @@ def _request( return resp resp.raise_for_status() except RestrictedIPAddress as e: - self.track_response_data("restricted_ip_address", span, e) + self.track_response_data("restricted_ip_address", span, e, extra=extra) self.record_error(e) raise ApiHostError.from_exception(e) from e except ConnectionError as e: - self.track_response_data("connection_error", span, e) + self.track_response_data("connection_error", span, e, extra=extra) self.record_error(e) raise ApiHostError.from_exception(e) from e except Timeout as e: - self.track_response_data("timeout", span, e) + self.track_response_data("timeout", span, e, extra=extra) self.record_error(e) raise ApiTimeoutError.from_exception(e) from e except HTTPError as e: error_resp = e.response if error_resp is None: - self.track_response_data("unknown", span, e) + self.track_response_data("unknown", span, e, extra=extra) - # It shouldn't be possible for integration_type to be null. - extra = {"url": full_url} - if self.integration_type: - extra[self.integration_type] = self.name self.logger.exception("request.error", extra=extra) self.record_error(e) raise ApiError("Internal Error", url=full_url) from e - self.track_response_data(error_resp.status_code, span, e) + + self.track_response_data(error_resp.status_code, span, e, extra=extra) self.record_error(e) raise ApiError.from_response(error_resp, url=full_url) from e @@ -301,19 +303,19 @@ def _request( # which is a ChunkedEncodingError caused by a ProtocolError caused by a ConnectionResetError. # Rather than worrying about what the other layers might be, we just stringify to detect this. if "ConnectionResetError" in str(e): - self.track_response_data("connection_reset_error", span, e) + self.track_response_data("connection_reset_error", span, e, extra=extra) self.record_error(e) raise ApiConnectionResetError("Connection reset by peer", url=full_url) from e # The same thing can happen with an InvalidChunkLength exception, which is a subclass of HTTPError if "InvalidChunkLength" in str(e): - self.track_response_data("invalid_chunk_length", span, e) + self.track_response_data("invalid_chunk_length", span, e, extra=extra) self.record_error(e) raise ApiError("Connection broken: invalid chunk length", url=full_url) from e # If it's not something we recognize, let the caller deal with it raise e - self.track_response_data(resp.status_code, span, None, resp) + self.track_response_data(resp.status_code, span, None, resp, extra=extra) self.record_response(resp) diff --git a/src/sentry/shared_integrations/track_response.py b/src/sentry/shared_integrations/track_response.py index db6379e82c04a6..2e48d35a2ad692 100644 --- a/src/sentry/shared_integrations/track_response.py +++ b/src/sentry/shared_integrations/track_response.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from typing import Mapping, Optional from django.utils.functional import cached_property from rest_framework.response import Response @@ -41,6 +42,7 @@ def track_response_data( span: Span | None = None, error: Exception | None = None, resp: Response | None = None, + extra: Optional[Mapping[str, str]] = None, ) -> None: # if no span was passed, create a dummy to which to add data to avoid having to wrap every # span call in `if span` @@ -57,12 +59,13 @@ def track_response_data( except ValueError: span.set_status(str(code)) - extra = { + log_params = { + **(extra or {}), "status_string": str(code), "error": str(error)[:256] if error else None, } if self.integration_type: - extra[self.integration_type] = self.name + log_params[self.integration_type] = self.name - extra.update(getattr(self, "logging_context", None) or {}) - self.logger.info(f"{self.integration_type}.http_response", extra=extra) + log_params.update(getattr(self, "logging_context", None) or {}) + self.logger.info(f"{self.integration_type}.http_response", extra=log_params)