Skip to content
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

Remove Azure Exporters out of main repo (master) #272

Merged
merged 5 commits into from
Nov 21, 2019

Conversation

lzchen
Copy link
Contributor

@lzchen lzchen commented Nov 6, 2019

Addresses [#193].
Remove Azure exporters out of OpenTelemetry repos.

I've published a new Pypi package called opentelemetry-azure-monitor-exporter which is in a separate repository. You can see it here.

@c24t I think we should deprecate the already existing package and link it to the new one.

@victoraugustolls FYI

@hectorhdzg
Copy link
Member

It would be good to add some reference to Azure repo in OpenTelemetry python readme or somewhere else so people are aware it exist, is there any plan for this?

Copy link
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

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

@c24t
Copy link
Member

c24t commented Nov 7, 2019

@lzchen why not continue to use the opentelemetry-ext-azure-monitor name?

@c24t c24t mentioned this pull request Nov 7, 2019
@lzchen
Copy link
Contributor Author

lzchen commented Nov 8, 2019

@c24t
For Microsoft, we've been using the term integrations instead of extensions. I am not that good at naming but I did not want to include anymore new terminology to our package names. opentelemetry-azure-monitor-exporter is very obvious at what it is for.

@c24t
Copy link
Member

c24t commented Nov 11, 2019

I did not want to include anymore new terminology to our package names

That makes sense, but now we've got two naming conventions. I opened #283 to track this.

@toumorokoshi
Copy link
Member

FYI technically you've relicensed that code from Apache2 to MIT, which generally isn't permitted without explicit permission from the code author.

I figure no one here is going to sue, but In the future we may want to consider license ramifications before moving code places.

@lzchen
Copy link
Contributor Author

lzchen commented Nov 14, 2019

@toumorokoshi
That is a good point. We've gone through the necessary legal resources to address this issue. As well, the code we are taking was written solely by @reyang so we shouldn't have any legal ramifications.

@codecov-io
Copy link

codecov-io commented Nov 14, 2019

Codecov Report

Merging #272 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #272   +/-   ##
=======================================
  Coverage   85.74%   85.74%           
=======================================
  Files          33       33           
  Lines        1620     1620           
  Branches      183      183           
=======================================
  Hits         1389     1389           
  Misses        184      184           
  Partials       47       47

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcdd9fe...875d73c. Read the comment docs.

@lzchen
Copy link
Contributor Author

lzchen commented Nov 18, 2019

@c24t Ready to merge?

@Oberon00 Oberon00 added the PR:please merge This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.) label Nov 19, 2019
@c24t c24t merged commit 077a08e into open-telemetry:master Nov 21, 2019
@c24t c24t removed the PR:please merge This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.) label Nov 23, 2019
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
* feat(metrics): add exporter-prometheus package

* doc(metrics): Add work in progress remark

* fix: removed tracing, kept metrics

* fix: Remove tslint.json
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.

6 participants