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

[OTLP Exporter] Use env for endpoint, timeout #451

Merged

Conversation

ozerovandrei
Copy link
Contributor

Add with_env method for OtlpPipelineBuilder. This method currently
only configures endpoint and timeout for OTLP Traces exporter.

Add default constants for those parameters and use them for the default
ExporterConfig. Use values defined in specification.

Add basic tests for the new method.

Specification reference: exporter.md

Part of #168

@ozerovandrei ozerovandrei requested a review from a team February 7, 2021 18:19
@codecov
Copy link

codecov bot commented Feb 7, 2021

Codecov Report

Merging #451 (f1596ed) into main (2269d1f) will increase coverage by 0.51%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #451      +/-   ##
==========================================
+ Coverage   47.91%   48.42%   +0.51%     
==========================================
  Files          95       95              
  Lines        8739     8739              
==========================================
+ Hits         4187     4232      +45     
+ Misses       4552     4507      -45     
Impacted Files Coverage Δ
opentelemetry-otlp/src/span.rs 17.77% <0.00%> (ø)
...ntelemetry/src/sdk/metrics/aggregators/ddsketch.rs 76.76% <0.00%> (-0.20%) ⬇️
opentelemetry/src/context.rs 62.09% <0.00%> (+30.06%) ⬆️

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 2269d1f...f1596ed. Read the comment docs.

@ozerovandrei ozerovandrei force-pushed the 168-otlp-span-exporter-env-cfg branch 2 times, most recently from 918464d to de53bb3 Compare February 7, 2021 19:52
@jtescher
Copy link
Member

@TommyCpp / @djc do you have further comments? or is this PR ready to be merged?

@TommyCpp
Copy link
Contributor

@TommyCpp / @djc do you have further comments? or is this PR ready to be merged?

Might as well fix the lint errors in this PR. But otherwise LGTM.

Add `with_env` method for `OtlpPipelineBuilder`. This method currently
only configures `endpoint` and `timeout` for OTLP Traces exporter.

Add default constants for those parameters and use them for the default
`ExporterConfig`. Use values defined in specification.

Add basic tests for the new method.
Check that env variables were unset after test.
@ozerovandrei ozerovandrei force-pushed the 168-otlp-span-exporter-env-cfg branch from bcf9774 to f1596ed Compare February 14, 2021 16:44
@ozerovandrei
Copy link
Contributor Author

@TommyCpp / @djc do you have further comments? or is this PR ready to be merged?

Might as well fix the lint errors in this PR. But otherwise LGTM.

I created a different PR to fix lint errors since they're unrelated to environment variables changes: #456

@jtescher
Copy link
Member

Ok sounds good

@jtescher jtescher merged commit 238e797 into open-telemetry:main Feb 14, 2021
@ozerovandrei ozerovandrei deleted the 168-otlp-span-exporter-env-cfg branch February 14, 2021 19:42
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.

4 participants