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

add NewExportPipeline and InstallNewPipeline for otlp #1373

Merged
merged 15 commits into from
Apr 8, 2021
Merged

add NewExportPipeline and InstallNewPipeline for otlp #1373

merged 15 commits into from
Apr 8, 2021

Conversation

binjip978
Copy link
Contributor

Signed-off-by: binjip978 binjip978@gmail.com

Signed-off-by: binjip978 <binjip978@gmail.com>
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 28, 2020

CLA Signed

The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Nov 28, 2020

Codecov Report

Merging #1373 (c00d808) into main (7d8e6bd) will increase coverage by 0.0%.
The diff coverage is 73.9%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #1373   +/-   ##
=====================================
  Coverage   78.8%   78.8%           
=====================================
  Files        134     134           
  Lines       7157    7180   +23     
=====================================
+ Hits        5641    5660   +19     
- Misses      1270    1271    +1     
- Partials     246     249    +3     
Impacted Files Coverage Δ
exporters/otlp/otlp.go 83.9% <73.9%> (-7.0%) ⬇️
sdk/trace/batch_span_processor.go 83.9% <0.0%> (ø)
exporters/otlp/otlpgrpc/connection.go 90.6% <0.0%> (+1.8%) ⬆️

@binjip978
Copy link
Contributor Author

fixes #1347

Hello, i tried to make it consistent API with rest of NewExportPipeline and InstallNewPipeline, but it's hard, will be nice to hear some feedback.

exporters/otlp/otlp.go Outdated Show resolved Hide resolved
exporters/otlp/otlp.go Outdated Show resolved Hide resolved
exporters/otlp/otlp.go Outdated Show resolved Hide resolved
exporters/otlp/otlp.go Outdated Show resolved Hide resolved
exporters/otlp/otlp.go Outdated Show resolved Hide resolved
exporters/otlp/otlp.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -52,6 +52,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Added

- The OTLP exporter now has two new convenience functions, `NewExportPipeline` and `InstallNewPipeline`, setup and install the exporter in tracing and metrics pipelines. (#1348)
Copy link
Member

Choose a reason for hiding this comment

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

This should be un the Unreleased section above, not under 0.14.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated and merged with current master

Signed-off-by: binjip978 <binjip978@gmail.com>
binjip978 added 2 commits January 16, 2021 09:04
Signed-off-by: binjip978 <binjip978@gmail.com>
Signed-off-by: binjip978 <binjip978@gmail.com>
Base automatically changed from master to main January 29, 2021 19:39
@binjip978
Copy link
Contributor Author

Hi, @Aneurysm9, @MrAlias is this PR is still relevant? Im rebasing it time to time to make it work, but I not if it still needed. :)

Copy link
Member

@Aneurysm9 Aneurysm9 left a comment

Choose a reason for hiding this comment

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

Please fix up the CHANGELOG entry, otherwise LGTM.

CHANGELOG.md Outdated Show resolved Hide resolved
Sergei Semenchuk and others added 2 commits April 8, 2021 06:47
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
CHANGELOG.md Outdated Show resolved Hide resolved
exporters/otlp/otlp.go Outdated Show resolved Hide resolved
Sergei Semenchuk and others added 2 commits April 8, 2021 20:51
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@MrAlias MrAlias merged commit 5843280 into open-telemetry:main Apr 8, 2021
@MrAlias MrAlias mentioned this pull request Apr 23, 2021
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.

5 participants