-
Notifications
You must be signed in to change notification settings - Fork 207
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
feat: Add OpenTelemetry support for Subscribe Side #1252
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
05eeb93
to
fedbe39
Compare
6deb637
to
1703dd2
Compare
SubscribeSpan information in the SubscriberMessage's Open Telemetry data
* This is required for the Open Telemetry information to be percolated to the dispatcher which performs acks, nacks and modacks async. * The Open Telemetry data passed into the dispatcher will be used to add events such as ack, modack, nack end to the subscribe span
1703dd2
to
a695c99
Compare
* Both these methods should add the "ack start" event to the subscribe span
* Both these methods should add "nack start" events to the subscribe span
* Both these methods should add the "modack start" event to the subscribe span
e5c89fc
to
c1f0ad8
Compare
6ab4432
to
65aae0f
Compare
65aae0f
to
28a192a
Compare
* Currently, subscribe_span is ended after modack end event is added to the subscribe_span. This should not happen, since subscribe_span should be ended only after ack,nack or drop, but not modack * Remove an extraneous modack start that was present in streaming_pull_manager.send_lease_modacks(). This is not required, since modack start and end events will anyways be added to the subscribe span in the dispatcher.modify_ack_deadline() method. * Replace the NamedTuple._replace() methods with creation of new NamedTuple ModackRequest items. This is because NamedTuple._replace() returns a new NamedTuple, rather than replacing the existing one. This results in the opentelemetry data not being plumbed down into the dispatcher.modify_ack_deadline_method.
21a5716
to
4e17352
Compare
4e17352
to
5874a96
Compare
dispatcher.modify_ack_deadlin()
481f7a5
to
3206c19
Compare
91d6344
to
5dd8fb8
Compare
@hongalex PTAL |
@parthea PTAL |
hongalex
approved these changes
Sep 24, 2024
google/cloud/pubsub_v1/subscriber/_protocol/streaming_pull_manager.py
Outdated
Show resolved
Hide resolved
google/cloud/pubsub_v1/open_telemetry/subscribe_opentelemetry.py
Outdated
Show resolved
Hide resolved
* Addressing the review comment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
api: pubsub
Issues related to the googleapis/python-pubsub API.
size: xl
Pull request size is extra large.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Subscribe Side OpenTelemetry Code