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

Add integerations for socket and grpc #1911

Merged
merged 38 commits into from
Mar 30, 2023
Merged

Add integerations for socket and grpc #1911

merged 38 commits into from
Mar 30, 2023

Conversation

hossein-raeisi
Copy link
Contributor

@hossein-raeisi hossein-raeisi commented Feb 18, 2023 •
edited by antonpirker
Loading

  • The gRPC integration instruments all incoming requests and outgoing unary-unary, unary-stream grpc requests using grpcio channels.
    Use this integration to start or continue transactions for incoming grpc requests, create spans for outgoing requests,
    and ensure traces are properly propagated to downstream services.
  • The Socket integration to create spans for dns resolves and connection creations.

Docs: getsentry/sentry-docs#6327

@hossein-raeisi
feat(integrations) Implement an integration for tracing socket module
cb107fd 
Implement SocketIntegration; patches socket's getaddrinfo and create_connection and adds spans to them
@hossein-raeisi hossein-raeisi force-pushed the add-socket-and-grpc-integrations branch 2 times, most recently from 49c8d30 to 61bdaf1 Compare February 20, 2023 16:06
@hossein-raeisi
feat(tracing) Implement an interceptor for grpc server
41631e4 
Implement gRPC ServerInterceptor; starts(or continues) a transaction for each grpc request
@hossein-raeisi hossein-raeisi mentioned this pull request Feb 20, 2023
Merged
@hossein-raeisi
Copy link
Contributor Author

docs pull request

@hossein-raeisi hossein-raeisi changed the title Draft: Add integerations for socket and grpc Add integerations for socket and grpc Feb 20, 2023
@hossein-raeisi
Copy link
Contributor Author

hossein-raeisi commented Feb 20, 2023 •
edited
Loading

I think this pull request has reached a stable state, would appreciate a review @sl0thentr0py @antonpirker

@hossein-raeisi
feat(tracing) Implement unary-unary and unary-stream interceptors for…
5c2db86 
… grpc stub

Implement gRPC UnaryUnaryClientInterceptor and grpc.UnaryStreamClientInterceptor; starts a span for grpc call and sends propagation headers in metadata
@hossein-raeisi hossein-raeisi marked this pull request as ready for review February 22, 2023 05:57
@sl0thentr0py
Copy link
Member

ty so much for the contribution @Hossain-raeisi , we'll try to get to this next week

@antonpirker
Copy link
Member

Hey @Hossain-raeisi
Amazing contribution! As @sl0thentr0py said we will have a look next week, we are already very stoked about this!

@hossein-raeisi
Copy link
Contributor Author

Hey!
Any updates @sl0thentr0py @antonpirker ?

@antonpirker
Copy link
Member

Have not had time yet. Please hang in there.

@antonpirker antonpirker self-assigned this Mar 3, 2023
@antonpirker
Copy link
Member

Hey @Hossain-raeisi !
At first glance it looks good.
I fixed some smaller things to make CI happy.

Still, there are some errors in CI mostly about typing:
https://github.com/getsentry/sentry-python/actions/runs/4321738769/jobs/7543323597

Could you fix them?
(Ignore the gevent errors, I have to fix those)

@hossein-raeisi
Copy link
Contributor Author

Hi @antonpirker
thanks for the review
I made some changes and hopefully they should solve most of the errors, but I dont think I can do anything about these ones
https://github.com/getsentry/sentry-python/actions/runs/4321738769/jobs/7543323597#step:4:81

@hossein-raeisi hossein-raeisi force-pushed the add-socket-and-grpc-integrations branch from 7f992e3 to 6ce7834 Compare March 5, 2023 12:23
@hossein-raeisi hossein-raeisi force-pushed the add-socket-and-grpc-integrations branch from 9573358 to 01a51d3 Compare March 10, 2023 10:38
@hossein-raeisi
Copy link
Contributor Author

Hey @antonpirker!
do you have any free time in next few days to pair and finish up this pr?

@antonpirker
Copy link
Member

I changed now the setup, that GRPC tests are only run in Python 3.7+ (because the protobuffers in the tests did not work with version installed on older pythons)

Is this fine with you @Oogwey ?

@antonpirker
Copy link
Member

antonpirker commented Mar 29, 2023 •
edited
Loading

I have one more ask @Oogwey , could you please rename TestService, TestMessage and test_grpc_server and prefix it with something like Grpc or whatever.
Pytest it trying to collect tests from all files prefixed test_ and all classes prefixed Test so pytest struggles with out code. Thank You!

@hossein-raeisi
Copy link
Contributor Author

hossein-raeisi commented Mar 29, 2023 •
edited
Loading

I changed now the setup, that GRPC tests are only run in Python 3.7+ (because the protobuffers in the tests did not work with version installed on older pythons)

Is this fine with you @Oogwey ?

alright with me 👍

@hossein-raeisi hossein-raeisi force-pushed the add-socket-and-grpc-integrations branch from 510852e to d81fe6c Compare March 29, 2023 16:46
@hossein-raeisi
Copy link
Contributor Author

I have one more ask @Oogwey , could you please rename TestService, TestMessage and test_grpc_server and prefix it with something like Grpc or whatever. Pytest it trying to collect tests from all files prefixed test_ and all classes prefixed Test so pytest struggles with out code. Thank You!

Done!

@antonpirker
Copy link
Member

I am not sure why the Python 2.7 tests are suddenly failing, because those are tests you and me did not touch. 🤷 Very strange. (thought it is because of a file called socket.py, but there are other things also failing...

Could you please merge the newest master from getsentry/sentry-python into your branch (or rebase yours on top of it?) Maybe this helps. Thanks @hossein-raeisi

@hossein-raeisi
Copy link
Contributor Author

I am not sure why the Python 2.7 tests are suddenly failing, because those are tests you and me did not touch. shrug Very strange. (thought it is because of a file called socket.py, but there are other things also failing...

Could you please merge the newest master from getsentry/sentry-python into your branch (or rebase yours on top of it?) Maybe this helps. Thanks @hossein-raeisi

strange 🤔
seems like is already merged:
image
(remote sentry refers to getsentry/sentry-python)

@antonpirker
Copy link
Member

Hm. Now all the tests pass. I guess it was a GitHub hiccup, because they had problems with Github actions in the last couple of days.

@antonpirker antonpirker merged commit 5d9cd4f into getsentry:master Mar 30, 2023
@antonpirker
Copy link
Member

Congrats and thanks for you patience @hossein-raeisi !

Really great contribution! Thank you so much! We will release this in the next couple of days.

If you would like a little thank you gift please send me your shipping address and t-shirt size to anton.pirker@sentry.io

@hossein-raeisi
Copy link
Contributor Author

Congrats and thanks for you patience @hossein-raeisi !

Really great contribution! Thank you so much! We will release this in the next couple of days.

If you would like a little thank you gift please send me your shipping address and t-shirt size to anton.pirker@sentry.io

thanks, I would love that

@antonpirker
Copy link
Member

Your contributions have been released @hossein-raeisi : https://github.com/getsentry/sentry-python/releases/tag/1.19.0

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

Assignees

@antonpirker antonpirker

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hossein-raeisi @sl0thentr0py @antonpirker