Skip to content

Commit

Permalink
chore(clickhouse): Capture final SQL in Sentry errors, again (#21688)
Browse files Browse the repository at this point in the history
* Revert "revert: "chore(clickhouse): Capture final SQL in Sentry errors" (#21479)"

This reverts commit 1562781.

* Only upgrade `clickhouse-driver` to 0.2.6

* Add "Run migrations for this PR"
  • Loading branch information
Twixes authored Apr 22, 2024
1 parent a6618bb commit 0cf2955
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 32 deletions.
27 changes: 19 additions & 8 deletions .github/workflows/ci-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,22 +197,33 @@ jobs:
sudo apt-get update
sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl
- name: Install python dependencies
run: |
uv pip install --system -r requirements.txt -r requirements-dev.txt
# First running migrations from master, to simulate the real-world scenario

- uses: actions/checkout@v3
- name: Checkout master
uses: actions/checkout@v3
with:
ref: master

- name: Run migrations up to master
- name: Install python dependencies for master
run: |
# We need to ensure we have requirements for the master branch
# now also, so we can run migrations up to master.
uv pip install --system -r requirements.txt -r requirements-dev.txt
- name: Run migrations up to master
run: |
python manage.py migrate
- uses: actions/checkout@v3
# Now we can consider this PR's migrations

- name: Checkout this PR
uses: actions/checkout@v3

- name: Install python dependencies for this PR
run: |
uv pip install --system -r requirements.txt -r requirements-dev.txt
- name: Run migrations for this PR
run: |
python manage.py migrate
- name: Check migrations
run: |
Expand Down
1 change: 1 addition & 0 deletions posthog/clickhouse/client/escape.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ def escape_param_for_clickhouse(param: Any) -> str:
version_patch="placeholder server_info value",
revision="placeholder server_info value",
display_name="placeholder server_info value",
used_revision="placeholder server_info value",
timezone="UTC",
)
return escape_param(param, context=context)
10 changes: 5 additions & 5 deletions posthog/settings/sentry.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from sentry_sdk.integrations.django import DjangoIntegration
from sentry_sdk.integrations.logging import LoggingIntegration
from sentry_sdk.integrations.redis import RedisIntegration
from sentry_sdk.integrations.clickhouse_driver import ClickhouseDriverIntegration
from posthog.git import get_git_commit_full

from posthog.settings import get_from_env
Expand Down Expand Up @@ -141,8 +142,6 @@ def traces_sampler(sampling_context: dict) -> float:

def sentry_init() -> None:
if not TEST and os.getenv("SENTRY_DSN"):
sentry_sdk.utils.MAX_STRING_LENGTH = 10_000_000

# Setting this on enables more visibility, at the risk of capturing personal information we should not:
# - standard sentry "client IP" field, through send_default_pii
# - django access logs (info level)
Expand All @@ -151,7 +150,6 @@ def sentry_init() -> None:
send_pii = get_from_env("SENTRY_SEND_PII", type_cast=bool, default=False)

sentry_logging_level = logging.INFO if send_pii else logging.ERROR
sentry_logging = LoggingIntegration(level=sentry_logging_level, event_level=None)
profiles_sample_rate = get_from_env("SENTRY_PROFILES_SAMPLE_RATE", type_cast=float, default=0.0)

