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

Move exporters out from core. #1118

Merged
merged 1 commit into from
Apr 23, 2019

Conversation

songy23
Copy link
Contributor

@songy23 songy23 commented Apr 19, 2019

Also run $ GO111MODULE=on go mod tidy.

@songy23 songy23 requested review from rakyll, rghetia and a team as code owners April 19, 2019 05:22
Copy link
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

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

LGTM

@rghetia
Copy link
Contributor

rghetia commented Apr 19, 2019

after vanity-url PR is merged I can replace with new dependencies in examples
Ideally, it would be nice to create a separate repo for example so we don't haver circular dependencies.

@songy23
Copy link
Contributor Author

songy23 commented Apr 19, 2019

after vanity-url PR is merged I can replace with new dependencies in examples
Ideally, it would be nice to create a separate repo for example so we don't haver circular dependencies.

Agree, I'll hold off this PR until examples are updated. Or we can move the exporter-specific examples to exporter repos.

@rghetia
Copy link
Contributor

rghetia commented Apr 19, 2019

The problem with moving examples to exporter repo is that most of the example related to core functionality will move to prometheus just because we use prometheus in most (if not all) examples.

@bogdandrutu, should we create example repo and simply link it in readme.md in core library?

@bogdandrutu
Copy link
Contributor

Probably we can have in each exporter repo it's own example.

@rghetia
Copy link
Contributor

rghetia commented Apr 19, 2019

Probably we can have in each exporter repo it's own example.

That would require that we repeat feature examples (like gauge) in all exporters.

@songy23
Copy link
Contributor Author

songy23 commented Apr 23, 2019

Rebased against the latest dev branch (included #1128). Should be good to merge now.

@songy23 songy23 merged commit a10a38b into census-instrumentation:dev Apr 23, 2019
@songy23 songy23 deleted the remove-exporters branch April 23, 2019 20:26
rghetia pushed a commit to rghetia/opencensus-go that referenced this pull request Apr 25, 2019
rghetia pushed a commit that referenced this pull request Apr 25, 2019
@hsanjuan
Copy link

If anyone lands here wondering where they were moved to: https://github.com/census-ecosystem (with imports as contrib.go.opencensus.io/exporter/jaeger etc.)

@rghetia rghetia mentioned this pull request May 2, 2019
rantav added a commit to rantav/go-sundheit that referenced this pull request Sep 22, 2019
This results in a better fix for the missing thrift repo on apache. The thrift version was required by opencensus but since oc factored the exporters out of the main package, this thrift dependency is no longer required. census-instrumentation/opencensus-go#1118

Incidently this also makes usage of go-sundheit easier for other libs as they don't need to work around the apache thrift issue themselves anymore
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants