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

feat(integrations): Add integration for clickhouse-driver #2167

Merged
merged 26 commits into from
Sep 13, 2023
Merged

feat(integrations): Add integration for clickhouse-driver #2167

merged 26 commits into from
Sep 13, 2023

Conversation

mimre25
Copy link
Contributor

@mimre25 mimre25 commented Jun 12, 2023 •
edited
Loading

Adds an integration that automatically facilitates tracing/recording of all queries, their parameters, data, and results.

Closes #2154
Companion PR for docs: getsentry/sentry-docs#7129

@mimre25
feat(integrations): Add integration for clickhouse-driver>=0.2.0
f3ad269 
Adds an integration that automatically facilitates tracing/recording of
all queries, their parameters, data, and results.

See getsentry#2154
@mimre25 mimre25 marked this pull request as ready for review June 12, 2023 16:20
@mimre25 mimre25 mentioned this pull request Jun 12, 2023
Closed
@mimre25 mimre25 mentioned this pull request Jun 12, 2023
Merged
3 tasks
@smeubank smeubank mentioned this pull request Jun 13, 2023
Closed
@shanamatthews
Copy link

@antonpirker - I was going to ping you to remind you about the docs PR for this, but it looks like you weren't even tagged in this PR yet 😆

@sentrivana sentrivana added the New Integration Integrating with a new framework or library label Jun 28, 2023
@antonpirker
Copy link
Member

Hey @mimre25 !
Thanks for the integrations. Unfortunately we are a bit swamped right now, so please be patient. We have this on our radar, but it might take some time to do a review!

@antonpirker
Copy link
Member

Hey @mimre25 !

I finally had time to look at your PR. Really great work! I added some code for adding more db connection data to the span (we need this to some magic stuff on the Sentry backend) and also made sure that the db.params are only sent to Sentry when send_default_pii parameter of the sentry_sdk.init() is set to True. (so by default we will not leak personal data.

Also added the tests to our test mastrix. There is still some work to do, but I will have a look at this tomorrow.

Do you know if there is a way to mock clickhouse for the tests? (I am not sure it is a good idea to spin up clickhouse in CI...)

@mimre25
Copy link
Contributor Author

mimre25 commented Sep 13, 2023

Unfortunately, I don't know anything to mock clickhouse for tests.
At work we even spin up a whole cluster (dockerized) for testing (unit + integration), because we haven't found anything when we started to adopt clickhouse

If you find something, though, please let me know, I would be very interested in that as well!

@antonpirker antonpirker changed the title feat(integrations): Add integration for clickhouse-driver>=0.2.0 feat(integrations): Add integration for clickhouse-driver Sep 13, 2023
@antonpirker antonpirker enabled auto-merge (squash) September 13, 2023 10:35
@antonpirker
Copy link
Member

Hey @mimre25 !
I think we are there!
(I just made sure all the tests are run and work in CI, plus added some more tests)

When all the tests are green this will be merged and released this week!
Thanks again for the great work. Really amazing!

antonpirker
antonpirker approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

🚢

@antonpirker antonpirker merged commit ad0ed59 into getsentry:master Sep 13, 2023
sentrivana pushed a commit that referenced this pull request Sep 18, 2023
@mimre25 @sentrivana
feat(integrations): Add integration for clickhouse-driver (#2167)
3edcde7 
Adds an integration that automatically facilitates tracing/recording of all queries, their parameters, data, and results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antonpirker antonpirker antonpirker approved these changes

@sentrivana sentrivana Awaiting requested review from sentrivana

Assignees

@sentrivana sentrivana

Labels
New Integration Integrating with a new framework or library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clickhouse Integration
4 participants
@mimre25 @shanamatthews @antonpirker @sentrivana