Skip to content
This repository has been archived by the owner on Dec 31, 2023. It is now read-only.

feat!: migrate to microgenerator #29

Merged
merged 10 commits into from Sep 14, 2020
Merged

feat!: migrate to microgenerator #29

merged 10 commits into from Sep 14, 2020

Conversation

busunkim96
Copy link
Contributor

@busunkim96 busunkim96 commented Jul 30, 2020

This library has a manual surface that calls out to the GAPIC layer. I suspect this is mainly because of when this library was written. See PR for context.

The only added functionality seems to be a stored project_id. This PR removes the handwritten layer since the costs of maintaining that handwritten layer seem greater than the benefits. The addition of proto plus makes the other helpers (dict to protobuf and datetime conversion) obsolete.

There is a directory of samples written against the new surface to help folks who were using the manual wrapper adjust their code.

CC @simonz130 and @daniel-sanche since this is one of the Stackdriver libraries.

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 30, 2020
@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 30, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 30, 2020
@product-auto-label product-auto-label bot added the api: cloudtrace Issues related to the googleapis/python-trace API. label Aug 21, 2020
@busunkim96 busunkim96 marked this pull request as ready for review August 26, 2020 16:03
@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 27, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 27, 2020
@busunkim96
Copy link
Contributor Author

I borked the repo name in the kokoro config 😆. Will re-run shortly.

@busunkim96 busunkim96 added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Sep 3, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 3, 2020
@daniel-sanche
Copy link

daniel-sanche commented Sep 11, 2020

This PR removes the handwritten layer since the costs of maintaining that handwritten layer seem greater than the benefits

I believe the long term plan is for our team to eventually update and maintain the handwritten libraries for this. It may be worthwhile keeping the existing API surface around instead of forcing customers to switch back and forth

That being said, I don't think Trace is going to be on our radar for a while. And I'm not familiar with the library, so I'd have to do some digging to see if the existing hand-written part is even worth keeping. It looks pretty light weight. I'd probably prefer to go fully auto-generated for now and re-assess later (if the customer impact isn't too high). What do you think @simonz130?

@busunkim96
Copy link
Contributor Author

I think it's uncommon for folks to interact with this library directly. It looks like the docs push folks to interact with the Trace API via OpenCensus or OpenTelemetry.

https://cloud.google.com/trace/docs/client-libraries

For your application to submit traces to Cloud Trace, it must be instrumented. You can instrument your code by using the Google client libraries. However, it's recommended that you use OpenTelemetry or OpenCensus to instrument your application. These are open source tracing packages. OpenTelemetry is actively in development and is the preferred package.

The OpenCensus StackDriver Trace Exporter declares google-cloud-trace as a dependency. https://github.com/census-instrumentation/opencensus-python/blob/862885af8081dc3ede50d1df4a28e4400c22e8f8/contrib/opencensus-ext-stackdriver/setup.py#L43

@simonz130
Copy link

We should trim the manual layer from this library and make a major release as a result. We should focus on steering folks to use OT exporter instead of this library directly.

Copy link

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

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

Yeah, that makes sense to me

@busunkim96
Copy link
Contributor Author

Thanks @simonz130 @daniel-sanche! I'll cut a release on Monday.

@busunkim96 busunkim96 merged commit f0d9d91 into master Sep 14, 2020
@busunkim96 busunkim96 deleted the microgen branch September 14, 2020 17:33
@release-please release-please bot mentioned this pull request Sep 14, 2020
gcf-merge-on-green bot pushed a commit that referenced this pull request Sep 14, 2020
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.0.0](https://www.github.com/googleapis/python-trace/compare/v0.24.0...v1.0.0) (2020-09-14)


### ⚠ BREAKING CHANGES

* migrate to microgenerator (#29)

### Features

* migrate to microgenerator ([#29](https://www.github.com/googleapis/python-trace/issues/29)) ([f0d9d91](https://www.github.com/googleapis/python-trace/commit/f0d9d9161d7aee344ad1765c477947cd04505bf5))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
@kintel
Copy link

kintel commented Sep 15, 2020

Thanks for getting this done. I've pinged the Cloud Trace team in case they want to follow up, but it looks great!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: cloudtrace Issues related to the googleapis/python-trace API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants