[GCP Observability C++] De-experimentalize API #32715
Merged
Conversation
This file contains hidden or 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
veblush
approved these changes
Mar 30, 2023
ctiller
reviewed
Mar 31, 2023
ctiller
reviewed
Mar 31, 2023
markdroth
reviewed
Mar 31, 2023
markdroth
approved these changes
Apr 4, 2023
veblush
approved these changes
Apr 5, 2023
ctiller
reviewed
Apr 5, 2023
ctiller
approved these changes
Apr 5, 2023
|
Thanks for providing reviews at such short notices! 🙏 |
yashykt
added a commit
to yashykt/grpc
that referenced
this pull request
Apr 5, 2023
This PR aims to de-experimentalize the APIs for GCP Observability. We would have ideally wanted public feedback before declaring the APIs stable, but we need stable APIs for GA. Changes made after API review with @markdroth @veblush, @ctiller and the entire Core/C++ team - * The old experimental APIs `grpc::experimental::GcpObservabilityInit` and `grpc::experimental::GcpObservabilityClose` are now deprecated and will be deleted after v.1.55 release. * The new API gets rid of the Close method and follows the RAII idiom with a single `grpc::GcpObservability::Init()` call that returns an `GcpObservability` object, the lifetime of which controls when observability data is flushed. * The `GcpObservability` class could in the future add more methods. For example, a debug method that shows the current configuration. * Document that GcpObservability initialization and flushing (on `GcpObservability` destruction) are blocking calls. * Document that gRPC is still usable if GcpObservability initialization failed. (Added a test to prove the same). * Since we don't have a good way to flush stats and tracing with OpenCensus, the examples required users to sleep for 25 seconds. This sleep is now part of `GcpObservability` destruction. Additional Implementation details - * `GcpObservability::Init` is now marked with `GRPC_MUST_USE_RESULT` to make sure that the results are used. We ideally want users to store it, but this is better than nothing. * Added a note on GCP Observability lifetime guarantees. <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. -->
yashykt
added a commit
that referenced
this pull request
Apr 5, 2023
) This PR aims to de-experimentalize the APIs for GCP Observability. We would have ideally wanted public feedback before declaring the APIs stable, but we need stable APIs for GA. Changes made after API review with @markdroth @veblush, @ctiller and the entire Core/C++ team - * The old experimental APIs `grpc::experimental::GcpObservabilityInit` and `grpc::experimental::GcpObservabilityClose` are now deprecated and will be deleted after v.1.55 release. * The new API gets rid of the Close method and follows the RAII idiom with a single `grpc::GcpObservability::Init()` call that returns an `GcpObservability` object, the lifetime of which controls when observability data is flushed. * The `GcpObservability` class could in the future add more methods. For example, a debug method that shows the current configuration. * Document that GcpObservability initialization and flushing (on `GcpObservability` destruction) are blocking calls. * Document that gRPC is still usable if GcpObservability initialization failed. (Added a test to prove the same). * Since we don't have a good way to flush stats and tracing with OpenCensus, the examples required users to sleep for 25 seconds. This sleep is now part of `GcpObservability` destruction. Additional Implementation details - * `GcpObservability::Init` is now marked with `GRPC_MUST_USE_RESULT` to make sure that the results are used. We ideally want users to store it, but this is better than nothing. * Added a note on GCP Observability lifetime guarantees. <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. --> <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. -->
stanley-cheung
added a commit
that referenced
this pull request
Apr 6, 2023
.. as it is now part of the `GcpObservabilityClose()` routine [itself](https://github.com/grpc/grpc/pull/32715/files#diff-e1ce0ccb4650e20b62a6d6b31655f446ad9ef72efca5849143e65f4a5a5e0310R223). Background: #32715 may have broken the CI for observability interop testing. The client seems to be taking too long to finish. More info at b/277145074 We need to backport this to the `v1.54.x` branch if this is the right fix.
stanley-cheung
added a commit
to stanley-cheung/grpc
that referenced
this pull request
Apr 6, 2023
…32817) .. as it is now part of the `GcpObservabilityClose()` routine [itself](https://github.com/grpc/grpc/pull/32715/files#diff-e1ce0ccb4650e20b62a6d6b31655f446ad9ef72efca5849143e65f4a5a5e0310R223). Background: grpc#32715 may have broken the CI for observability interop testing. The client seems to be taking too long to finish. More info at b/277145074 We need to backport this to the `v1.54.x` branch if this is the right fix.
stanley-cheung
added a commit
that referenced
this pull request
Apr 6, 2023
Backport of #32817 .. as it is now part of the `GcpObservabilityClose()` routine [itself](https://github.com/grpc/grpc/pull/32715/files#diff-e1ce0ccb4650e20b62a6d6b31655f446ad9ef72efca5849143e65f4a5a5e0310R223). Background: #32715 may have broken the CI for observability interop testing. The client seems to be taking too long to finish. More info at b/277145074 We need to backport this to the `v1.54.x` branch.
XuanWang-Amos
pushed a commit
to XuanWang-Amos/grpc
that referenced
this pull request
May 1, 2023
This PR aims to de-experimentalize the APIs for GCP Observability. We would have ideally wanted public feedback before declaring the APIs stable, but we need stable APIs for GA. Changes made after API review with @markdroth @veblush, @ctiller and the entire Core/C++ team - * The old experimental APIs `grpc::experimental::GcpObservabilityInit` and `grpc::experimental::GcpObservabilityClose` are now deprecated and will be deleted after v.1.55 release. * The new API gets rid of the Close method and follows the RAII idiom with a single `grpc::GcpObservability::Init()` call that returns an `GcpObservability` object, the lifetime of which controls when observability data is flushed. * The `GcpObservability` class could in the future add more methods. For example, a debug method that shows the current configuration. * Document that GcpObservability initialization and flushing (on `GcpObservability` destruction) are blocking calls. * Document that gRPC is still usable if GcpObservability initialization failed. (Added a test to prove the same). * Since we don't have a good way to flush stats and tracing with OpenCensus, the examples required users to sleep for 25 seconds. This sleep is now part of `GcpObservability` destruction. Additional Implementation details - * `GcpObservability::Init` is now marked with `GRPC_MUST_USE_RESULT` to make sure that the results are used. We ideally want users to store it, but this is better than nothing. * Added a note on GCP Observability lifetime guarantees. <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. -->
XuanWang-Amos
pushed a commit
to XuanWang-Amos/grpc
that referenced
this pull request
May 1, 2023
…32817) .. as it is now part of the `GcpObservabilityClose()` routine [itself](https://github.com/grpc/grpc/pull/32715/files#diff-e1ce0ccb4650e20b62a6d6b31655f446ad9ef72efca5849143e65f4a5a5e0310R223). Background: grpc#32715 may have broken the CI for observability interop testing. The client seems to be taking too long to finish. More info at b/277145074 We need to backport this to the `v1.54.x` branch if this is the right fix.
wanlin31
pushed a commit
that referenced
this pull request
May 18, 2023
This PR aims to de-experimentalize the APIs for GCP Observability. We would have ideally wanted public feedback before declaring the APIs stable, but we need stable APIs for GA. Changes made after API review with @markdroth @veblush, @ctiller and the entire Core/C++ team - * The old experimental APIs `grpc::experimental::GcpObservabilityInit` and `grpc::experimental::GcpObservabilityClose` are now deprecated and will be deleted after v.1.55 release. * The new API gets rid of the Close method and follows the RAII idiom with a single `grpc::GcpObservability::Init()` call that returns an `GcpObservability` object, the lifetime of which controls when observability data is flushed. * The `GcpObservability` class could in the future add more methods. For example, a debug method that shows the current configuration. * Document that GcpObservability initialization and flushing (on `GcpObservability` destruction) are blocking calls. * Document that gRPC is still usable if GcpObservability initialization failed. (Added a test to prove the same). * Since we don't have a good way to flush stats and tracing with OpenCensus, the examples required users to sleep for 25 seconds. This sleep is now part of `GcpObservability` destruction. Additional Implementation details - * `GcpObservability::Init` is now marked with `GRPC_MUST_USE_RESULT` to make sure that the results are used. We ideally want users to store it, but this is better than nothing. * Added a note on GCP Observability lifetime guarantees. <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. -->
wanlin31
pushed a commit
that referenced
this pull request
May 18, 2023
.. as it is now part of the `GcpObservabilityClose()` routine [itself](https://github.com/grpc/grpc/pull/32715/files#diff-e1ce0ccb4650e20b62a6d6b31655f446ad9ef72efca5849143e65f4a5a5e0310R223). Background: #32715 may have broken the CI for observability interop testing. The client seems to be taking too long to finish. More info at b/277145074 We need to backport this to the `v1.54.x` branch if this is the right fix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR aims to de-experimentalize the APIs for GCP Observability.
We would have ideally wanted public feedback before declaring the APIs stable, but we need stable APIs for GA.
Changes made after API review with @markdroth @veblush, @ctiller and the entire Core/C++ team -
grpc::experimental::GcpObservabilityInitandgrpc::experimental::GcpObservabilityCloseare now deprecated and will be deleted after v.1.55 release.grpc::GcpObservability::Init()call that returns anGcpObservabilityobject, the lifetime of which controls when observability data is flushed.GcpObservabilityclass could in the future add more methods. For example, a debug method that shows the current configuration.GcpObservabilitydestruction) are blocking calls.GcpObservabilitydestruction.Additional Implementation details -
GcpObservability::Initis now marked withGRPC_MUST_USE_RESULTto make sure that the results are used. We ideally want users to store it, but this is better than nothing.