-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(internal/trace): add OpenTelemetry support #8655
Conversation
Add GOOGLE_API_GO_EXPERIMENTAL_TELEMETRY_PLATFORM_TRACING env var flag for opt-in OpenTelemetry tracing. refs: googleapis#2205
@@ -53,6 +113,18 @@ func toStatus(err error) trace.Status { | |||
} | |||
} | |||
|
|||
// toOpenTelemetryStatus converts an error to an equivalent OpenTelemetry status description. | |||
func toOpenTelemetryStatusDescription(err error) string { | |||
var err2 *googleapi.Error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this just be an APIError now? Everything should return those now, right? Same with the above toStatus stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into this, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this logic in bigtable shows a situation where the err
closing the span could be an unwrapped Status
or even a io.EOF
:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, but I just meant L117 here. Other cases can still be left.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah sorry, i misunderstood
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After trying this, I think it's best to just extract the googleapi.Error
(if found), since it exposes the HTTP error message. With APIError, that message is only exposed by Error()
but it gets some added prefix:
trace_test.go:131: got googleapi: Error 400: INVALID ARGUMENT, want INVALID ARGUMENT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would be fine to call Error and all HTTP APIError *should have wrap a googleapis.Error. But I am fine with this for now. We can always make that change later.
Sorry, marked as draft by mistake |
I tested this manually using only an OpenTelemetry exporter and the /cc @cjcotter |
🤖 I have created a release *beep* *boop* --- ## [0.111.0](https://togithub.com/googleapis/google-cloud-go/compare/v0.110.10...v0.111.0) (2023-11-29) ### Features * **internal/trace:** Add OpenTelemetry support ([#8655](https://togithub.com/googleapis/google-cloud-go/issues/8655)) ([7a46b54](https://togithub.com/googleapis/google-cloud-go/commit/7a46b5428f239871993d66be2c7c667121f60a6f)), refs [#2205](https://togithub.com/googleapis/google-cloud-go/issues/2205) ### Bug Fixes * **all:** Bump google.golang.org/api to v0.149.0 ([#8959](https://togithub.com/googleapis/google-cloud-go/issues/8959)) ([8d2ab9f](https://togithub.com/googleapis/google-cloud-go/commit/8d2ab9f320a86c1c0fab90513fc05861561d0880)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
* Change default for GOOGLE_API_GO_EXPERIMENTAL_TELEMETRY_PLATFORM_TRACING env var flag to OpenTelemetry. * See https://github.com/googleapis/google-cloud-go/blob/main/debug.md\#telemetry refs: googleapis#2205 refs: googleapis#8655
* Change default for `GOOGLE_API_GO_EXPERIMENTAL_TELEMETRY_PLATFORM_TRACING` env var flag to OpenTelemetry. * See https://togithub.com/googleapis/google-cloud-go/blob/main/debug.md\#telemetry refs: #2205 refs: #8655
🤖 I have created a release *beep* *boop* --- ## [0.115.0](https://togithub.com/googleapis/google-cloud-go/compare/v0.114.0...v0.115.0) (2024-06-12) ### Features * **internal/trace:** Deprecate OpenCensus support ([#10287](https://togithub.com/googleapis/google-cloud-go/issues/10287)) ([430ce8a](https://togithub.com/googleapis/google-cloud-go/commit/430ce8adea2d0be43461e2ca783b7c17794e983f)), refs [#2205](https://togithub.com/googleapis/google-cloud-go/issues/2205) [#8655](https://togithub.com/googleapis/google-cloud-go/issues/8655) ### Bug Fixes * **internal/postprocessor:** Use approved image tag ([#10341](https://togithub.com/googleapis/google-cloud-go/issues/10341)) ([a388fe5](https://togithub.com/googleapis/google-cloud-go/commit/a388fe5cf075d0af986861c70dcb7b9f97c31019)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Add GOOGLE_API_GO_EXPERIMENTAL_TELEMETRY_PLATFORM_TRACING env var for opt-in OpenTelemetry tracing.
refs: #2205
OpenCensus sunsetting and OpenTelemetry support
The OpenCensus project is obsolete and was archived on July 31st, 2023. This means that any security vulnerabilities that are found will not be patched. We recommend that you begin migrating to OpenCensus tracing to OpenTelemetry, the successor project.
One year after the Google Cloud Go client libraries release the experimental, opt-in OpenTelemetry tracing support in this PR (mid-Q4 2023), the prior experimental OpenCensus tracing will be removed.
Using the OpenTelemetry-Go - OpenCensus Bridge, you can immediately begin exporting your traces with OpenTelemetry, even while dependencies remain instrumented with OpenCensus. If you do not use the bridge, you will need to migrate your entire application and all of its instrumented dependencies at once. For simple applications, this may be possible, but we expect the bridge to be helpful if multiple libraries with instrumentation are used.
Please refer to the following resources:
Testing support for library maintainers
PTAL at
internal/testutil/trace_otel.go
and the update totrace_test.go
. I wasn't able to match the existingtestutil
api perfectly, so testing support for OpenTelemetry will not be completely transparent. However, the newtestutil
functions are very close:OpenCensus
OpenTelemetry