Skip to content

Merge opentelemetry-stackdriver into this repo. #487

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

Merged
merged 107 commits into from
Mar 21, 2021
Merged

Merge opentelemetry-stackdriver into this repo. #487

merged 107 commits into from
Mar 21, 2021

Conversation

Xaeroxe
Copy link
Contributor

@Xaeroxe Xaeroxe commented Mar 18, 2021

Per @djc this PR merges the history of https://github.com/vivint-smarthome/opentelemetry-stackdriver into this repository.

I, the author of this crate, will soon have no further reason to work on this as I'm changing employers. When I informed @djc of this they suggested that we transfer this crate to the open-telemetry organization. That seems like a good idea to me, so this PR aims to accomplish that. If this PR is accepted I'll transfer the ownership on crates.io accordingly.

@Xaeroxe Xaeroxe requested a review from a team March 18, 2021 17:33
@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Mar 18, 2021

@jasonahills @luisholanda can you guys agree to the CLA here?

@djc
Copy link
Contributor

djc commented Mar 18, 2021

Note, I've been a fairly substantial contributor to this project for the last few months -- I would be continuing my maintenance on this crate as part of the otel repo organization.

There might be an issue here because opentelemetry-stackdriver depends on tracing-opentelemetry, which depends on opentelemetry but does not live in this repository (and generally gets updated a bit slower than stuff in this repository).

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Mar 18, 2021

That's true @djc but that's only a dependency because of the example. I've deleted the example for now as it's non-trivial to get it working from this location. So, we can put off that problem.

@djc
Copy link
Contributor

djc commented Mar 18, 2021

Oh, good call!

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Mar 18, 2021

I have no idea why CI is failing at this point. I can't reproduce that problem locally, and I'm not getting any useful info from the CI logs.

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Mar 18, 2021

It's worth noting the code coverage test is the only one using a nightly toolchain.

Copy link
Member

@jtescher jtescher left a comment

Choose a reason for hiding this comment

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

@Xaeroxe the coverage tests are a previous issue, don't worry about those. I think this org has an issue with MIT license, maybe remove and just keep the Apache 2?

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Mar 19, 2021

@jtescher Done! Thanks for your review!

Copy link
Member

@jtescher jtescher left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@TommyCpp TommyCpp left a comment

Choose a reason for hiding this comment

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

LGTM. We may need to edit the README file to move the opentelemetry-stackdriver into projects maintained by opentelemetry but we can also do that in a different PR. Thanks for the work on this!

@jtescher jtescher merged commit a32b962 into open-telemetry:main Mar 21, 2021
@Xaeroxe Xaeroxe deleted the stackdriver branch March 23, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants