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

chore(clickhouse): Capture final SQL in Sentry errors, again #21688

Merged
merged 3 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
23 changes: 15 additions & 8 deletions .github/workflows/ci-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,22 +197,29 @@ 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

this also needs to run this PR's migrations, I don't think check migrations below does that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Weirdly, it should, yet it looks like it never did. 🤔 Let's add that, sure

run: |
uv pip install --system -r requirements.txt -r requirements-dev.txt

- 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
Loading