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

Unify trace and metric exporter helpers #944

Merged

Conversation

matej-g
Copy link
Contributor

@matej-g matej-g commented Jul 17, 2020

Brings consistency to helper methods of trace and metric exporters (Prometheus, Jaeger, Zipkin). Each of these now has 3 helper methods with the same purpose:

  • NewRawExporter - method to return exporter only, with default or passed options
  • NewExportPipeline - method which creates a new exporter and uses it to return a provider with recommended configuration
  • InstallNewPipeline - same as NewExportPipeline, but it directly registers provider as global

Specifically the changeset includes:

  • for Jager exporter, introduced InstallNewPipeline and removed RegisterGlobal option instead
  • for Zipkin exporter, introduced pipeline methods and adjusted the new exporter method

New tests, example updates and minor improvements (e.g. added documentation) are included here as well.

Related Issues:

Resolves #800

CHANGELOG.md Outdated Show resolved Hide resolved
exporters/trace/jaeger/jaeger_test.go Outdated Show resolved Hide resolved
exporters/trace/zipkin/zipkin.go Outdated Show resolved Hide resolved
exporters/trace/zipkin/zipkin_test.go Outdated Show resolved Hide resolved
@matej-g
Copy link
Contributor Author

matej-g commented Jul 21, 2020

@MrAlias thanks for the feedback, I implemented the suggested changes

Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

A zipkin binary file was included in 623f125

This will need to be purged from the git history before this can merge.

@MrAlias
Copy link
Contributor

MrAlias commented Jul 22, 2020

Other than the binary purge, this is looking good and should be approved afterwards. Great job 👍

@matej-g matej-g force-pushed the unify-trace-and-metric-exporter-helpers branch from 0b26f9e to e0b7e3c Compare July 22, 2020 07:43
@matej-g
Copy link
Contributor Author

matej-g commented Jul 22, 2020

Other than the binary purge, this is looking good and should be approved afterwards. Great job

Sorry about that, removed the binary, hopefully now all is good 👍

@MrAlias MrAlias merged commit f31d8ec into open-telemetry:master Jul 22, 2020
This was referenced Jul 28, 2020
@pellared pellared added this to the untracked milestone Nov 8, 2024
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.

Inconsistency between trace and metric exporter helpers
6 participants