Skip to content

feat(pubsub): report publisher outstanding metrics#6187

Merged
hongalex merged 10 commits intogoogleapis:mainfrom
schallert:schallert/publisher_outstanding_metrics
Jun 23, 2022
Merged

feat(pubsub): report publisher outstanding metrics#6187
hongalex merged 10 commits intogoogleapis:mainfrom
schallert:schallert/publisher_outstanding_metrics

Conversation

@schallert
Copy link
Copy Markdown
Contributor

Fixes #6180.

@schallert schallert requested review from a team June 15, 2022 23:33
@product-auto-label product-auto-label Bot added size: m Pull request size is medium. api: pubsub Issues related to the Pub/Sub API. labels Jun 15, 2022
@schallert
Copy link
Copy Markdown
Contributor Author

Marking as draft for a bit. Based on internal testing, there's one more path where we need to upsert the topic tag on the context.

@schallert schallert marked this pull request as ready for review June 16, 2022 17:10
@schallert
Copy link
Copy Markdown
Contributor Author

Updated with the tag fix. With the most recent commit, the behavior we see in our testing is:

  1. While metrics are buffered but before the first publish call:
opencensus_cloud_google_com_go_pubsub_publisher_outstanding_messages{topic="<INTERNAL_TOPIC>"} 8
  1. After the publish call, outstanding goes back to 0 and published_messages increments to the total:
opencensus_cloud_google_com_go_pubsub_published_messages{error="",status="OK",topic="<INTERNAL_TOPIC>"} 8
opencensus_cloud_google_com_go_pubsub_publisher_outstanding_messages{topic="<INTERNAL_TOPIC>"} 0

@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 16, 2022
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 16, 2022
Comment thread pubsub/flow_controller.go Outdated
func (f *flowController) recordOutstandingBytes(ctx context.Context, n int64) {
if f.purpose == flowControllerPurposeTopic {
recordStat(ctx, PublisherOutstandingBytes, n)
} else {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we're not going to check the unknown value here, we should either remove that value above or explicitly check f.purpose == flowControllerPurposeSubscription here.

Comment thread pubsub/flow_controller.go Outdated
case flowControllerPurposeTopic:
recordStat(ctx, PublisherOutstandingMessages, n)
case flowControllerPurposeSubscription:
fallthrough
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Made this fall through to maintain backwards compatibility with the old behavior. However, as written there shouldn't be any cases where a "real" client has an unknown purpose, so I'm happy to make default a noop. Or I can remove the unknown value entirely and default to flowControllerPurposeSubscription.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should remove the unknown value since flowcontroller is only used for publisher/subscribers here. Thanks!

@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 23, 2022
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 23, 2022
@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 23, 2022
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 23, 2022
@hongalex hongalex merged commit cc1528b into googleapis:main Jun 23, 2022
@schallert schallert deleted the schallert/publisher_outstanding_metrics branch June 23, 2022 18:02
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 Pub/Sub API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pubsub: Emit outstanding messages metrics for publishers

3 participants