release = get_git_commit_full()
Expand All @@ -164,9 +162,11 @@ def sentry_init() -> None:
DjangoIntegration(),
CeleryIntegration(),
RedisIntegration(),
sentry_logging,
ClickhouseDriverIntegration(),
LoggingIntegration(level=sentry_logging_level, event_level=None),
],
request_bodies="always" if send_pii else "never",
max_request_body_size="always" if send_pii else "never",
max_value_length=8192, # Increased from the default of 1024 to capture SQL statements in full
sample_rate=1.0,
# Configures the sample rate for error events, in the range of 0.0 to 1.0 (default).
# If set to 0.1 only 10% of error events will be sent. Events are picked randomly.
Expand Down
9 changes: 2 additions & 7 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ coreapi==2.3.3
coreschema==0.0.4
# via coreapi
coverage[toml]==5.5
# via
# coverage
# pytest-cov
# via pytest-cov
datamodel-code-generator==0.25.2
# via -r requirements-dev.in
django==4.2.11
Expand Down Expand Up @@ -92,9 +90,7 @@ exceptiongroup==1.2.0
faker==17.5.0
# via -r requirements-dev.in
fakeredis[lua]==2.11.0
# via
# -r requirements-dev.in
# fakeredis
# via -r requirements-dev.in
flaky==3.7.0
# via -r requirements-dev.in
freezegun==1.2.2
Expand Down Expand Up @@ -172,7 +168,6 @@ pydantic[email]==2.5.3
# via
# -c requirements.txt
# datamodel-code-generator
# pydantic
pydantic-core==2.14.6
# via
# -c requirements.txt
Expand Down
4 changes: 2 additions & 2 deletions requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ boto3-stubs[s3]
brotli==1.1.0
celery==5.3.4
celery-redbeat==2.1.1
clickhouse-driver==0.2.4
clickhouse-driver==0.2.6
clickhouse-pool==0.5.3
cryptography==37.0.2
defusedxml==0.6.0
Expand Down Expand Up @@ -79,7 +79,7 @@ requests-oauthlib==1.3.0
s3fs==2023.10.0
stripe==7.4.0
selenium==4.1.5
sentry-sdk==1.14.0
sentry-sdk[clickhouse-driver,celery,openai,django]~=1.44.1
semantic_version==2.8.5
scikit-learn==1.4.0
slack_sdk==3.17.1
Expand Down
22 changes: 12 additions & 10 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ aioboto3==12.0.0
aiobotocore[boto3]==2.7.0
# via
# aioboto3
# aiobotocore
# s3fs
aiohttp==3.9.3
# via
Expand Down Expand Up @@ -82,6 +81,7 @@ celery==5.3.4
# via
# -r requirements.in
# celery-redbeat
# sentry-sdk
celery-redbeat==2.1.1
# via -r requirements.in
certifi==2019.11.28
Expand Down Expand Up @@ -115,10 +115,11 @@ click-plugins==1.1.1
# via celery
click-repl==0.3.0
# via celery
clickhouse-driver==0.2.4
clickhouse-driver==0.2.6
# via
# -r requirements.in
# clickhouse-pool
# sentry-sdk
clickhouse-pool==0.5.3
# via -r requirements.in
cryptography==37.0.2
Expand Down Expand Up @@ -165,6 +166,7 @@ django==4.2.11
# djangorestframework
# djangorestframework-dataclasses
# drf-spectacular
# sentry-sdk
django-axes==5.9.0
# via -r requirements.in
django-cors-headers==3.5.0
Expand Down Expand Up @@ -253,7 +255,6 @@ giturlparse==0.12.0
# via dlt
google-api-core[grpc]==2.11.1
# via
# google-api-core
# google-cloud-bigquery
# google-cloud-core
google-auth==2.22.0
Expand Down Expand Up @@ -387,7 +388,9 @@ oauthlib==3.1.0
# requests-oauthlib
# social-auth-core
openai==1.10.0
# via -r requirements.in
# via
# -r requirements.in
# sentry-sdk
openapi-schema-validator==0.6.2
# via openapi-spec-validator
openapi-spec-validator==0.7.1
Expand Down Expand Up @@ -449,9 +452,7 @@ protobuf==4.22.1
# proto-plus
# temporalio
psycopg[binary]==3.1.13
# via
# -r requirements.in
# psycopg
# via -r requirements.in
psycopg-binary==3.1.13
# via psycopg
psycopg2-binary==2.9.7
Expand Down Expand Up @@ -590,7 +591,7 @@ semantic-version==2.8.5
# via -r requirements.in
semver==3.0.2
# via dlt
sentry-sdk==1.14.0
sentry-sdk[celery,clickhouse-driver,django,openai]==1.44.1
# via -r requirements.in
simplejson==3.19.2
# via dlt
Expand Down Expand Up @@ -649,7 +650,9 @@ tenacity==8.2.3
threadpoolctl==3.3.0
# via scikit-learn
tiktoken==0.6.0
# via -r requirements.in
# via
# -r requirements.in
# sentry-sdk
token-bucket==0.3.0
# via -r requirements.in
tomlkit==0.12.3
Expand Down Expand Up @@ -711,7 +714,6 @@ urllib3[secure,socks]==1.26.18
# requests
# selenium
# sentry-sdk
# urllib3
urllib3-secure-extra==0.1.0
# via urllib3
vine==5.0.0
Expand Down

0 comments on commit 0cf2955

Please sign in to comment.